Re: Fairly serious bug induced by latest guc enum changes

Magnus Hagander <magnus(at)hagander(dot)net> writes:
> I still think going with the older method would be the safest, though,
> for the other reasons. You agree?

Seems reasonable to me.

> (I assume you mean GUC enum here, that seems fairly obvious)

Sorry, was writing in too much of a hurry.

> In these, the value was previously derived from a string and set in the
> hook. It's now set directly by the GUC code, and the hook only updates
> "other things" (setting the actual syslog facility, and resetting the
> cache, respectively).

> I think that means there are no bugs there.

Yeah, that's fine. I think though that I may have created a bug inside
GUC itself: the new stacking code could conceivably fail (palloc error)
between success return from the assign hook and setting up the stack
entry that is needed to undo the assignment on abort. In this situation
the assign hook would've made its "other thing" changes but there is no
GUC state to cause the hook to be called again to undo 'em. I need to
fix it so that any palloc'ing needed is done before calling the assign
hook.

regards, tom lane