Joshua Brindle <joshua.brin...@crunchydata.com> writes:
> On Wed, Mar 16, 2022 at 3:06 PM Tom Lane <t...@sss.pgh.pa.us> wrote:
>> It's going to be hard to do anything useful in a hook that (a) does
>> not know which GUC is being assigned to and (b) cannot do catalog
>> accesses for fear that we're not inside a transaction. (b), in
>> particular, seems like a rather thorough API break; up to now
>> ObjectPostAlter hooks could assume that catalog accesses are OK.
> Can you elaborate on this point?
Hmm, I glossed over too many details there perhaps. I was thinking
about the restrictions on GUC check_hooks, which can be run outside
a transaction altogether. But that's not quite relevant here.
ExecSetVariableStmt can assume it's inside a transaction, but what
it *can't* assume is that we've set a transaction snapshot as yet
(cf. PlannedStmtRequiresSnapshot). If we call an ObjectPostAlter hook
there, and it does a catalog access, that's going to break things for
modifications of GUCs that are supposed to be modifiable without
freezing the transaction snapshot. So the net result is the same:
catalog access not okay, at least not in general.
Between that and the fact that an OID-based API is largely useless here,
I don't think it's sane to try to use the existing ObjectPostAlter API.
Maybe there is a case for inventing a separate hook API that could pass
the GUC name as a string, instead. I remain of the opinion that this
patch should not concern itself with that, though.
regards, tom lane
