Thread: InvokeObjectPostAlterHook() vs. CommandCounterIncrement()
objectaccess.h has this to say:
* OAT_POST_ALTER should be invoked just after the object is altered,* but before the command counter is incremented.
Anextension using the* hook can use SnapshotNow and SnapshotSelf to get the old and new* versions of the tuple.
That's a clever design, but it precludes CommandCounterIncrement() appearing
between an applicable catalog change and the InvokeObjectPostAlterHook() call.
Otherwise, both SnapshotNow and SnapshotSelf see the new version. I find at
least one violation of that requirement. AlterTSConfiguration() calls
makeConfigurationDependencies(), which typically issues a CCI, before
InvokeObjectPostAlterHook(). A second example, arguably harmless, is
AlterEnum(). It can get a CCI through RenumberEnumType(), so SnapshotNow sees
the post-renumbered entries while SnapshotSelf sees those plus the added one.
sepgsql does not currently intercede for text search configurations or enums,
so only a third-party object_access_hook consumer could encounter the problem.
This is a dangerously-easy problem to introduce; unrelated development could
add a CCI to some intermediate code in a DDL implementation and quietly break
object_access_hook consumers. Can we detect that mistake? One possibility is
to call a macro, say StartObjectAlter(), before the first catalog modification
associated with a particular hook invocation. This macro would stash the
current CID. Then, RunObjectPostAlterHook() would elog(ERROR) if the CID had
changed since that moment. But how would one actually fix the code after such
a check fires? It's rarely easy to avoid adding a CCI.
I'm thinking we would be better off saying the firing point needs to appear
right after the heap_{insert,update,delete} + CatalogUpdateIndexes(). The
hooks already have the form of applying to a single catalog row rather than a
complete DDL operation, and many of the firing sites are close to that
placement already. If these hooks will need to apply to a larger operation, I
think that mandates a different means to reliably expose the before/after
object states.
Thanks,
nm
--
Noah Misch
EnterpriseDB http://www.enterprisedb.com
<p dir="ltr">On Jul 21, 2013 4:06 AM, "Noah Misch" <<a href="mailto:noah@leadboat.com">noah@leadboat.com</a>> wrote:<br/> > If these hooks will need to apply to a larger operation, I<br /> > think that mandates a different meansto reliably expose the before/after<br /> > object states.<p dir="ltr">I haven't checked the code to see how it wouldfit the API, but what about taking a snapshot before altering and passing this to the hook. Would there be other issuesbesides performance? If the snapshot is taken only when there is a hook present then the performance can be fixed later.<pdir="ltr">Regards,<br /> Ants Aasma
On Sun, Jul 21, 2013 at 11:44:51AM +0300, Ants Aasma wrote: > On Jul 21, 2013 4:06 AM, "Noah Misch" <noah@leadboat.com> wrote: > > If these hooks will need to apply to a larger operation, I > > think that mandates a different means to reliably expose the before/after > > object states. > > I haven't checked the code to see how it would fit the API, but what about > taking a snapshot before altering and passing this to the hook. Would there > be other issues besides performance? If the snapshot is taken only when > there is a hook present then the performance can be fixed later. That would work. -- Noah Misch EnterpriseDB http://www.enterprisedb.com
On Sun, Jul 21, 2013 at 4:44 AM, Ants Aasma <ants.aasma@eesti.ee> wrote: > On Jul 21, 2013 4:06 AM, "Noah Misch" <noah@leadboat.com> wrote: >> If these hooks will need to apply to a larger operation, I >> think that mandates a different means to reliably expose the before/after >> object states. > > I haven't checked the code to see how it would fit the API, but what about > taking a snapshot before altering and passing this to the hook. Would there > be other issues besides performance? If the snapshot is taken only when > there is a hook present then the performance can be fixed later. I had the idea of finding a way to pass either the old tuple, or perhaps just the TID of the old tuple. Not sure if passing a snapshot is better. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas wrote: > On Sun, Jul 21, 2013 at 4:44 AM, Ants Aasma <ants.aasma@eesti.ee> wrote: > > On Jul 21, 2013 4:06 AM, "Noah Misch" <noah@leadboat.com> wrote: > >> If these hooks will need to apply to a larger operation, I > >> think that mandates a different means to reliably expose the before/after > >> object states. > > > > I haven't checked the code to see how it would fit the API, but what about > > taking a snapshot before altering and passing this to the hook. Would there > > be other issues besides performance? If the snapshot is taken only when > > there is a hook present then the performance can be fixed later. > > I had the idea of finding a way to pass either the old tuple, or > perhaps just the TID of the old tuple. Not sure if passing a snapshot > is better. It seems this issue was forgotten. Any takers? -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services