> On Apr 4, 2020, at 7:11 AM, Thomas Munro <thomas.mu...@gmail.com> wrote:
>
> On Sat, Apr 4, 2020 at 4:45 AM Mark Dilger <mark.dil...@enterprisedb.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
