Whole-row fixes for ALTER COLUMN SET EXPRESSION

When changing the expression of a generated column via ALTER TABLE
ALTER COLUMN SET EXPRESSION, objects that depend on the column via
indirect whole-row references (such as CHECK constraints, indexes)
must be handled specially, because technically pg_depend does not
contain such dependencies, see
recordDependencyOnSingleRelExpr->find_expr_references_walker.

This is a fix for commit f80bedd52, "Allow ALTER COLUMN SET EXPRESSION
on virtual columns with CHECK constraints".

Author: jian he <jian.universality@gmail.com>
Co-authored-by: Peter Eisentraut <peter@eisentraut.org>
Reported-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reviewed-by: solai v <solai.cdac@gmail.com>
Reviewed-by: Zsolt Parragi <zsolt.parragi@percona.com>
Discussion: https://www.postgresql.org/message-id/flat/CAJTYsWXOkyeDVbzymWc9sKrq7Y_MUv6XJXN4H9GfsBOPd3NJ+w@mail.gmail.com

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 472db112fa7a08e016bc0edb51fa2dc5ad6ae56a..44bb18d34a0f3f9695035d61ca240c2934a080dc 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -693,6 +693,7 @@ static ObjectAddress ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
                                                                                   AlterTableCmd *cmd, LOCKMODE lockmode);
 static void RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype,
                                                                                          Relation rel, AttrNumber attnum, const char *colName);
+static void RememberWholeRowDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, Relation rel);
 static void RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab);
 static void RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab);
 static void RememberStatisticsForRebuilding(Oid stxoid, AlteredTableInfo *tab);
@@ -8773,6 +8774,13 @@ ATExecSetExpression(AlteredTableInfo *tab, Relation rel, const char *colName,
         */
        RememberAllDependentForRebuilding(tab, AT_SetExpression, rel, attnum, colName);
 
+       /*
+        * Find whole-row referenced objects that depend on the column
+        * (constraints, indexes, etc.), and record enough information to let us
+        * recreate the objects.
+        */
+       RememberWholeRowDependentForRebuilding(tab, AT_SetExpression, rel);
+
        /*
         * Drop the dependency records of the GENERATED expression, in particular
         * its INTERNAL dependency on the column, which would otherwise cause
@@ -15735,6 +15743,174 @@ RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype,
        table_close(depRel, NoLock);
 }
 
+/*
+ * Record information about dependencies between objects with whole-row Var
+ * references (indexes, check constraints, etc.) and the relation.
+ *
+ * See also RememberAllDependentForRebuilding, which handles non-whole-row Var
+ * references.
+ *
+ * This function currently applies only to ALTER COLUMN SET EXPRESSION.
+ */
+static void
+RememberWholeRowDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, Relation rel)
+{
+       ScanKeyData skey;
+       Relation        pg_constraint;
+       Relation        pg_index;
+       SysScanDesc conscan;
+       SysScanDesc indscan;
+       HeapTuple       constrTuple;
+       HeapTuple       indexTuple;
+       bool            isnull;
+
+       Assert(subtype == AT_SetExpression);
+
+       /*
+        * Check CHECK constraints with whole-row references first.
+        */
+       if (RelationGetDescr(rel)->constr &&
+               RelationGetDescr(rel)->constr->num_check > 0)
+       {
+               pg_constraint = table_open(ConstraintRelationId, AccessShareLock);
+
+               ScanKeyInit(&skey,
+                                       Anum_pg_constraint_conrelid,
+                                       BTEqualStrategyNumber, F_OIDEQ,
+                                       ObjectIdGetDatum(RelationGetRelid(rel)));
+
+               conscan = systable_beginscan(pg_constraint,
+                                                                        ConstraintRelidTypidNameIndexId,
+                                                                        true,
+                                                                        NULL,
+                                                                        1,
+                                                                        &skey);
+
+               while (HeapTupleIsValid(constrTuple = systable_getnext(conscan)))
+               {
+                       Form_pg_constraint conform = (Form_pg_constraint) GETSTRUCT(constrTuple);
+                       Datum           exprDatum;
+
+                       if (conform->contype != CONSTRAINT_CHECK)
+                               continue;
+
+                       exprDatum = heap_getattr(constrTuple,
+                                                                        Anum_pg_constraint_conbin,
+                                                                        RelationGetDescr(pg_constraint),
+                                                                        &isnull);
+                       if (isnull)
+                               elog(ERROR, "null conbin for relation \"%s\"",
+                                        RelationGetRelationName(rel));
+                       else
+                       {
+                               char       *exprString;
+                               Node       *expr;
+                               Bitmapset  *expr_attrs = NULL;
+
+                               exprString = TextDatumGetCString(exprDatum);
+                               expr = stringToNode(exprString);
+                               pfree(exprString);
+
+                               /* Find all attributes referenced */
+                               pull_varattnos(expr, 1, &expr_attrs);
+
+                               /*
+                                * If the CHECK constraint contains whole-row reference then
+                                * remember it.
+                                */
+                               if (bms_is_member(InvalidAttrNumber - FirstLowInvalidHeapAttributeNumber, expr_attrs))
+                               {
+                                       RememberConstraintForRebuilding(conform->oid, tab);
+                               }
+                       }
+               }
+               systable_endscan(conscan);
+               table_close(pg_constraint, AccessShareLock);
+       }
+
+       /*
+        * Now check indexes with whole-row references. Prepare to scan pg_index
+        * for entries having indrelid matching this relation.
+        */
+       ScanKeyInit(&skey,
+                               Anum_pg_index_indrelid,
+                               BTEqualStrategyNumber, F_OIDEQ,
+                               ObjectIdGetDatum(RelationGetRelid(rel)));
+
+       pg_index = table_open(IndexRelationId, AccessShareLock);
+
+       indscan = systable_beginscan(pg_index,
+                                                                IndexIndrelidIndexId,
+                                                                true,
+                                                                NULL,
+                                                                1,
+                                                                &skey);
+
+       while (HeapTupleIsValid(indexTuple = systable_getnext(indscan)))
+       {
+               Form_pg_index index = (Form_pg_index) GETSTRUCT(indexTuple);
+               Datum           exprDatum;
+
+               exprDatum = heap_getattr(indexTuple,
+                                                                Anum_pg_index_indexprs,
+                                                                RelationGetDescr(pg_index),
+                                                                &isnull);
+               if (!isnull)
+               {
+                       char       *exprString;
+                       Node       *expr;
+                       Bitmapset  *expr_attrs = NULL;
+
+                       exprString = TextDatumGetCString(exprDatum);
+                       expr = stringToNode(exprString);
+                       pfree(exprString);
+
+                       /* Find all attributes referenced */
+                       pull_varattnos(expr, 1, &expr_attrs);
+
+                       /*
+                        * If the index expression contains a whole-row reference then
+                        * remember it.
+                        */
+                       if (bms_is_member(InvalidAttrNumber - FirstLowInvalidHeapAttributeNumber, expr_attrs))
+                       {
+                               RememberIndexForRebuilding(index->indexrelid, tab);
+                               continue;
+                       }
+               }
+
+               exprDatum = heap_getattr(indexTuple,
+                                                                Anum_pg_index_indpred,
+                                                                RelationGetDescr(pg_index),
+                                                                &isnull);
+               if (!isnull)
+               {
+                       char       *exprString;
+                       Node       *expr;
+                       Bitmapset  *expr_attrs = NULL;
+
+                       exprString = TextDatumGetCString(exprDatum);
+                       expr = stringToNode(exprString);
+                       pfree(exprString);
+
+                       /* Find all attributes referenced */
+                       pull_varattnos(expr, 1, &expr_attrs);
+
+                       /*
+                        * If the index predicate expression contains a whole-row
+                        * reference then remember it.
+                        */
+                       if (bms_is_member(InvalidAttrNumber - FirstLowInvalidHeapAttributeNumber, expr_attrs))
+                       {
+                               RememberIndexForRebuilding(index->indexrelid, tab);
+                       }
+               }
+       }
+
+       systable_endscan(indscan);
+       table_close(pg_index, AccessShareLock);
+}
+
 /*
  * Subroutine for ATExecAlterColumnType: remember that a replica identity
  * needs to be reset.

diff --git a/src/test/regress/expected/generated_stored.out b/src/test/regress/expected/generated_stored.out
index e17ba2f4881f5ed9dd42b8d5e9862f6acd133b37..6a8b5113e7369ff3114effbe74778acba4b9bdf3 100644 (file)
--- a/src/test/regress/expected/generated_stored.out
+++ b/src/test/regress/expected/generated_stored.out
@@ -688,6 +688,15 @@ INSERT INTO gtest20c VALUES (1);  -- ok
 INSERT INTO gtest20c VALUES (NULL);  -- fails
 ERROR:  new row for relation "gtest20c" violates check constraint "whole_row_check"
 DETAIL:  Failing row contains (null, null).
+ALTER TABLE gtest20c ALTER COLUMN b SET EXPRESSION AS (NULL::int);  -- violates constraint
+ERROR:  check constraint "whole_row_check" of relation "gtest20c" is violated by some row
+-- index with whole-row reference needs rebuild
+CREATE TABLE gtest20d (a int, b int GENERATED ALWAYS AS (a * 2) STORED);
+INSERT INTO gtest20d VALUES (1), (1);
+CREATE INDEX gtest20d_idx1 ON gtest20d (a) WHERE gtest20d = ROW (1, 2);
+ALTER TABLE gtest20d ALTER COLUMN b SET EXPRESSION AS (a * 2::bigint);  -- index rebuild
+CREATE INDEX gtest20d_idx2 ON gtest20d ((gtest20d = ROW (1, 2)));
+ALTER TABLE gtest20d ALTER COLUMN b SET EXPRESSION AS (a * 3);  -- index rebuild
 -- not-null constraints
 CREATE TABLE gtest21a (a int PRIMARY KEY, b int GENERATED ALWAYS AS (nullif(a, 0)) STORED NOT NULL);
 INSERT INTO gtest21a (a) VALUES (1);  -- ok

diff --git a/src/test/regress/expected/generated_virtual.out b/src/test/regress/expected/generated_virtual.out
index 01ee29fee10704dbacb7aa9c26fede69f67fe8dc..6ee029796f1784f7330a7e8636091d94c24383ee 100644 (file)
--- a/src/test/regress/expected/generated_virtual.out
+++ b/src/test/regress/expected/generated_virtual.out
@@ -694,6 +694,15 @@ INSERT INTO gtest20c VALUES (1);  -- ok
 INSERT INTO gtest20c VALUES (NULL);  -- fails
 ERROR:  new row for relation "gtest20c" violates check constraint "whole_row_check"
 DETAIL:  Failing row contains (null, virtual).
+ALTER TABLE gtest20c ALTER COLUMN b SET EXPRESSION AS (NULL::int);  -- violates constraint
+ERROR:  check constraint "whole_row_check" of relation "gtest20c" is violated by some row
+-- index with whole-row reference needs rebuild
+CREATE TABLE gtest20d (a int, b int GENERATED ALWAYS AS (a * 2) VIRTUAL);
+INSERT INTO gtest20d VALUES (1), (1);
+CREATE INDEX gtest20d_idx1 ON gtest20d (a) WHERE gtest20d = ROW (1, 2);
+ALTER TABLE gtest20d ALTER COLUMN b SET EXPRESSION AS (a * 2::bigint);  -- index rebuild
+CREATE INDEX gtest20d_idx2 ON gtest20d ((gtest20d = ROW (1, 2)));
+ALTER TABLE gtest20d ALTER COLUMN b SET EXPRESSION AS (a * 3);  -- index rebuild
 -- not-null constraints
 CREATE TABLE gtest21a (a int PRIMARY KEY, b int GENERATED ALWAYS AS (nullif(a, 0)) VIRTUAL NOT NULL);
 INSERT INTO gtest21a (a) VALUES (1);  -- ok

diff --git a/src/test/regress/sql/generated_stored.sql b/src/test/regress/sql/generated_stored.sql
index 85b6212023df01cc3cf6849f98dcb7ef73922578..b349a16ddf3d3b24b89fbd03e73f98f387f0d0dd 100644 (file)
--- a/src/test/regress/sql/generated_stored.sql
+++ b/src/test/regress/sql/generated_stored.sql
@@ -341,6 +341,16 @@ CREATE TABLE gtest20c (a int, b int GENERATED ALWAYS AS (a * 2) STORED);
 ALTER TABLE gtest20c ADD CONSTRAINT whole_row_check CHECK (gtest20c IS NOT NULL);
 INSERT INTO gtest20c VALUES (1);  -- ok
 INSERT INTO gtest20c VALUES (NULL);  -- fails
+ALTER TABLE gtest20c ALTER COLUMN b SET EXPRESSION AS (NULL::int);  -- violates constraint
+
+-- index with whole-row reference needs rebuild
+CREATE TABLE gtest20d (a int, b int GENERATED ALWAYS AS (a * 2) STORED);
+INSERT INTO gtest20d VALUES (1), (1);
+CREATE INDEX gtest20d_idx1 ON gtest20d (a) WHERE gtest20d = ROW (1, 2);
+
+ALTER TABLE gtest20d ALTER COLUMN b SET EXPRESSION AS (a * 2::bigint);  -- index rebuild
+CREATE INDEX gtest20d_idx2 ON gtest20d ((gtest20d = ROW (1, 2)));
+ALTER TABLE gtest20d ALTER COLUMN b SET EXPRESSION AS (a * 3);  -- index rebuild
 
 -- not-null constraints
 CREATE TABLE gtest21a (a int PRIMARY KEY, b int GENERATED ALWAYS AS (nullif(a, 0)) STORED NOT NULL);

diff --git a/src/test/regress/sql/generated_virtual.sql b/src/test/regress/sql/generated_virtual.sql
index 0cb14eb0e36f81079c66303e63735bdd1553fb76..ed9d50fe784c0bab6de95092403e1ce1bb91f5ce 100644 (file)
--- a/src/test/regress/sql/generated_virtual.sql
+++ b/src/test/regress/sql/generated_virtual.sql
@@ -347,6 +347,16 @@ CREATE TABLE gtest20c (a int, b int GENERATED ALWAYS AS (a * 2) VIRTUAL);
 ALTER TABLE gtest20c ADD CONSTRAINT whole_row_check CHECK (gtest20c IS NOT NULL);
 INSERT INTO gtest20c VALUES (1);  -- ok
 INSERT INTO gtest20c VALUES (NULL);  -- fails
+ALTER TABLE gtest20c ALTER COLUMN b SET EXPRESSION AS (NULL::int);  -- violates constraint
+
+-- index with whole-row reference needs rebuild
+CREATE TABLE gtest20d (a int, b int GENERATED ALWAYS AS (a * 2) VIRTUAL);
+INSERT INTO gtest20d VALUES (1), (1);
+CREATE INDEX gtest20d_idx1 ON gtest20d (a) WHERE gtest20d = ROW (1, 2);
+
+ALTER TABLE gtest20d ALTER COLUMN b SET EXPRESSION AS (a * 2::bigint);  -- index rebuild
+CREATE INDEX gtest20d_idx2 ON gtest20d ((gtest20d = ROW (1, 2)));
+ALTER TABLE gtest20d ALTER COLUMN b SET EXPRESSION AS (a * 3);  -- index rebuild
 
 -- not-null constraints
 CREATE TABLE gtest21a (a int PRIMARY KEY, b int GENERATED ALWAYS AS (nullif(a, 0)) VIRTUAL NOT NULL);