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

> On Apr 1, 2020, at 8:21 PM, Thomas Munro <thomas.munro@gmail.com> wrote:
>
> On Sat, Mar 21, 2020 at 11:14 AM Thomas Munro <thomas.munro@gmail.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
withthe 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
thismany in the regression test suite may lead other developers to follow that lead and use txid_current() in newly
developedcode.  ("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
madethat I didn't bother listing here by name. 

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