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

> On Apr 4, 2020, at 7:11 AM, Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
>
> On Sat, Apr 4, 2020 at 4:45 AM Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com> wrote:
>> FYI, (not the responsibility of this patch), we never quite define what the abbreviation "xip" stands for. If "Active xid8s at the time of the snapshot." were rewritten as "In progress xid8s at the time of the snapshot", it might be slightly easier for the reader to figure out that "xip" = "Xid8s In Progress". As it stands, nothing in the docs seems to 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_activity exposes both "backend_xid" and "backend_xmin" as (32-bit) xid. Your new function names are not sufficiently distinct 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, if the xid columns in pg_stat_activity and elsewhere eventually get upgraded to xid8 fields, then the new naming convention in v10 will be cleaner.

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

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