> On Apr 1, 2020, at 8:21 PM, Thomas Munro <thomas.mu...@gmail.com> wrote:
> 
> On Sat, Mar 21, 2020 at 11:14 AM Thomas Munro <thomas.mu...@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 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





Reply via email to