Re: [COMMITTERS] pgsql: Do a pass of code review for the

Alvaro Herrera wrote:
> Bruce Momjian wrote:
> > Neil Conway wrote:
> > > Log Message:
> > > -----------
> > > Do a pass of code review for the ALTER TABLE ADD INHERITS patch. Keep
> > > the read lock we hold on the table's parent relation until commit.
> > > Update equalfuncs.c for the new field in AlterTableCmd. Various
> > > improvements to comments, variable names, and error reporting.
> > >
> > > There is room for further improvement here, but this is at least
> > > a step in the right direction.
> >
> > Thanks, that is what was needed. The author obviously took the patch as
> > far as he could, and we needed to adjust his XXX areas, rather than not
> > apply the patch and have the code drifting.
>
> Hmm, is this how we should do things? I mean, should I finish the
> autovacuum parts of my relminxid patch, apply it, and then hope for
> someone to fix the mistaeks? And if we don't see any failure in the
> buildfarm, assume that all is well?
>
> To me this is really the easiest way, but I have a hard time convincing
> myself that I want to have it easy but break things in a way that nobody
> notices. The other day when I typoed a commit to the 8.1 branch I was
> all red in the face. I wonder what will happen if someone points to me
> or Greg as causing major breakage somewhere, just because the patch was
> applied in a hurry without careful review.

The author had been through several iterations of the patch, and I
needed to clean it up to look more like our code.

The XXX comments where of the variety where he was asking for
confirmation on things, and we can adjust those after application.
Doing it with his version of the patch would have been harder. If the
issues aren't addressed, the patch is eventually reverted, as we have
done in the past.

--
Bruce Momjian bruce(at)momjian(dot)us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +