Обсуждение: 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