Re: oat_post_create expected behavior

On Mon, 2022-06-06 at 10:51 -0400, Robert Haas wrote:
> I don't think a proposal to add CommandCounterIncrement() calls just
> for the convenience of object access hooks has much chance of being
> accepted.

Out of curiosity, why not? The proposed patch only runs it if the
object access hook is set. Do you see a situation where it would be
confusing that an earlier DDL change is visible? And if so, would it
make more sense to call CCI unconditionally?

Also, would it ever be reasonable for such a hook to call CCI itself?
As you say, it could use SnapshotSelf, but sometimes you might want to
call routines that assume they can use an MVCC snapshot. This question
applies to the OAT_POST_ALTER hook as well as OAT_POST_CREATE.

> Possibly there is some work that could be done to ensure
> consistent placement of the calls to post-create hooks so that either
> all of them happen before, or all of them happen after, a CCI has
> occurred, but I'm not sure whether or not that is feasible.

I like the idea of having a test in place so that we at least know when
something changes. Otherwise it would be pretty easy to break an
extension by adjusting some code.

> Currently,
> I don't think we promise anything about whether a post-create hook
> call will occur before or after a CCI, which is why
> sepgsql_schema_post_create(), sepgsql_schema_post_create(), and
> sepgsql_attribute_post_create() perform a catalog scan using
> SnapshotSelf, while sepgsql_database_post_create() uses
> get_database_oid(). You might want to adopt a similar technique.

It would be good to document this a little better though:

* OAT_POST_CREATE should be invoked just after the object is created.
* Typically, this is done after inserting the primary catalog records
and
* associated dependencies.

doesn't really give any guidance, while the comment for alter does:

* OAT_POST_ALTER should be invoked just after the object is altered,
* but before the command counter is incremented. An extension using
the
* hook can use a current MVCC snapshot to get the old version of the
tuple,
* and can use SnapshotSelf to get the new version of the tuple.

Regards,
Jeff Davis

PS: I added this to the July CF:
https://commitfest.postgresql.org/38/3676/