From 4aaddf2f009821e29aea3735e44332ad9ca47aaa Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Thu, 24 Nov 2016 15:39:55 -0300 Subject: [PATCH] Fix commit_ts for FrozenXid and BootstrapXid Previously, requesting commit timestamp for transactions FrozenTransactionId and BootstrapTransactionId resulted in an error. But since those values can validly appear in committed tuples' Xmin, this behavior is unhelpful and error prone: each caller would have to special-case those values before requesting timestamp data for an Xid. We already have a perfectly good interface for returning "the Xid you requested is too old for us to have commit TS data for it", so let's use that instead. Backpatch to 9.5, where commit timestamps appeared. Author: Craig Ringer Discussion: https://www.postgresql.org/message-id/CAMsr+YFM5Q=+ry3mKvWEqRTxrB0iU3qUSRnS28nz6FJYtBwhJg@mail.gmail.com --- src/backend/access/transam/commit_ts.c | 11 +++++++++-- .../commit_ts/expected/commit_timestamp.out | 12 ++++++++++-- .../commit_ts/expected/commit_timestamp_1.out | 12 ++++++++++-- src/test/modules/commit_ts/t/004_restart.pl | 14 ++++---------- 4 files changed, 33 insertions(+), 16 deletions(-) diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c index 7746578825..a5b270cd20 100644 --- a/src/backend/access/transam/commit_ts.c +++ b/src/backend/access/transam/commit_ts.c @@ -289,11 +289,18 @@ TransactionIdGetCommitTsData(TransactionId xid, TimestampTz *ts, TransactionId oldestCommitTsXid; TransactionId newestCommitTsXid; - /* error if the given Xid doesn't normally commit */ - if (!TransactionIdIsNormal(xid)) + if (!TransactionIdIsValid(xid)) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("cannot retrieve commit timestamp for transaction %u", xid))); + else if (!TransactionIdIsNormal(xid)) + { + /* frozen and bootstrap xids are always committed far in the past */ + *ts = 0; + if (nodeid) + *nodeid = 0; + return false; + } LWLockAcquire(CommitTsLock, LW_SHARED); diff --git a/src/test/modules/commit_ts/expected/commit_timestamp.out b/src/test/modules/commit_ts/expected/commit_timestamp.out index 99f3322c42..5b7783b58f 100644 --- a/src/test/modules/commit_ts/expected/commit_timestamp.out +++ b/src/test/modules/commit_ts/expected/commit_timestamp.out @@ -28,9 +28,17 @@ DROP TABLE committs_test; SELECT pg_xact_commit_timestamp('0'::xid); ERROR: cannot retrieve commit timestamp for transaction 0 SELECT pg_xact_commit_timestamp('1'::xid); -ERROR: cannot retrieve commit timestamp for transaction 1 + pg_xact_commit_timestamp +-------------------------- + +(1 row) + SELECT pg_xact_commit_timestamp('2'::xid); -ERROR: cannot retrieve commit timestamp for transaction 2 + pg_xact_commit_timestamp +-------------------------- + +(1 row) + SELECT x.xid::text::bigint > 0, x.timestamp > '-infinity'::timestamptz, x.timestamp <= now() FROM pg_last_committed_xact() x; ?column? | ?column? | ?column? ----------+----------+---------- diff --git a/src/test/modules/commit_ts/expected/commit_timestamp_1.out b/src/test/modules/commit_ts/expected/commit_timestamp_1.out index 2f1f41d209..c10b0abc2b 100644 --- a/src/test/modules/commit_ts/expected/commit_timestamp_1.out +++ b/src/test/modules/commit_ts/expected/commit_timestamp_1.out @@ -23,9 +23,17 @@ DROP TABLE committs_test; SELECT pg_xact_commit_timestamp('0'::xid); ERROR: cannot retrieve commit timestamp for transaction 0 SELECT pg_xact_commit_timestamp('1'::xid); -ERROR: cannot retrieve commit timestamp for transaction 1 + pg_xact_commit_timestamp +-------------------------- + +(1 row) + SELECT pg_xact_commit_timestamp('2'::xid); -ERROR: cannot retrieve commit timestamp for transaction 2 + pg_xact_commit_timestamp +-------------------------- + +(1 row) + SELECT x.xid::text::bigint > 0, x.timestamp > '-infinity'::timestamptz, x.timestamp <= now() FROM pg_last_committed_xact() x; ERROR: could not get commit timestamp data HINT: Make sure the configuration parameter "track_commit_timestamp" is set. diff --git a/src/test/modules/commit_ts/t/004_restart.pl b/src/test/modules/commit_ts/t/004_restart.pl index 900e9b7970..32be07c741 100644 --- a/src/test/modules/commit_ts/t/004_restart.pl +++ b/src/test/modules/commit_ts/t/004_restart.pl @@ -25,19 +25,13 @@ like( ($ret, $stdout, $stderr) = $node_master->psql('postgres', qq[SELECT pg_xact_commit_timestamp('1');]); -is($ret, 3, 'getting ts of BootstrapTransactionId reports error'); -like( - $stderr, - qr/cannot retrieve commit timestamp for transaction/, - 'expected error from BootstrapTransactionId'); +is($ret, 0, 'getting ts of BootstrapTransactionId succeeds'); +is($stdout, '', 'timestamp of BootstrapTransactionId is null'); ($ret, $stdout, $stderr) = $node_master->psql('postgres', qq[SELECT pg_xact_commit_timestamp('2');]); -is($ret, 3, 'getting ts of FrozenTransactionId reports error'); -like( - $stderr, - qr/cannot retrieve commit timestamp for transaction/, - 'expected error from FrozenTransactionId'); +is($ret, 0, 'getting ts of FrozenTransactionId succeeds'); +is($stdout, '', 'timestamp of FrozenTransactionId is null'); # Since FirstNormalTransactionId will've occurred during initdb, long before we # enabled commit timestamps, it'll be null since we have no cts data for it but -- 2.40.0