Fix toast rewrites in logical decoding.

Commit 325f2ec555 introduced pg_class.relwrite to skip operations on
tables created as part of a heap rewrite during DDL. It links such
transient heaps to the original relation OID via this new field in
pg_class but forgot to do anything about toast tables. So, logical
decoding was not able to skip operations on internally created toast
tables. This leads to an error when we tried to decode the WAL for the
next operation for which it appeared that there is a toast data where
actually it didn't have any toast data.

To fix this, we set pg_class.relwrite for internally created toast tables
as well which allowed skipping operations on them during logical decoding.

Author: Bertrand Drouvot
Reviewed-by: David Zhang, Amit Kapila
Backpatch-through: 11, where it was introduced
Discussion: https://postgr.es/m/b5146fb1-ad9e-7d6e-f980-98ed68744a7c@amazon.com

diff --git a/contrib/test_decoding/expected/toast.out b/contrib/test_decoding/expected/toast.out
index 75c4d22d8013136ed0019746a29b15d236ced6e8..cd03e9d50a16cf5e70f71864fe418964e9628e00 100644 (file)
--- a/contrib/test_decoding/expected/toast.out
+++ b/contrib/test_decoding/expected/toast.out
@@ -360,6 +360,28 @@ WHERE data NOT LIKE '%INSERT: %';
  COMMIT
 (4 rows)
 
+/*
+ * Test decoding relation rewrite with toast. The insert into tbl2 within the
+ * same transaction is there to check that there is no remaining toast_hash not
+ * being reset.
+ */
+CREATE TABLE tbl1 (a INT, b TEXT);
+CREATE TABLE tbl2 (a INT);
+ALTER TABLE tbl1 ALTER COLUMN b SET STORAGE EXTERNAL;
+BEGIN;
+INSERT INTO tbl1 VALUES(1, repeat('a', 4000)) ;
+ALTER TABLE tbl1 ADD COLUMN id serial primary key;
+INSERT INTO tbl2 VALUES(1);
+commit;
+SELECT substr(data, 1, 200) FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1');
+                                                                                                  substr                                                                                                  
+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ BEGIN
+ table public.tbl1: INSERT: a[integer]:1 b[text]:'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+ table public.tbl2: INSERT: a[integer]:1
+ COMMIT
+(4 rows)
+
 SELECT pg_drop_replication_slot('regression_slot');
  pg_drop_replication_slot 
 --------------------------

diff --git a/contrib/test_decoding/sql/toast.sql b/contrib/test_decoding/sql/toast.sql
index 016c3ab78424986ff80e4089cc9a5a277c49ad8f..d1c560a174d63bc2463257980519326ebfe73f46 100644 (file)
--- a/contrib/test_decoding/sql/toast.sql
+++ b/contrib/test_decoding/sql/toast.sql
@@ -308,4 +308,20 @@ DROP TABLE toasted_several;
 
 SELECT regexp_replace(data, '^(.{100}).*(.{100})$', '\1..\2') FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1')
 WHERE data NOT LIKE '%INSERT: %';
+
+/*
+ * Test decoding relation rewrite with toast. The insert into tbl2 within the
+ * same transaction is there to check that there is no remaining toast_hash not
+ * being reset.
+ */
+CREATE TABLE tbl1 (a INT, b TEXT);
+CREATE TABLE tbl2 (a INT);
+ALTER TABLE tbl1 ALTER COLUMN b SET STORAGE EXTERNAL;
+BEGIN;
+INSERT INTO tbl1 VALUES(1, repeat('a', 4000)) ;
+ALTER TABLE tbl1 ADD COLUMN id serial primary key;
+INSERT INTO tbl2 VALUES(1);
+commit;
+SELECT substr(data, 1, 200) FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1');
+
 SELECT pg_drop_replication_slot('regression_slot');

diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c
index de6282a66750d77df0670b2404ae8450b556d80e..6695175f1bf1f18e91b6f5b21607be667156c799 100644 (file)
--- a/src/backend/catalog/toasting.c
+++ b/src/backend/catalog/toasting.c
@@ -38,9 +38,11 @@
 Oid                    binary_upgrade_next_toast_pg_type_oid = InvalidOid;
 
 static void CheckAndCreateToastTable(Oid relOid, Datum reloptions,
-                                                                        LOCKMODE lockmode, bool check);
+                                                                        LOCKMODE lockmode, bool check,
+                                                                        Oid OIDOldToast);
 static bool create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
-                                                          Datum reloptions, LOCKMODE lockmode, bool check);
+                                                          Datum reloptions, LOCKMODE lockmode, bool check,
+                                                          Oid OIDOldToast);
 static bool needs_toast_table(Relation rel);
 
 
@@ -59,30 +61,34 @@ static bool needs_toast_table(Relation rel);
 void
 AlterTableCreateToastTable(Oid relOid, Datum reloptions, LOCKMODE lockmode)
 {
-       CheckAndCreateToastTable(relOid, reloptions, lockmode, true);
+       CheckAndCreateToastTable(relOid, reloptions, lockmode, true, InvalidOid);
 }
 
 void
-NewHeapCreateToastTable(Oid relOid, Datum reloptions, LOCKMODE lockmode)
+NewHeapCreateToastTable(Oid relOid, Datum reloptions, LOCKMODE lockmode,
+                                               Oid OIDOldToast)
 {
-       CheckAndCreateToastTable(relOid, reloptions, lockmode, false);
+       CheckAndCreateToastTable(relOid, reloptions, lockmode, false, OIDOldToast);
 }
 
 void
 NewRelationCreateToastTable(Oid relOid, Datum reloptions)
 {
-       CheckAndCreateToastTable(relOid, reloptions, AccessExclusiveLock, false);
+       CheckAndCreateToastTable(relOid, reloptions, AccessExclusiveLock, false,
+                                                        InvalidOid);
 }
 
 static void
-CheckAndCreateToastTable(Oid relOid, Datum reloptions, LOCKMODE lockmode, bool check)
+CheckAndCreateToastTable(Oid relOid, Datum reloptions, LOCKMODE lockmode,
+                                                bool check, Oid OIDOldToast)
 {
        Relation        rel;
 
        rel = table_open(relOid, lockmode);
 
        /* create_toast_table does all the work */
-       (void) create_toast_table(rel, InvalidOid, InvalidOid, reloptions, lockmode, check);
+       (void) create_toast_table(rel, InvalidOid, InvalidOid, reloptions, lockmode,
+                                                         check, OIDOldToast);
 
        table_close(rel, NoLock);
 }
@@ -108,7 +114,7 @@ BootstrapToastTable(char *relName, Oid toastOid, Oid toastIndexOid)
 
        /* create_toast_table does all the work */
        if (!create_toast_table(rel, toastOid, toastIndexOid, (Datum) 0,
-                                                       AccessExclusiveLock, false))
+                                                       AccessExclusiveLock, false, InvalidOid))
                elog(ERROR, "\"%s\" does not require a toast table",
                         relName);
 
@@ -125,7 +131,8 @@ BootstrapToastTable(char *relName, Oid toastOid, Oid toastIndexOid)
  */
 static bool
 create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
-                                  Datum reloptions, LOCKMODE lockmode, bool check)
+                                  Datum reloptions, LOCKMODE lockmode, bool check,
+                                  Oid OIDOldToast)
 {
        Oid                     relOid = RelationGetRelid(rel);
        HeapTuple       reltup;
@@ -270,7 +277,7 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
                                                                                   false,
                                                                                   true,
                                                                                   true,
-                                                                                  InvalidOid,
+                                                                                  OIDOldToast,
                                                                                   NULL);
        Assert(toast_relid != InvalidOid);
 

diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index c7fda21634dec5f6ee1636b2dac2e60d03d49b97..bd6f408bcf566dbd9f4aa972073b2c9e3482f7f7 100644 (file)
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -745,7 +745,7 @@ make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, char relpersistence,
                if (isNull)
                        reloptions = (Datum) 0;
 
-               NewHeapCreateToastTable(OIDNewHeap, reloptions, lockmode);
+               NewHeapCreateToastTable(OIDNewHeap, reloptions, lockmode, toastid);
 
                ReleaseSysCache(tuple);
        }
@@ -1504,6 +1504,14 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
 
                        RenameRelationInternal(toastidx,
                                                                   NewToastName, true, true);
+
+                       /*
+                        * Reset the relrewrite for the toast. The command-counter
+                        * increment is required here as we are about to update
+                        * the tuple that is updated as part of RenameRelationInternal.
+                        */
+                       CommandCounterIncrement();
+                       ResetRelRewrite(newrel->rd_rel->reltoastrelid);
                }
                relation_close(newrel, NoLock);
        }

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 2e43a32ae55db2df7b2c36817da5996026b25b2e..40a56839166c5c8010101586d85b86446e18832e 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -3520,6 +3520,37 @@ RenameRelationInternal(Oid myrelid, const char *newrelname, bool is_internal, bo
        relation_close(targetrelation, NoLock);
 }
 
+/*
+ *             ResetRelRewrite - reset relrewrite
+ */
+void
+ResetRelRewrite(Oid myrelid)
+{
+       Relation        relrelation;    /* for RELATION relation */
+       HeapTuple       reltup;
+       Form_pg_class relform;
+
+       /*
+        * Find relation's pg_class tuple.
+        */
+       relrelation = table_open(RelationRelationId, RowExclusiveLock);
+
+       reltup = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(myrelid));
+       if (!HeapTupleIsValid(reltup))  /* shouldn't happen */
+               elog(ERROR, "cache lookup failed for relation %u", myrelid);
+       relform = (Form_pg_class) GETSTRUCT(reltup);
+
+       /*
+        * Update pg_class tuple.
+        */
+       relform->relrewrite = InvalidOid;
+
+       CatalogTupleUpdate(relrelation, &reltup->t_self, reltup);
+
+       heap_freetuple(reltup);
+       table_close(relrelation, RowExclusiveLock);
+}
+
 /*
  * Disallow ALTER TABLE (and similar commands) when the current backend has
  * any open reference to the target table besides the one just acquired by

diff --git a/src/include/catalog/toasting.h b/src/include/catalog/toasting.h
index cc5dfed0bf6b9475ecfd775f0267cca02bdf7a8c..02af8ead045427a444ba913208cfe5b8a7dc1311 100644 (file)
--- a/src/include/catalog/toasting.h
+++ b/src/include/catalog/toasting.h
@@ -24,7 +24,7 @@
  */
 extern void NewRelationCreateToastTable(Oid relOid, Datum reloptions);
 extern void NewHeapCreateToastTable(Oid relOid, Datum reloptions,
-                                                                       LOCKMODE lockmode);
+                                                                       LOCKMODE lockmode, Oid OIDOldToast);
 extern void AlterTableCreateToastTable(Oid relOid, Datum reloptions,
                                                                           LOCKMODE lockmode);
 extern void BootstrapToastTable(char *relName,

diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h
index b09afa2775b9d1cf5951dff9db0d9020745d5c60..f57b4034c87c4957a17b19ab76f74146acc155c1 100644 (file)
--- a/src/include/commands/tablecmds.h
+++ b/src/include/commands/tablecmds.h
@@ -70,6 +70,8 @@ extern void RenameRelationInternal(Oid myrelid,
                                                                   const char *newrelname, bool is_internal,
                                                                   bool is_index);
 
+extern void ResetRelRewrite(Oid myrelid);
+
 extern void find_composite_type_dependencies(Oid typeOid,
                                                                                         Relation origRelation,
                                                                                         const char *origTypeName);