Re: Should we add xid_current() or a int8->xid cast? - Mailing list pgsql-hackers

> On Apr 4, 2020, at 7:11 AM, Thomas Munro <thomas.munro@gmail.com> wrote:
>
> On Sat, Apr 4, 2020 at 4:45 AM Mark Dilger <mark.dilger@enterprisedb.com> wrote:
>> FYI, (not the responsibility of this patch), we never quite define what the abbreviation "xip" stands for.  If
"Activexid8s at the time of the snapshot." were rewritten as "In progress xid8s at the time of the snapshot", it might
beslightly easier for the reader to figure out that "xip" = "Xid8s In Progress".  As it stands, nothing in the docs
seemsto explain the abbrevation.  See doc/src/sgml/func.sgml 
>
> You're right.  Done.

Thanks!

> However, I am getting cold feet about the new function names.  The
> existing naming structure made sense when all this stuff originated in
> a contrib module with "txid_" as a prefix all over the place, but now
> that 64 bit IDs are a core concept, I wonder if we shouldn't aim for
> something that looks a little more like core functionality and doesn't
> have those "xid8_" warts in the names.

The "xid8_" warts are partly motivated by having a type named "xid8", which is a bit of a wart in itself.

> Here's what I now propose:
>
> Transaction ID functions, using names that fit with others (cf
> pg_xact_commit_timestamp()):
>
>  pg_current_xact_id()
>  pg_current_xact_id_if_assigned()
>  pg_xact_status(xid8)
>
> Snapshot functions (cf pg_export_snapshot()):
>
>  pg_current_snapshot()
>  pg_snapshot_xmin(pg_snapshot)
>  pg_snapshot_xmax(pg_snapshot)
>  pg_snapshot_xip(pg_snapshot)
>  pg_visible_in_snapshot(xid8, pg_snapshot)

I like some aspects of this, but not others.  Function pg_stat_get_activity(), which gets exposed through view
pg_stat_activityexposes both "backend_xid" and "backend_xmin" as (32-bit) xid.  Your new function names are not
sufficientlydistinct from these older names for users to easily remember the difference: 

select pg_snapshot_xmax(st.snap)
    from snapshot_test st, pg_stat_activity sa
    where pg_snapshot_xmin(st.snap) = sa.backend_xmin;
ERROR:  operator does not exist: xid8 = xid

SELECT * FROM pg_stat_activity s WHERE backend_xid = pg_current_xact_id();
ERROR:  operator does not exist: xid = xid8

SELECT pg_xact_status(backend_xmin), * FROM pg_stat_activity;
ERROR:  function pg_xact_status(xid) does not exist

It's not the end of the world, and users can figure out to put a cast on those, but it's kind of ugly.

It was cleaner before v10, when "backend_xid = xid8_current()" clearly had a xid vs. xid8 mismatch. On the other hand,
ifthe xid columns in pg_stat_activity and elsewhere eventually get upgraded to xid8 fields, then the new naming
conventionin v10 will be cleaner. 

As such, I'm ±0 for the change.


—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company