Hello, Antonin!
PART 1:
I started rebasing the MVCC-safe version on top of the multi-snapshot
version and realized it becomes complex.
But, what's really bad about MVCC-unsafety is the ability to access
*incorrect* data and break some logic (or even constraints).
If we may *prevent* such data access with some kind of error (which is
going to be very infrequent) - I don't see any sense to achieve true
MVCC-safety.
I remembered a way it works with indcheckxmin for indexes. And made
something similar for pg_class: it records the rewriting transaction XID
and causes the executor to raise an error if a transaction with an older
snapshot attempts to access the rewritten relation.
For the normal case - check is never executed, no performance regression
here. Also, the flag is automatically cleared by VACUUM once the
transaction ID is frozen.
It also "fixes" ALTER TABLE, not only REPACK concurrently.
Attached patch contains more details (some in the commit message).
PART 2:
I have continued working with stress tests. This time I added your WIP
patch to fix the LR\CLOG race.
I made the following configs:
1) just REPACK CONCURRENTLY - ok
2) + relcheckxmin (see PART1) - ok
3) + worker - ok
4) + multiple snapshots - broken in multiple ways.
You may see example of run here -
https://cirrus-ci.com/build/6359048020295680
Some examples:
1) 'pgbench: error: client 11 script 0 aborted in command 20 query 0:
ERROR: could not read blocks 0..0 in file "base/5/16414": read only 0 of
8192 bytes
2) at /home/postgres/postgres/contrib/amcheck/t/008_repack_concurrently.pl
line 51.
[15:36:37.204] # 'pgbench: error: client 5 script 0
aborted in command 28 query 0: ERROR: division by zero
3) 'pgbench: error: client 12 script 0 aborted in command 6 query 0:
ERROR: cache lookup failed for relation 17400
Subject: [PATCH] stress tests for repack concurrently
---
Index: contrib/amcheck/meson.build
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/contrib/amcheck/meson.build b/contrib/amcheck/meson.build
--- a/contrib/amcheck/meson.build (revision 26b7a7cbedd8c0848c1056ef820bed2ddb5f522c)
+++ b/contrib/amcheck/meson.build (revision 14bf8dbc0ac0d52a6121be2484f652fc0c04732d)
@@ -50,6 +50,8 @@
't/004_verify_nbtree_unique.pl',
't/005_pitr.pl',
't/006_verify_gin.pl',
+ 't/007_repack_concurrently.pl',
+ 't/008_repack_concurrently.pl',
],
},
}
Index: contrib/amcheck/t/007_repack_concurrently.pl
===================================================================
diff --git a/contrib/amcheck/t/007_repack_concurrently.pl b/contrib/amcheck/t/007_repack_concurrently.pl
new file mode 100644
--- /dev/null (revision 14bf8dbc0ac0d52a6121be2484f652fc0c04732d)
+++ b/contrib/amcheck/t/007_repack_concurrently.pl (revision 14bf8dbc0ac0d52a6121be2484f652fc0c04732d)
@@ -0,0 +1,111 @@
+
+# Copyright (c) 2021-2025, PostgreSQL Global Development Group
+
+# Test REPACK CONCURRENTLY with concurrent modifications
+use strict;
+use warnings FATAL => 'all';
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+
+use Test::More;
+
+my $node;
+
+#
+# Test set-up
+#
+$node = PostgreSQL::Test::Cluster->new('CIC_test');
+$node->init;
+$node->append_conf('postgresql.conf',
+ 'lock_timeout = ' . (1000 * $PostgreSQL::Test::Utils::timeout_default));
+$node->append_conf(
+ 'postgresql.conf', qq(
+wal_level = logical
+max_worker_processes = 32
+));
+
+my $n=1000;
+my $no_hot = int(rand(2));
+
+$node->start;
+$node->safe_psql('postgres', q(CREATE TABLE tbl(i int PRIMARY KEY, j int)));
+
+if ($no_hot)
+{
+ $node->safe_psql('postgres', q(CREATE INDEX test_idx ON tbl(j);));
+}
+else
+{
+ $node->safe_psql('postgres', q(CREATE INDEX test_idx ON tbl(i);));
+}
+
+
+# Load amcheck
+$node->safe_psql('postgres', q(CREATE EXTENSION amcheck));
+
+# Insert $n rows into tbl
+$node->safe_psql('postgres', qq(
+ INSERT INTO tbl SELECT i, i FROM generate_series(1,$n) i
+));
+
+my $sum = $node->safe_psql('postgres', q(
+ SELECT SUM(j) AS sum FROM tbl
+));
+
+
+$node->pgbench(
+'--no-vacuum --client=15 --jobs=4 --exit-on-abort --transactions=5000',
+0,
+[qr{actually processed}],
+[qr{^$}],
+'concurrent operations with REPACK CONCURRENTLY',
+{
+ 'concurrent_ops' => qq(
+ SELECT pg_try_advisory_lock(42)::integer AS gotlock \\gset
+ \\if :gotlock
+ REPACK (CONCURRENTLY) tbl USING INDEX tbl_pkey;
+ SELECT bt_index_parent_check('tbl_pkey', heapallindexed => true);
+ SELECT bt_index_parent_check('test_idx', heapallindexed => true);
+ \\sleep 10 ms
+
+ REPACK (CONCURRENTLY) tbl USING INDEX test_idx;
+ SELECT bt_index_parent_check('tbl_pkey', heapallindexed => true);
+ SELECT bt_index_parent_check('test_idx', heapallindexed => true);
+ \\sleep 10 ms
+
+ REPACK (CONCURRENTLY) tbl;
+ SELECT bt_index_parent_check('tbl_pkey', heapallindexed => true);
+ SELECT bt_index_parent_check('test_idx', heapallindexed => true);
+ \\sleep 10 ms
+
+ SELECT pg_advisory_unlock(42);
+ \\else
+ \\set num_a random(1, $n)
+ \\set num_b random(1, $n)
+ \\set diff random(1, 10000)
+ BEGIN;
+ UPDATE tbl SET j = j + :diff WHERE i = :num_a;
+ \\sleep 1 ms
+ UPDATE tbl SET j = j - :diff WHERE i = :num_b;
+ \\sleep 1 ms
+ COMMIT;
+
+ BEGIN
+ --TRANSACTION ISOLATION LEVEL REPEATABLE READ
+ ;
+ SELECT 1;
+ \\sleep 1 ms
+ SELECT COALESCE(SUM(j), 0) AS sum FROM tbl \\gset p_
+ \\if :p_sum != $sum
+ COMMIT;
+ SELECT (:p_sum) / 0;
+ \\endif
+
+ COMMIT;
+ \\endif
+ )
+});
+
+$node->stop;
+done_testing();
Index: contrib/amcheck/t/008_repack_concurrently.pl
===================================================================
diff --git a/contrib/amcheck/t/008_repack_concurrently.pl b/contrib/amcheck/t/008_repack_concurrently.pl
new file mode 100644
--- /dev/null (revision 14bf8dbc0ac0d52a6121be2484f652fc0c04732d)
+++ b/contrib/amcheck/t/008_repack_concurrently.pl (revision 14bf8dbc0ac0d52a6121be2484f652fc0c04732d)
@@ -0,0 +1,102 @@
+
+# Copyright (c) 2021-2025, PostgreSQL Global Development Group
+
+# Test REPACK CONCURRENTLY with concurrent modifications
+use strict;
+use warnings FATAL => 'all';
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+
+use Test::More;
+
+my $node;
+
+#
+# Test set-up
+#
+$node = PostgreSQL::Test::Cluster->new('CIC_test');
+$node->init;
+$node->append_conf('postgresql.conf',
+ 'lock_timeout = ' . (1000 * $PostgreSQL::Test::Utils::timeout_default));
+$node->append_conf(
+ 'postgresql.conf', qq(
+wal_level = logical
+max_worker_processes = 32
+));
+
+my $no_hot = int(rand(2));
+
+$node->start;
+$node->safe_psql('postgres', q(CREATE TABLE tbl(i SERIAL PRIMARY KEY, j int)));
+if ($no_hot)
+{
+ $node->safe_psql('postgres', q(CREATE INDEX test_idx ON tbl(j);));
+}
+else
+{
+ $node->safe_psql('postgres', q(CREATE INDEX test_idx ON tbl(i);));
+}
+
+# Load amcheck
+$node->safe_psql('postgres', q(CREATE EXTENSION amcheck));
+
+my $sum = $node->safe_psql('postgres', q(
+ SELECT SUM(j) AS sum FROM tbl
+));
+
+$node->safe_psql('postgres', q(CREATE UNLOGGED SEQUENCE last_j START 1 INCREMENT 1;));
+
+
+$node->pgbench(
+'--no-vacuum --client=15 --jobs=4 --exit-on-abort --transactions=1000',
+0,
+[qr{actually processed}],
+[qr{^$}],
+'concurrent operations with REPACK CONCURRENTLY',
+{
+ 'concurrent_ops' => qq(
+ SELECT pg_try_advisory_lock(42)::integer AS gotlock \\gset
+ \\if :gotlock
+ REPACK (CONCURRENTLY) tbl USING INDEX tbl_pkey;
+ SELECT bt_index_parent_check('tbl_pkey', heapallindexed => true);
+ SELECT bt_index_parent_check('test_idx', heapallindexed => true);
+ \\sleep 10 ms
+
+ REPACK (CONCURRENTLY) tbl USING INDEX test_idx;
+ SELECT bt_index_parent_check('tbl_pkey', heapallindexed => true);
+ SELECT bt_index_parent_check('test_idx', heapallindexed => true);
+ \\sleep 10 ms
+
+ REPACK (CONCURRENTLY) tbl;
+ SELECT bt_index_parent_check('tbl_pkey', heapallindexed => true);
+ SELECT bt_index_parent_check('test_idx', heapallindexed => true);
+ \\sleep 10 ms
+
+ SELECT pg_advisory_unlock(42);
+ \\else
+ SELECT pg_advisory_lock(43);
+ BEGIN;
+ INSERT INTO tbl(j) VALUES (nextval('last_j')) RETURNING j \\gset p_
+ COMMIT;
+ SELECT pg_advisory_unlock(43);
+ \\sleep 1 ms
+
+ BEGIN
+ --TRANSACTION ISOLATION LEVEL REPEATABLE READ
+ ;
+ SELECT 1;
+ \\sleep 1 ms
+ SELECT COUNT(*) AS count FROM tbl WHERE j <= :p_j \\gset p_
+ \\if :p_count != :p_j
+ COMMIT;
+ SELECT (:p_count) / 0;
+ \\endif
+
+ COMMIT;
+ \\endif
+ )
+});
+
+$node->stop;
+done_testing();
Subject: [PATCH] Add `relcheckxmin` to track and enforce tuple visibility in `pg_class`
---
Index: doc/src/sgml/mvcc.sgml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/doc/src/sgml/mvcc.sgml b/doc/src/sgml/mvcc.sgml
--- a/doc/src/sgml/mvcc.sgml (revision 14bf8dbc0ac0d52a6121be2484f652fc0c04732d)
+++ b/doc/src/sgml/mvcc.sgml (revision e5311f14c463819fabe6c57fa74512f10c881508)
@@ -1833,22 +1833,30 @@
<title>Caveats</title>
<para>
- Some commands, currently only <link linkend="sql-truncate"><command>TRUNCATE</command></link>, the
- table-rewriting forms of <link linkend="sql-altertable"><command>ALTER
- TABLE</command></link> and <command>REPACK</command> with
- the <literal>CONCURRENTLY</literal> option, are not
- MVCC-safe. This means that after the truncation or rewrite commits, the
+ Some commands, currently only <link linkend="sql-truncate"><command>TRUNCATE</command></link>,
+ are not MVCC-safe. This means that after the truncation commits, the
table will appear empty to concurrent transactions, if they are using a
- snapshot taken before the command committed. This will only be an
- issue for a transaction that did not access the table in question
- before the command started — any transaction that has done so
- would hold at least an <literal>ACCESS SHARE</literal> table lock,
- which would block the truncating or rewriting command until that transaction completes.
- So these commands will not cause any apparent inconsistency in the
- table contents for successive queries on the target table, but they
+ snapshot taken before the command committed. This will only be an issue
+ for a transaction that did not access the table in question before the
+ command started — any transaction that has done so would hold at
+ least an <literal>ACCESS SHARE</literal> table lock, which would block
+ the <command>TRUNCATE</command> command until that transaction completes.
+ So <command>TRUNCATE</command> will not cause any apparent inconsistency
+ in the table contents for successive queries on the target table, but it
could cause visible inconsistency between the contents of the target
table and other tables in the database.
</para>
+
+ <para>
+ The table-rewriting forms of
+ <link linkend="sql-altertable"><command>ALTER TABLE</command></link>
+ and <link linkend="sql-repack"><command>REPACK</command></link>
+ with the <literal>CONCURRENTLY</literal> option are also not MVCC-safe.
+ If a transaction attempts to access a relation that was rewritten
+ after the transaction's snapshot was taken, an error will be raised.
+ As above, this will only be an issue for a transaction that did not
+ access the table before the rewriting command started.
+ </para>
<para>
Support for the Serializable transaction isolation level has not yet
Index: src/backend/catalog/heap.c
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
--- a/src/backend/catalog/heap.c (revision 14bf8dbc0ac0d52a6121be2484f652fc0c04732d)
+++ b/src/backend/catalog/heap.c (revision e5311f14c463819fabe6c57fa74512f10c881508)
@@ -953,6 +953,7 @@
values[Anum_pg_class_relrewrite - 1] = ObjectIdGetDatum(rd_rel->relrewrite);
values[Anum_pg_class_relfrozenxid - 1] = TransactionIdGetDatum(rd_rel->relfrozenxid);
values[Anum_pg_class_relminmxid - 1] = MultiXactIdGetDatum(rd_rel->relminmxid);
+ values[Anum_pg_class_relcheckxmin - 1] = TransactionIdGetDatum(rd_rel->relcheckxmin);
if (relacl != (Datum) 0)
values[Anum_pg_class_relacl - 1] = relacl;
else
@@ -1023,6 +1024,8 @@
/* relispartition is always set by updating this tuple later */
new_rel_reltup->relispartition = false;
+ new_rel_reltup->relcheckxmin = InvalidTransactionId;
+
/* fill rd_att's type ID with something sane even if reltype is zero */
new_rel_desc->rd_att->tdtypeid = new_type_oid ? new_type_oid : RECORDOID;
new_rel_desc->rd_att->tdtypmod = -1;
Index: src/backend/commands/cluster.c
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
--- a/src/backend/commands/cluster.c (revision 14bf8dbc0ac0d52a6121be2484f652fc0c04732d)
+++ b/src/backend/commands/cluster.c (revision e5311f14c463819fabe6c57fa74512f10c881508)
@@ -1031,7 +1031,7 @@
* rebuild the target's indexes and throw away the transient table.
*/
finish_heap_swap(tableOid, OIDNewHeap, is_system_catalog,
- swap_toast_by_content, false, true, true,
+ swap_toast_by_content, false, true, true, InvalidTransactionId,
frozenXid, cutoffMulti,
relpersistence);
}
@@ -1429,6 +1429,7 @@
swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class,
bool swap_toast_by_content,
bool is_internal,
+ TransactionId check_xmin,
TransactionId frozenXid,
MultiXactId cutoffMulti,
Oid *mapped_tables)
@@ -1611,6 +1612,9 @@
relform2->relallfrozen = swap_allfrozen;
}
+ relform1->relcheckxmin = check_xmin;
+ relform2->relcheckxmin = check_xmin;
+
/*
* Update the tuples in pg_class --- unless the target relation of the
* swap is pg_class itself. In that case, there is zero point in making
@@ -1688,6 +1692,7 @@
target_is_pg_class,
swap_toast_by_content,
is_internal,
+ InvalidTransactionId,
frozenXid,
cutoffMulti,
mapped_tables);
@@ -1791,6 +1796,7 @@
target_is_pg_class,
swap_toast_by_content,
is_internal,
+ check_xmin,
InvalidTransactionId,
InvalidMultiXactId,
mapped_tables);
@@ -1814,6 +1820,7 @@
bool check_constraints,
bool is_internal,
bool reindex,
+ TransactionId check_xmin,
TransactionId frozenXid,
MultiXactId cutoffMulti,
char newrelpersistence)
@@ -1835,7 +1842,7 @@
*/
swap_relation_files(OIDOldHeap, OIDNewHeap,
(OIDOldHeap == RelationRelationId),
- swap_toast_by_content, is_internal,
+ swap_toast_by_content, is_internal, check_xmin,
frozenXid, cutoffMulti, mapped_tables);
/*
@@ -3335,6 +3342,7 @@
false, /* swap_toast_by_content */
true,
InvalidTransactionId,
+ InvalidTransactionId,
InvalidMultiXactId,
mapped_tables);
@@ -3371,7 +3379,7 @@
finish_heap_swap(old_table_oid, new_table_oid,
is_system_catalog,
false, /* swap_toast_by_content */
- false, true, false,
+ false, true, false, GetCurrentTransactionId(),
frozenXid, cutoffMulti,
relpersistence);
}
Index: src/backend/commands/matview.c
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
--- a/src/backend/commands/matview.c (revision 14bf8dbc0ac0d52a6121be2484f652fc0c04732d)
+++ b/src/backend/commands/matview.c (revision e5311f14c463819fabe6c57fa74512f10c881508)
@@ -892,7 +892,7 @@
static void
refresh_by_heap_swap(Oid matviewOid, Oid OIDNewHeap, char relpersistence)
{
- finish_heap_swap(matviewOid, OIDNewHeap, false, false, true, true, true,
+ finish_heap_swap(matviewOid, OIDNewHeap, false, false, true, true, true, InvalidTransactionId,
RecentXmin, ReadNextMultiXactId(), relpersistence);
}
Index: src/backend/commands/tablecmds.c
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
--- a/src/backend/commands/tablecmds.c (revision 14bf8dbc0ac0d52a6121be2484f652fc0c04732d)
+++ b/src/backend/commands/tablecmds.c (revision e5311f14c463819fabe6c57fa74512f10c881508)
@@ -6026,6 +6026,7 @@
false, false, true,
!OidIsValid(tab->newTableSpace),
true,
+ GetCurrentTransactionId(),
RecentXmin,
ReadNextMultiXactId(),
persistence);
Index: src/backend/commands/vacuum.c
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
--- a/src/backend/commands/vacuum.c (revision 14bf8dbc0ac0d52a6121be2484f652fc0c04732d)
+++ b/src/backend/commands/vacuum.c (revision e5311f14c463819fabe6c57fa74512f10c881508)
@@ -1534,6 +1534,9 @@
if (update)
{
pgcform->relfrozenxid = frozenxid;
+ /* Clear relcheckxmin if frozenxid is higher, now it is safe */
+ if (TransactionIdPrecedes(pgcform->relcheckxmin, pgcform->relfrozenxid))
+ pgcform->relcheckxmin = InvalidTransactionId;
dirty = true;
if (frozenxid_updated)
*frozenxid_updated = true;
Index: src/backend/executor/execUtils.c
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
--- a/src/backend/executor/execUtils.c (revision 14bf8dbc0ac0d52a6121be2484f652fc0c04732d)
+++ b/src/backend/executor/execUtils.c (revision e5311f14c463819fabe6c57fa74512f10c881508)
@@ -865,6 +865,24 @@
estate->es_relations[rti - 1] = rel;
}
+ if (unlikely(TransactionIdIsValid(rel->rd_check_xmin)))
+ {
+ /* We know it is committed, just need to be sure it is visible for current exec */
+ TransactionId xmin = rel->rd_check_xmin;
+ /* Fast check first */
+ if (unlikely(!TransactionIdIsCurrentTransactionId(xmin) &&
+ !TransactionIdPrecedes(xmin, TransactionXmin)))
+ {
+ Snapshot snapshot = GetActiveSnapshot();
+ /* Test remaining rules */
+ if (XidInMVCCSnapshot(xmin, snapshot))
+ ereport(ERROR,
+ errcode(ERRCODE_RELATION_UNAVAILABLE_FOR_CURRENT_TRANSACTION),
+ errmsg("relation \"%s\" cannot be accessed in current transaction",
+ RelationGetRelationName(rel)));
+ }
+ }
+
return rel;
}
Index: src/backend/utils/cache/relcache.c
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
--- a/src/backend/utils/cache/relcache.c (revision 14bf8dbc0ac0d52a6121be2484f652fc0c04732d)
+++ b/src/backend/utils/cache/relcache.c (revision e5311f14c463819fabe6c57fa74512f10c881508)
@@ -1203,6 +1203,8 @@
*/
RelationBuildTupleDesc(relation);
+ relation->rd_check_xmin = relp->relcheckxmin;
+
/* foreign key data is not loaded till asked for */
relation->rd_fkeylist = NIL;
relation->rd_fkeyvalid = false;
Index: src/backend/utils/errcodes.txt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/backend/utils/errcodes.txt b/src/backend/utils/errcodes.txt
--- a/src/backend/utils/errcodes.txt (revision 14bf8dbc0ac0d52a6121be2484f652fc0c04732d)
+++ b/src/backend/utils/errcodes.txt (revision e5311f14c463819fabe6c57fa74512f10c881508)
@@ -259,6 +259,7 @@
25P02 E ERRCODE_IN_FAILED_SQL_TRANSACTION in_failed_sql_transaction
25P03 E ERRCODE_IDLE_IN_TRANSACTION_SESSION_TIMEOUT idle_in_transaction_session_timeout
25P04 E ERRCODE_TRANSACTION_TIMEOUT transaction_timeout
+25P05 E ERRCODE_RELATION_UNAVAILABLE_FOR_CURRENT_TRANSACTION relation_unavailable_for_current_transaction
Section: Class 26 - Invalid SQL Statement Name
Index: src/include/catalog/catversion.h
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
--- a/src/include/catalog/catversion.h (revision 14bf8dbc0ac0d52a6121be2484f652fc0c04732d)
+++ b/src/include/catalog/catversion.h (revision e5311f14c463819fabe6c57fa74512f10c881508)
@@ -57,6 +57,6 @@
*/
/* yyyymmddN */
-#define CATALOG_VERSION_NO 202601221
+#define CATALOG_VERSION_NO 202601242
#endif
Index: src/include/catalog/pg_class.h
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/include/catalog/pg_class.h b/src/include/catalog/pg_class.h
--- a/src/include/catalog/pg_class.h (revision 14bf8dbc0ac0d52a6121be2484f652fc0c04732d)
+++ b/src/include/catalog/pg_class.h (revision e5311f14c463819fabe6c57fa74512f10c881508)
@@ -131,6 +131,9 @@
/* all multixacts in this rel are >= this; it is really a MultiXactId */
TransactionId relminmxid BKI_DEFAULT(1); /* FirstMultiXactId */
+ /* need to check to prevent MVCC violations */
+ TransactionId relcheckxmin BKI_DEFAULT(0); /* InvalidTransactionId */
+
#ifdef CATALOG_VARLEN /* variable-length fields start here */
/* NOTE: These fields are not present in a relcache entry's rd_rel field. */
/* access permissions */
@@ -146,7 +149,7 @@
/* Size of fixed part of pg_class tuples, not counting var-length fields */
#define CLASS_TUPLE_SIZE \
- (offsetof(FormData_pg_class,relminmxid) + sizeof(TransactionId))
+ (offsetof(FormData_pg_class,relcheckxmin) + sizeof(TransactionId))
/* ----------------
* Form_pg_class corresponds to a pointer to a tuple with
Index: src/include/commands/cluster.h
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/include/commands/cluster.h b/src/include/commands/cluster.h
--- a/src/include/commands/cluster.h (revision 14bf8dbc0ac0d52a6121be2484f652fc0c04732d)
+++ b/src/include/commands/cluster.h (revision e5311f14c463819fabe6c57fa74512f10c881508)
@@ -130,6 +130,7 @@
bool check_constraints,
bool is_internal,
bool reindex,
+ TransactionId check_xmin,
TransactionId frozenXid,
MultiXactId cutoffMulti,
char newrelpersistence);
Index: src/include/utils/rel.h
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
--- a/src/include/utils/rel.h (revision 14bf8dbc0ac0d52a6121be2484f652fc0c04732d)
+++ b/src/include/utils/rel.h (revision e5311f14c463819fabe6c57fa74512f10c881508)
@@ -143,6 +143,11 @@
*/
TransactionId rd_partdesc_nodetached_xmin;
+ /*
+ * Used at execution phase to prevent violation of MVCC rules.
+ */
+ TransactionId rd_check_xmin;
+
/* data managed by RelationGetPartitionQual: */
List *rd_partcheck; /* partition CHECK quals */
bool rd_partcheckvalid; /* true if list has been computed */
Index: src/test/modules/injection_points/Makefile
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile
--- a/src/test/modules/injection_points/Makefile (revision 14bf8dbc0ac0d52a6121be2484f652fc0c04732d)
+++ b/src/test/modules/injection_points/Makefile (revision e5311f14c463819fabe6c57fa74512f10c881508)
@@ -16,6 +16,7 @@
inplace \
repack \
repack_toast \
+ repack_mvcc \
syscache-update-pruned \
heap_lock_update
ISOLATION_OPTS = --temp-config $(top_srcdir)/src/test/modules/injection_points/logical.conf
Index: src/test/modules/injection_points/expected/repack_mvcc.out
===================================================================
diff --git a/src/test/modules/injection_points/expected/repack_mvcc.out b/src/test/modules/injection_points/expected/repack_mvcc.out
new file mode 100644
--- /dev/null (revision e5311f14c463819fabe6c57fa74512f10c881508)
+++ b/src/test/modules/injection_points/expected/repack_mvcc.out (revision e5311f14c463819fabe6c57fa74512f10c881508)
@@ -0,0 +1,19 @@
+Parsed test spec with 2 sessions
+
+starting permutation: begin repack check
+step begin:
+ BEGIN TRANSACTION ISOLATION LEVEL REPEATABLE READ;
+ SELECT 1;
+
+?column?
+--------
+ 1
+(1 row)
+
+step repack:
+ REPACK (CONCURRENTLY) repack_test;
+
+step check:
+ SELECT * FROM repack_test;
+
+ERROR: relation "repack_test" cannot be accessed in current transaction
Index: src/test/modules/injection_points/meson.build
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build
--- a/src/test/modules/injection_points/meson.build (revision 14bf8dbc0ac0d52a6121be2484f652fc0c04732d)
+++ b/src/test/modules/injection_points/meson.build (revision e5311f14c463819fabe6c57fa74512f10c881508)
@@ -47,6 +47,7 @@
'inplace',
'repack',
'repack_toast',
+ 'repack_mvcc',
'syscache-update-pruned',
'heap_lock_update',
],
Index: src/test/modules/injection_points/specs/repack_mvcc.spec
===================================================================
diff --git a/src/test/modules/injection_points/specs/repack_mvcc.spec b/src/test/modules/injection_points/specs/repack_mvcc.spec
new file mode 100644
--- /dev/null (revision e5311f14c463819fabe6c57fa74512f10c881508)
+++ b/src/test/modules/injection_points/specs/repack_mvcc.spec (revision e5311f14c463819fabe6c57fa74512f10c881508)
@@ -0,0 +1,38 @@
+# REPACK (CONCURRENTLY);
+#
+# Test handling of preventing access to non-mvcc safe data.
+setup
+{
+ CREATE EXTENSION injection_points;
+
+ CREATE TABLE repack_test(i int PRIMARY KEY);
+ INSERT INTO repack_test(i) VALUES (1), (2), (3);
+}
+
+teardown
+{
+ DROP TABLE repack_test;
+ DROP EXTENSION injection_points;
+}
+
+session s1
+step repack
+{
+ REPACK (CONCURRENTLY) repack_test;
+}
+
+session s2
+step begin
+{
+ BEGIN TRANSACTION ISOLATION LEVEL REPEATABLE READ;
+ SELECT 1;
+}
+step check
+{
+ SELECT * FROM repack_test;
+}
+
+permutation
+ begin
+ repack
+ check
