Bruce Momjian <bruce@momjian.us> writes:
> There were three things wrong with the original patch:
> o spacing, e.g. "if( x =- 1 )"
> o an incorrect API for memory freeing by parse_value()
> o verify_config_option() didn't consider custom variables
> These have all been corrected, so I don't see the value in removing the
> patch now that it is working.
You mean, there were three things wrong that we've identified so far,
and as for "it's working now", you thought it worked each previous
time you applied or patched it. I repeat my statement that I've got
zero confidence in it. It needs line-by-line review, and not by you.
I don't have time for it right now (I have more important things
to worry about, like bitmap indexes). Unless Peter or someone else
who knows the GUC code very well is willing to look it over pronto,
I want it out so it's not destabilizing things while we try to get
other patches committed.
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.)
regards, tom lane
