diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c index 9a199dd9bfb..860e2aecbe9 100644 --- a/src/backend/commands/repack.c +++ b/src/backend/commands/repack.c @@ -118,6 +118,9 @@ typedef struct ChangeContext ScanKey cc_ident_key; int cc_ident_key_nentries; + /* The latest column we need to deform to have the tuple identity */ + AttrNumber cc_last_key_attno; + /* Sequential number of the file containing the changes. */ int cc_file_seq; } ChangeContext; @@ -182,6 +185,9 @@ static void adjust_toast_pointers(Relation relation, TupleTableSlot *dest, static bool find_target_tuple(Relation rel, ChangeContext *chgcxt, TupleTableSlot *locator, TupleTableSlot *retrieved); +static bool identity_key_equal(ChangeContext *chgcxt, + TupleTableSlot *locator, + TupleTableSlot *candidate); static void process_concurrent_changes(XLogRecPtr end_of_wal, ChangeContext *chgcxt, bool done); @@ -2807,7 +2813,7 @@ find_target_tuple(Relation rel, ChangeContext *chgcxt, TupleTableSlot *locator, { Form_pg_index idx = chgcxt->cc_ident_index->rd_index; IndexScanDesc scan; - bool retval; + bool retval = false; /* * Scan key is passed by caller, so it does not have to be constructed @@ -2829,12 +2835,61 @@ find_target_tuple(Relation rel, ChangeContext *chgcxt, TupleTableSlot *locator, scan = index_beginscan(rel, chgcxt->cc_ident_index, GetActiveSnapshot(), NULL, chgcxt->cc_ident_key_nentries, 0, 0); index_rescan(scan, chgcxt->cc_ident_key, chgcxt->cc_ident_key_nentries, NULL, 0); - retval = index_getnext_slot(scan, ForwardScanDirection, retrieved); + while (index_getnext_slot(scan, ForwardScanDirection, retrieved)) + { + /* Be wary of temporal constraints */ + if (scan->xs_recheck && !identity_key_equal(chgcxt, locator, retrieved)) + { + CHECK_FOR_INTERRUPTS(); + continue; + } + + retval = true; + break; + } index_endscan(scan); return retval; } +/* + * Check whether the candidate tuple matches the locator tuple on all replica + * identity key columns, using the same equality operators as the identity + * index scan. The locator tuple has already been loaded into cc_ident_key. + * + * This is needed to filter lossy index matches, such as GiST multirange scans + * used for temporal constraints. + */ +static bool +identity_key_equal(ChangeContext *chgcxt, TupleTableSlot *locator, + TupleTableSlot *candidate) +{ + slot_getsomeattrs(locator, chgcxt->cc_last_key_attno); + slot_getsomeattrs(candidate, chgcxt->cc_last_key_attno); + + for (int i = 0; i < chgcxt->cc_ident_key_nentries; i++) + { + ScanKey entry = &chgcxt->cc_ident_key[i]; + AttrNumber attno = chgcxt->cc_ident_index->rd_index->indkey.values[i]; + + Assert(attno > 0); + + if (locator->tts_isnull[attno - 1] != candidate->tts_isnull[attno - 1]) + return false; + + if (locator->tts_isnull[attno - 1]) + continue; + + if (!DatumGetBool(FunctionCall2Coll(&entry->sk_func, + entry->sk_collation, + candidate->tts_values[attno - 1], + entry->sk_argument))) + return false; + } + + return true; +} + /* * Decode and apply concurrent changes, up to (and including) the record whose * LSN is 'end_of_wal'. @@ -2944,28 +2999,46 @@ initialize_change_context(ChangeContext *chgcxt, opcintype, opno, opcode; + StrategyNumber eq_strategy; entry = &chgcxt->cc_ident_key[i]; opfamily = chgcxt->cc_ident_index->rd_opfamily[i]; opcintype = chgcxt->cc_ident_index->rd_opcintype[i]; + eq_strategy = IndexAmTranslateCompareType(COMPARE_EQ, + chgcxt->cc_ident_index->rd_rel->relam, + opfamily, false); + if (eq_strategy == InvalidStrategy) + elog(ERROR, "could not find equality strategy for index operator family %u for type %u", + opfamily, opcintype); opno = get_opfamily_member(opfamily, opcintype, opcintype, - BTEqualStrategyNumber); + eq_strategy); if (!OidIsValid(opno)) - elog(ERROR, "failed to find = operator for type %u", opcintype); + elog(ERROR, "missing operator %d(%u,%u) in opfamily %u", + eq_strategy, opcintype, opcintype, opfamily); opcode = get_opcode(opno); if (!OidIsValid(opcode)) - elog(ERROR, "failed to find = operator for operator %u", opno); + elog(ERROR, "missing oprcode for operator %u", opno); /* Initialize everything but argument. */ ScanKeyInit(entry, i + 1, - BTEqualStrategyNumber, opcode, + eq_strategy, opcode, (Datum) 0); entry->sk_collation = chgcxt->cc_ident_index->rd_indcollation[i]; } } + /* Determine the last column we must deform to read the identity */ + chgcxt->cc_last_key_attno = InvalidAttrNumber; + for (int i = 0; i < chgcxt->cc_ident_key_nentries; i++) + { + AttrNumber attno = chgcxt->cc_ident_index->rd_index->indkey.values[i]; + + Assert(attno > 0); + chgcxt->cc_last_key_attno = Max(chgcxt->cc_last_key_attno, attno); + } + chgcxt->cc_file_seq = WORKER_FILE_SNAPSHOT + 1; } diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile index f057d143d1a..c01d2fb095c 100644 --- a/src/test/modules/injection_points/Makefile +++ b/src/test/modules/injection_points/Makefile @@ -15,6 +15,8 @@ REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress ISOLATION = basic \ inplace \ repack \ + repack_temporal \ + repack_temporal_multirange \ repack_toast \ syscache-update-pruned \ heap_lock_update diff --git a/src/test/modules/injection_points/expected/repack_temporal.out b/src/test/modules/injection_points/expected/repack_temporal.out new file mode 100644 index 00000000000..ea02089c79a --- /dev/null +++ b/src/test/modules/injection_points/expected/repack_temporal.out @@ -0,0 +1,68 @@ +Parsed test spec with 2 sessions + +starting permutation: wait_before_lock update_target check_after_update wakeup_before_lock check_after_repack +injection_points_attach +----------------------- + +(1 row) + +step wait_before_lock: + REPACK (CONCURRENTLY) repack_temporal USING INDEX rt_pkey; + +step update_target: + UPDATE repack_temporal + SET label = 'updated' + WHERE id = '[2,3)' AND valid_at = '[2000-01-10,2000-01-20)'; + +step check_after_update: + INSERT INTO relfilenodes(node) + SELECT relfilenode FROM pg_class WHERE relname = 'repack_temporal'; + + -- Expect 2 rows + SELECT id, valid_at, label + FROM repack_temporal + ORDER BY id, valid_at, label; + +id |valid_at |label +------+-----------------------+------- +[1,10)|[01-01-2000,02-01-2000)|other +[2,3) |[01-10-2000,01-20-2000)|updated +(2 rows) + +step wakeup_before_lock: + SELECT injection_points_wakeup('repack-concurrently-before-lock'); + +injection_points_wakeup +----------------------- + +(1 row) + +step wait_before_lock: <... completed> +step check_after_repack: + INSERT INTO relfilenodes(node) + SELECT relfilenode FROM pg_class WHERE relname = 'repack_temporal'; + + -- Expect 2, proving that repack has rewritten the table + SELECT count(DISTINCT node) FROM relfilenodes; + + -- Expect 2 rows + SELECT id, valid_at, label + FROM repack_temporal + ORDER BY id, valid_at, label; + +count +----- + 2 +(1 row) + +id |valid_at |label +------+-----------------------+------- +[1,10)|[01-01-2000,02-01-2000)|other +[2,3) |[01-10-2000,01-20-2000)|updated +(2 rows) + +injection_points_detach +----------------------- + +(1 row) + diff --git a/src/test/modules/injection_points/expected/repack_temporal_multirange.out b/src/test/modules/injection_points/expected/repack_temporal_multirange.out new file mode 100644 index 00000000000..a2e015c3b85 --- /dev/null +++ b/src/test/modules/injection_points/expected/repack_temporal_multirange.out @@ -0,0 +1,74 @@ +Parsed test spec with 2 sessions + +starting permutation: wait_before_lock update_target check_after_update wakeup_before_lock final_check +injection_points_attach +----------------------- + +(1 row) + +step wait_before_lock: + REPACK (CONCURRENTLY) repack_temporal_multirange + USING INDEX rtm_pkey; + +step update_target: + UPDATE repack_temporal_multirange + SET label = 'updated' + WHERE id = int4multirange(int4range(1, 7)) + AND valid_at = datemultirange(daterange('2000-01-01', '2000-02-01')); + +step check_after_update: + INSERT INTO relfilenodes(node) + SELECT relfilenode + FROM pg_class + WHERE relname = 'repack_temporal_multirange'; + + -- Expect 2 rows + SELECT id, valid_at, label + FROM repack_temporal_multirange + ORDER BY id, valid_at, label; + +id |valid_at |label +-------------+-------------------------+------- +{[1,3),[5,7)}|{[01-01-2000,02-01-2000)}|other +{[1,7)} |{[01-01-2000,02-01-2000)}|updated +(2 rows) + +step wakeup_before_lock: + SELECT injection_points_wakeup('repack-concurrently-before-lock'); + +injection_points_wakeup +----------------------- + +(1 row) + +step wait_before_lock: <... completed> +step final_check: + INSERT INTO relfilenodes(node) + SELECT relfilenode + FROM pg_class + WHERE relname = 'repack_temporal_multirange'; + + -- Expect 2, proving that repack has rewritten the table + SELECT count(DISTINCT node) FROM relfilenodes; + + -- Expect 2 rows + SELECT id, valid_at, label + FROM repack_temporal_multirange + ORDER BY id, valid_at, label; + +count +----- + 2 +(1 row) + +id |valid_at |label +-------------+-------------------------+------- +{[1,3),[5,7)}|{[01-01-2000,02-01-2000)}|other +{[1,7)} |{[01-01-2000,02-01-2000)}|updated +(2 rows) + +injection_points_detach +----------------------- + +(1 row) + diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build index fb1418e2caa..59dba1cb023 100644 --- a/src/test/modules/injection_points/meson.build +++ b/src/test/modules/injection_points/meson.build @@ -46,6 +46,8 @@ tests += { 'basic', 'inplace', 'repack', + 'repack_temporal', + 'repack_temporal_multirange', 'repack_toast', 'syscache-update-pruned', 'heap_lock_update', diff --git a/src/test/modules/injection_points/specs/repack_temporal.spec b/src/test/modules/injection_points/specs/repack_temporal.spec new file mode 100644 index 00000000000..c9fd3f61a84 --- /dev/null +++ b/src/test/modules/injection_points/specs/repack_temporal.spec @@ -0,0 +1,90 @@ +# REPACK (CONCURRENTLY) on a temporal replica identity index. +# +# The table's replica identity is a GiST index created by a temporal primary +# key. A concurrent UPDATE changes a non-key column of one row, while another +# row overlaps it on all indexed columns. Replay must still find the exact +# target row. +setup +{ + CREATE EXTENSION injection_points; + + CREATE TABLE repack_temporal ( + id int4range, + valid_at daterange, + label text, + CONSTRAINT rt_pkey PRIMARY KEY (id, valid_at WITHOUT OVERLAPS) + ); + + ALTER TABLE repack_temporal REPLICA IDENTITY USING INDEX rt_pkey; + + INSERT INTO repack_temporal(id, valid_at, label) + VALUES + ('[1,10)', '[2000-01-01,2000-02-01)', 'other'), + ('[2,3)', '[2000-01-10,2000-01-20)', 'target'); + + CREATE TABLE relfilenodes(node oid); +} + +teardown +{ + DROP TABLE repack_temporal; + DROP EXTENSION injection_points; + DROP TABLE relfilenodes; +} + +session s1 +setup +{ + SELECT injection_points_set_local(); + SELECT injection_points_attach('repack-concurrently-before-lock', 'wait'); +} +step wait_before_lock +{ + REPACK (CONCURRENTLY) repack_temporal USING INDEX rt_pkey; +} +step check_after_repack +{ + INSERT INTO relfilenodes(node) + SELECT relfilenode FROM pg_class WHERE relname = 'repack_temporal'; + + -- Expect 2, proving that repack has rewritten the table + SELECT count(DISTINCT node) FROM relfilenodes; + + -- Expect 2 rows + SELECT id, valid_at, label + FROM repack_temporal + ORDER BY id, valid_at, label; +} +teardown +{ + SELECT injection_points_detach('repack-concurrently-before-lock'); +} + +session s2 +step update_target +{ + UPDATE repack_temporal + SET label = 'updated' + WHERE id = '[2,3)' AND valid_at = '[2000-01-10,2000-01-20)'; +} +step check_after_update +{ + INSERT INTO relfilenodes(node) + SELECT relfilenode FROM pg_class WHERE relname = 'repack_temporal'; + + -- Expect 2 rows + SELECT id, valid_at, label + FROM repack_temporal + ORDER BY id, valid_at, label; +} +step wakeup_before_lock +{ + SELECT injection_points_wakeup('repack-concurrently-before-lock'); +} + +permutation + wait_before_lock + update_target + check_after_update + wakeup_before_lock + check_after_repack diff --git a/src/test/modules/injection_points/specs/repack_temporal_multirange.spec b/src/test/modules/injection_points/specs/repack_temporal_multirange.spec new file mode 100644 index 00000000000..8150c0b4ba8 --- /dev/null +++ b/src/test/modules/injection_points/specs/repack_temporal_multirange.spec @@ -0,0 +1,102 @@ +# REPACK (CONCURRENTLY) on a temporal replica identity index with lossy +# multirange equality. +# +# The leading identity column is of type int4multirange. Two distinct rows +# have different multirange values but the same union range, so GiST equality +# can produce both as candidates and requires exact recheck. +setup +{ + CREATE EXTENSION injection_points; + + CREATE TABLE repack_temporal_multirange ( + id int4multirange, + valid_at datemultirange, + label text, + CONSTRAINT rtm_pkey PRIMARY KEY (id, valid_at WITHOUT OVERLAPS) + ); + + ALTER TABLE repack_temporal_multirange + REPLICA IDENTITY USING INDEX rtm_pkey; + + -- (1,3)+(5,7) is the same union range of (1-7), but needs recheck + INSERT INTO repack_temporal_multirange(id, valid_at, label) + VALUES + (int4multirange(int4range(1, 3), int4range(5, 7)), + datemultirange(daterange('2000-01-01', '2000-02-01')), + 'other'), + (int4multirange(int4range(1, 7)), + datemultirange(daterange('2000-01-01', '2000-02-01')), + 'target'); + + CREATE TABLE relfilenodes(node oid); +} + +teardown +{ + DROP TABLE repack_temporal_multirange; + DROP EXTENSION injection_points; + DROP TABLE relfilenodes; +} + +session s1 +setup +{ + SELECT injection_points_set_local(); + SELECT injection_points_attach('repack-concurrently-before-lock', 'wait'); +} +step wait_before_lock +{ + REPACK (CONCURRENTLY) repack_temporal_multirange + USING INDEX rtm_pkey; +} +step final_check +{ + INSERT INTO relfilenodes(node) + SELECT relfilenode + FROM pg_class + WHERE relname = 'repack_temporal_multirange'; + + -- Expect 2, proving that repack has rewritten the table + SELECT count(DISTINCT node) FROM relfilenodes; + + -- Expect 2 rows + SELECT id, valid_at, label + FROM repack_temporal_multirange + ORDER BY id, valid_at, label; +} +teardown +{ + SELECT injection_points_detach('repack-concurrently-before-lock'); +} + +session s2 +step update_target +{ + UPDATE repack_temporal_multirange + SET label = 'updated' + WHERE id = int4multirange(int4range(1, 7)) + AND valid_at = datemultirange(daterange('2000-01-01', '2000-02-01')); +} +step check_after_update +{ + INSERT INTO relfilenodes(node) + SELECT relfilenode + FROM pg_class + WHERE relname = 'repack_temporal_multirange'; + + -- Expect 2 rows + SELECT id, valid_at, label + FROM repack_temporal_multirange + ORDER BY id, valid_at, label; +} +step wakeup_before_lock +{ + SELECT injection_points_wakeup('repack-concurrently-before-lock'); +} + +permutation + wait_before_lock + update_target + check_after_update + wakeup_before_lock + final_check