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

> On Apr 1, 2020, at 8:21 PM, Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
>
> On Sat, Mar 21, 2020 at 11:14 AM Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
>> * updated OIDs to avoid collisions
>> * added btequalimage to btree/xid8_ops
>
> Here's the version I'm planning to commit tomorrow, if no one objects. Changes:
>
> * txid.c renamed to xid8funcs.c
> * remaining traces of "txid" replaced various internal identifiers
> * s/backwards compatible/backward compatible/ in funcs.sgml (en_GB -> en_US)
> <v8-0001-Add-SQL-type-xid8-to-expose-FullTransactionId-to-.patch><v8-0002-Introduce-xid8_XXX-functions-to-replace-txid_XXX.patch>

Hi Thomas, Thanks for working on this.

I'm taking a quick look at your patches. It's not a big deal, and certainly not a show stopper if you want to go ahead with the commit, but you've left some mentions of "txid_current" that might better be modified to use the new name "xid8_current". At least one mention of "txid_current" is needed to check that the old name still works, but leaving this many in the regression test suite may lead other developers to follow that lead and use txid_current() in newly developed code. ("xid8_current" is not exercised by name anywhere in the regression suite, that I can see.)

> contrib/test_decoding/expected/ddl.out:SELECT txid_current() != 0; -- so no fixed xid apears in the outfile
> contrib/test_decoding/expected/decoding_in_xact.out:SELECT txid_current() = 0;
> contrib/test_decoding/expected/decoding_in_xact.out:SELECT txid_current() = 0;
> contrib/test_decoding/expected/decoding_in_xact.out:SELECT txid_current() = 0;
> contrib/test_decoding/expected/oldest_xmin.out:step s0_getxid: SELECT txid_current() IS NULL;
> contrib/test_decoding/expected/ondisk_startup.out:step s2txid: SELECT txid_current() IS NULL;
> contrib/test_decoding/expected/ondisk_startup.out:step s3txid: SELECT txid_current() IS NULL;
> contrib/test_decoding/expected/ondisk_startup.out:step s2txid: SELECT txid_current() IS NULL;
> contrib/test_decoding/expected/snapshot_transfer.out:step s0_log_assignment: SELECT txid_current() IS NULL;
> contrib/test_decoding/expected/snapshot_transfer.out:step s0_log_assignment: SELECT txid_current() IS NULL;
> contrib/test_decoding/specs/oldest_xmin.spec:step "s0_getxid" { SELECT txid_current() IS NULL; }
> contrib/test_decoding/specs/ondisk_startup.spec:step "s2txid" { SELECT txid_current() IS NULL; }
> contrib/test_decoding/specs/ondisk_startup.spec:step "s3txid" { SELECT txid_current() IS NULL; }
> contrib/test_decoding/specs/snapshot_transfer.spec:step "s0_log_assignment" { SELECT txid_current() IS NULL; }
> contrib/test_decoding/sql/ddl.sql:SELECT txid_current() != 0; -- so no fixed xid apears in the outfile
> contrib/test_decoding/sql/decoding_in_xact.sql:SELECT txid_current() = 0;
> contrib/test_decoding/sql/decoding_in_xact.sql:SELECT txid_current() = 0;
> contrib/test_decoding/sql/decoding_in_xact.sql:SELECT txid_current() = 0;
>
> src/test/modules/commit_ts/t/004_restart.pl: SELECT txid_current();
> src/test/modules/commit_ts/t/004_restart.pl: EXECUTE 'SELECT txid_current()';
> src/test/modules/commit_ts/t/004_restart.pl: SELECT txid_current();
> src/test/recovery/t/003_recovery_targets.pl: "SELECT pg_current_wal_lsn(), txid_current();");
> src/test/recovery/t/011_crash_recovery.pl:SELECT txid_current();
> src/test/recovery/t/011_crash_recovery.pl:cmp_ok($node->safe_psql('postgres', 'SELECT txid_current()'),
> src/test/regress/expected/alter_table.out: where transactionid = txid_current()::integer)
> src/test/regress/expected/alter_table.out: where transactionid = txid_current()::integer)
> src/test/regress/expected/hs_standby_functions.out:select txid_current();
> src/test/regress/expected/hs_standby_functions.out:ERROR: cannot execute txid_current() during recovery
> src/test/regress/expected/hs_standby_functions.out:select length(txid_current_snapshot()::text) >= 4;
> src/test/regress/expected/txid.out:select txid_current() >= txid_snapshot_xmin(txid_current_snapshot());
> src/test/regress/expected/txid.out:select txid_visible_in_snapshot(txid_current(), txid_current_snapshot());
> src/test/regress/expected/txid.out:-- test txid_current_if_assigned
> src/test/regress/expected/txid.out:SELECT txid_current_if_assigned() IS NULL;
> src/test/regress/expected/txid.out:SELECT txid_current() \gset
> src/test/regress/expected/txid.out:SELECT txid_current_if_assigned() IS NOT DISTINCT FROM BIGINT :'txid_current';
> src/test/regress/expected/txid.out:SELECT txid_current() AS committed \gset
> src/test/regress/expected/txid.out:SELECT txid_current() AS rolledback \gset
> src/test/regress/expected/txid.out:SELECT txid_current() AS inprogress \gset
> src/test/regress/expected/update.out:CREATE FUNCTION xid_current() RETURNS xid LANGUAGE SQL AS $$SELECT (txid_current() % ((1::int8<<32)))::text::xid;$$;
> src/test/regress/sql/alter_table.sql: where transactionid = txid_current()::integer)
> src/test/regress/sql/alter_table.sql: where transactionid = txid_current()::integer)
> src/test/regress/sql/hs_standby_functions.sql:select txid_current();
> src/test/regress/sql/hs_standby_functions.sql:select length(txid_current_snapshot()::text) >= 4;
> src/test/regress/sql/txid.sql:select txid_current() >= txid_snapshot_xmin(txid_current_snapshot());
> src/test/regress/sql/txid.sql:select txid_visible_in_snapshot(txid_current(), txid_current_snapshot());
> src/test/regress/sql/txid.sql:-- test txid_current_if_assigned
> src/test/regress/sql/txid.sql:SELECT txid_current_if_assigned() IS NULL;
> src/test/regress/sql/txid.sql:SELECT txid_current() \gset
> src/test/regress/sql/txid.sql:SELECT txid_current_if_assigned() IS NOT DISTINCT FROM BIGINT :'txid_current';
> src/test/regress/sql/txid.sql:SELECT txid_current() AS committed \gset
> src/test/regress/sql/txid.sql:SELECT txid_current() AS rolledback \gset
> src/test/regress/sql/txid.sql:SELECT txid_current() AS inprogress \gset
> src/test/regress/sql/update.sql:CREATE FUNCTION xid_current() RETURNS xid LANGUAGE SQL AS $$SELECT (txid_current() % ((1::int8<<32)))::text::xid;$$;

A reasonable argument could be made for treating txid_current as the preferred form, and xid8_current merely as a synonym, but then I can't make sense of the change your patch makes to the docs:

+ <para>
+ In releases of <productname>PostgreSQL</productname> before 13 there was
+ no <type>xid8</type> type, so variants of these functions were provided
+ that used <type>bigint</type>. The older functions with
+ <literal>txid</literal>
+ in the name are still supported for backward compatibility, but may be
+ removed from a future release. The <type>bigint</type> variants are shown
+ in <xref linkend="functions-txid-snapshot"/>.
+ </para>

which looks like a txid deprecation warning to me.

Am I reading all this wrong? If I'm reading this right, then FYI there are similar s/txid_(.*)/xid8_$1/g changes to be made that I didn't bother listing here by name.

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