database/postgres
database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-04-21 12:05:57 +03:00
Code

Relax overly strict assertion

Browse Source 
Ever since its birth, ReorderBufferBuildTupleCidHash() has contained an
assertion that a catalog tuple cannot change Cmax after acquiring one.  But
that's wrong: if a subtransaction executes DDL that affects that catalog
tuple, and later aborts and another DDL affects the same tuple, it will
change Cmax.  Relax the assertion to merely verify that the Cmax remains
valid and monotonically increasing, instead.

Add a test that tickles the relevant code.

Diagnosed by, and initial patch submitted by: Arseny Sher
Co-authored-by: Arseny Sher
Discussion: https://postgr.es/m/874l9p8hyw.fsf@ars-thinkpad
This commit is contained in:
Alvaro Herrera 2019-02-12 18:42:37 -03:00
parent d357a16997
commit 8c67d29fd5
3 changed files with 40 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
contrib/test_decoding/expected/ddl.out
View File

@ -409,6 +409,24 @@ SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'inc
COMMIT COMMIT
(6 rows) (6 rows)
-- check that DDL in aborted subtransactions handled correctly
CREATE TABLE tr_sub_ddl(data int);
BEGIN;
SAVEPOINT a;
ALTER TABLE tr_sub_ddl ALTER COLUMN data TYPE text;
INSERT INTO tr_sub_ddl VALUES ('blah-blah');
ROLLBACK TO SAVEPOINT a;
ALTER TABLE tr_sub_ddl ALTER COLUMN data TYPE bigint;
INSERT INTO tr_sub_ddl VALUES(43);
COMMIT;
SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1');
data
--------------------------------------------------
BEGIN
table public.tr_sub_ddl: INSERT: data[bigint]:43
COMMIT
(3 rows)
/* /*
* Check whether treating a table as a catalog table works somewhat * Check whether treating a table as a catalog table works somewhat
*/ */

13
contrib/test_decoding/sql/ddl.sql
View File

@ -234,6 +234,19 @@ INSERT INTO tr_sub(path) VALUES ('5-top-1-#1');
COMMIT; COMMIT;
SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1');
-- check that DDL in aborted subtransactions handled correctly
CREATE TABLE tr_sub_ddl(data int);
BEGIN;
SAVEPOINT a;
ALTER TABLE tr_sub_ddl ALTER COLUMN data TYPE text;
INSERT INTO tr_sub_ddl VALUES ('blah-blah');
ROLLBACK TO SAVEPOINT a;
ALTER TABLE tr_sub_ddl ALTER COLUMN data TYPE bigint;
INSERT INTO tr_sub_ddl VALUES(43);
COMMIT;
SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1'); SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1');

14
src/backend/replication/logical/reorderbuffer.c
View File

@ -1326,15 +1326,19 @@ ReorderBufferBuildTupleCidHash(ReorderBuffer *rb, ReorderBufferTXN *txn)
} }
else else
{ {
/*
* Maybe we already saw this tuple before in this transaction,
* but if so it must have the same cmin.
*/
Assert(ent->cmin == change->data.tuplecid.cmin); Assert(ent->cmin == change->data.tuplecid.cmin);
Assert(ent->cmax == InvalidCommandId ||
ent->cmax == change->data.tuplecid.cmax);
/* /*
* if the tuple got valid in this transaction and now got deleted * cmax may be initially invalid, but once set it can only grow,
* we already have a valid cmin stored. The cmax will be * and never become invalid again.
* InvalidCommandId though.
*/ */
Assert((ent->cmax == InvalidCommandId) ||
((change->data.tuplecid.cmax != InvalidCommandId) &&
(change->data.tuplecid.cmax > ent->cmax)));
ent->cmax = change->data.tuplecid.cmax; ent->cmax = change->data.tuplecid.cmax;
} }
} }