Re: [PATCHES] Custom variable class segmentation fault

Andrew Dunstan wrote:
>
> Bruce,
>
> I think you have misunderstood.
>
> If you and Peter have reviewed it thoroughly then I see no reason the
> patch should not be applied.

We have. I did extensive rework, and Peter exchanged emails with the
author asking questions. I did have questions about how the original
patch was freeing memory, but didn't second-guess the author. Turns out
it needed rework, and that was done after build farm failures.

> My post below was merely to agree with Tom that in principle, patches
> should be be reviewed before application and not after. I still think
> that's right - I have been concerned lately that the buildfarm has been
> broken a bit too much.

Well, just because they are reviewed doesn't mean they aren't going to
break the build farm. In fact, the build farm is there to be broken ---
if all patches worked fine on all machines, we wouldn't need the build
farm. Let's not get into a case where keeping the build farm green is
our primary goal, "Oh, let's not apply that patch or it might break the
build farm". Hey, I have an idea, let's stop CVS update on the build
farm, and it will stay green forever. :-) LOL (Of course, we don't
want the build farm to stay broken or it masks newly introduced errors.)

If you withdraw your object to the GUC patch, then with a single person
objecting, the patch either goes in or that person takes responsibility
for getting it into 8.2, or the blame for leaving it for 8.3.

---------------------------------------------------------------------------

> cheers
>
> andrew
>
>
> Bruce Momjian wrote:
> > Zdenek Kotala wrote:
> >
> >> Bruce, Andrew, Tom.
> >>
> >> I little bit confuse now, what status of this patch is? I check your
> >> observation and I agree with them. But I don't where is "ball" now and
> >> what I can/must do now like author of this patch?
> >>
> >
> > I am unsure too. I would not back out a patch for nonspecific concerns
> > from one person, but from two people I reverted it. Tom wants more eyes
> > on it, but I don't know how that is going to happen, especially since
> > Peter, who has done a lot of GUC work, has reviewed it already, and so
> > have I.
> >
> > I will keep the patches and if no one works on it, again ask to apply it
> > before we finish 8.2, and see if there are still objections. If there
> > are still objections, we will have to vote on whether we want it
> > applied.
> >
> > ---------------------------------------------------------------------------
> >
> >
> >
> >
> >> Bruce Momjian napsal(a):
> >>
> >>> OK, with two people now concerned, patch reverted.
> >>>
> >>> ---------------------------------------------------------------------------
> >>>
> >>> Andrew Dunstan wrote:
> >>>
> >>>> Tom Lane wrote:
> >>>>
> >>>>> I've always found it easier to review uncommitted patches than committed
> >>>>> ones anyway. When you're playing catch-up by reviewing a committed
> >>>>> patch, you have to deal with three states of the code rather than two
> >>>>> (pre-patch, post-patch, your own mods). That gets rapidly worse if the
> >>>>> patch has been in there awhile and other changes go in on top of it.
> >>>>> Plus, once other changes accumulate on top, it becomes extremely painful
> >>>>> to revert if the conclusion is that the patch is completely broken.
> >>>>> (A conclusion that I don't think is at all unlikely with respect to
> >>>>> this patch.)
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>> Easy or not this strikes me as good policy. And nothing is urgent quite
> >>>> yet - we still have another 18 days to the end of the month, which is
> >>>> the stated deadline for getting patches reviewed and committed.
> >>>>
> >>>> cheers
> >>>>
> >>>> andrew
> >>>>
> >>>> ---------------------------(end of broadcast)---------------------------
> >>>> TIP 2: Don't 'kill -9' the postmaster
> >>>>
> >
> >

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

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