> 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