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 [EMAIL PROTECTED] EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---------------------------(end of broadcast)--------------------------- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
