Re: Generated column is not updated (Postgres 13) - Mailing list pgsql-bugs

After looking at this further, I see that there already is a check
for system columns in GENERATED expressions; it's just in the
parser (scanNSItemForColumn), not where I was looking for it.
And it explicitly exempts tableoid.  I continue to think that
that's a bad idea and we're gonna regret it, but at least the
issue in ATRewriteTable turns out to be trivial to fix.
So I did that and upgraded the test scenario to be a bit
more realistic, in 0001 below.  0002 then just disallows
whole-row variables.  (I added an errdetail trying to
explain that restriction, too.)

It's worth pointing out here that what seems to me to be a
"more realistic" test scenario involves a regclass constant
for the table's own OID.  If that doesn't scare you, it should,
because it implies all kinds of constraints on the order in which
these expressions are processed during CREATE or ALTER TABLE.
The test passes check-world, which includes dump/reload, but I am
sorely afraid that there are ways in which this sort of thing
would fail.  Do we *really* want to buy into supporting it?

            regards, tom lane

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index ebc62034d2..85dcc33063 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -5761,6 +5761,14 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode)
                 foreach(lc, dropped_attrs)
                     newslot->tts_isnull[lfirst_int(lc)] = true;

+                /*
+                 * Constraints and GENERATED expressions might reference the
+                 * tableoid column, so fill tts_tableOid with the desired
+                 * value.  (We must do this each time, because it gets
+                 * overwritten with newrel's OID during storing.)
+                 */
+                newslot->tts_tableOid = RelationGetRelid(oldrel);
+
                 /*
                  * Process supplied expressions to replace selected columns.
                  *
@@ -5804,11 +5812,6 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode)
                                        &newslot->tts_isnull[ex->attnum - 1]);
                 }

-                /*
-                 * Constraints might reference the tableoid column, so
-                 * initialize t_tableOid before evaluating them.
-                 */
-                newslot->tts_tableOid = RelationGetRelid(oldrel);
                 insertslot = newslot;
             }
             else
diff --git a/src/test/regress/expected/generated.out b/src/test/regress/expected/generated.out
index 675773f0c1..71542e95d5 100644
--- a/src/test/regress/expected/generated.out
+++ b/src/test/regress/expected/generated.out
@@ -64,6 +64,7 @@ ERROR:  both identity and generation expression specified for column "b" of tabl
 LINE 1: ...t PRIMARY KEY, b int GENERATED ALWAYS AS identity GENERATED ...
                                                              ^
 -- reference to system column not allowed in generated column
+-- (except tableoid, which we test below)
 CREATE TABLE gtest_err_6a (a int PRIMARY KEY, b bool GENERATED ALWAYS AS (xmin <> 37) STORED);
 ERROR:  cannot use system column "xmin" in column generation expression
 LINE 1: ...a (a int PRIMARY KEY, b bool GENERATED ALWAYS AS (xmin <> 37...
@@ -455,14 +456,16 @@ DROP TYPE double_int;
 -- using tableoid is allowed
 CREATE TABLE gtest_tableoid (
   a int PRIMARY KEY,
-  b bool GENERATED ALWAYS AS (tableoid <> 0) STORED
+  b bool GENERATED ALWAYS AS (tableoid = 'gtest_tableoid'::regclass) STORED
 );
 INSERT INTO gtest_tableoid VALUES (1), (2);
+ALTER TABLE gtest_tableoid ADD COLUMN
+  c regclass GENERATED ALWAYS AS (tableoid) STORED;
 SELECT * FROM gtest_tableoid;
- a | b
----+---
- 1 | t
- 2 | t
+ a | b |       c
+---+---+----------------
+ 1 | t | gtest_tableoid
+ 2 | t | gtest_tableoid
 (2 rows)

 -- drop column behavior
diff --git a/src/test/regress/sql/generated.sql b/src/test/regress/sql/generated.sql
index 63251c443a..914197608b 100644
--- a/src/test/regress/sql/generated.sql
+++ b/src/test/regress/sql/generated.sql
@@ -29,6 +29,7 @@ CREATE TABLE gtest_err_5a (a int PRIMARY KEY, b int DEFAULT 5 GENERATED ALWAYS A
 CREATE TABLE gtest_err_5b (a int PRIMARY KEY, b int GENERATED ALWAYS AS identity GENERATED ALWAYS AS (a * 2) STORED);

 -- reference to system column not allowed in generated column
+-- (except tableoid, which we test below)
 CREATE TABLE gtest_err_6a (a int PRIMARY KEY, b bool GENERATED ALWAYS AS (xmin <> 37) STORED);

 -- various prohibited constructs
@@ -218,9 +219,11 @@ DROP TYPE double_int;
 -- using tableoid is allowed
 CREATE TABLE gtest_tableoid (
   a int PRIMARY KEY,
-  b bool GENERATED ALWAYS AS (tableoid <> 0) STORED
+  b bool GENERATED ALWAYS AS (tableoid = 'gtest_tableoid'::regclass) STORED
 );
 INSERT INTO gtest_tableoid VALUES (1), (2);
+ALTER TABLE gtest_tableoid ADD COLUMN
+  c regclass GENERATED ALWAYS AS (tableoid) STORED;
 SELECT * FROM gtest_tableoid;

 -- drop column behavior
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 431e62e389..ba3e1ecf45 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -3020,15 +3020,26 @@ check_nested_generated_walker(Node *node, void *context)
         AttrNumber    attnum;

         relid = rt_fetch(var->varno, pstate->p_rtable)->relid;
+        if (!OidIsValid(relid))
+            return false;        /* XXX shouldn't we raise an error? */
+
         attnum = var->varattno;

-        if (OidIsValid(relid) && AttributeNumberIsValid(attnum) && get_attgenerated(relid, attnum))
+        if (attnum > 0 && get_attgenerated(relid, attnum))
             ereport(ERROR,
-                    (errcode(ERRCODE_SYNTAX_ERROR),
+                    (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
                      errmsg("cannot use generated column \"%s\" in column generation expression",
                             get_attname(relid, attnum, false)),
                      errdetail("A generated column cannot reference another generated column."),
                      parser_errposition(pstate, var->location)));
+        /* A whole-row Var is necessarily self-referential, so forbid it */
+        if (attnum == 0)
+            ereport(ERROR,
+                    (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                     errmsg("cannot use whole-row variable in column generation expression"),
+                     errdetail("This would cause the generated column to depend on its own value."),
+                     parser_errposition(pstate, var->location)));
+        /* System columns were already checked in the parser */

         return false;
     }
diff --git a/src/test/regress/expected/generated.out b/src/test/regress/expected/generated.out
index 71542e95d5..c2e5676196 100644
--- a/src/test/regress/expected/generated.out
+++ b/src/test/regress/expected/generated.out
@@ -46,6 +46,13 @@ ERROR:  cannot use generated column "b" in column generation expression
 LINE 1: ...AYS AS (a * 2) STORED, c int GENERATED ALWAYS AS (b * 3) STO...
                                                              ^
 DETAIL:  A generated column cannot reference another generated column.
+-- a whole-row var is a self-reference on steroids, so disallow that too
+CREATE TABLE gtest_err_2c (a int PRIMARY KEY,
+    b int GENERATED ALWAYS AS (num_nulls(gtest_err_2c)) STORED);
+ERROR:  cannot use whole-row variable in column generation expression
+LINE 2:     b int GENERATED ALWAYS AS (num_nulls(gtest_err_2c)) STOR...
+                                                 ^
+DETAIL:  This would cause the generated column to depend on its own value.
 -- invalid reference
 CREATE TABLE gtest_err_3 (a int PRIMARY KEY, b int GENERATED ALWAYS AS (c * 2) STORED);
 ERROR:  column "c" does not exist
diff --git a/src/test/regress/sql/generated.sql b/src/test/regress/sql/generated.sql
index 914197608b..b7eb072671 100644
--- a/src/test/regress/sql/generated.sql
+++ b/src/test/regress/sql/generated.sql
@@ -17,6 +17,9 @@ CREATE TABLE gtest_err_1 (a int PRIMARY KEY, b int GENERATED ALWAYS AS (a * 2) S
 -- references to other generated columns, including self-references
 CREATE TABLE gtest_err_2a (a int PRIMARY KEY, b int GENERATED ALWAYS AS (b * 2) STORED);
 CREATE TABLE gtest_err_2b (a int PRIMARY KEY, b int GENERATED ALWAYS AS (a * 2) STORED, c int GENERATED ALWAYS AS (b *
3)STORED); 
+-- a whole-row var is a self-reference on steroids, so disallow that too
+CREATE TABLE gtest_err_2c (a int PRIMARY KEY,
+    b int GENERATED ALWAYS AS (num_nulls(gtest_err_2c)) STORED);

 -- invalid reference
 CREATE TABLE gtest_err_3 (a int PRIMARY KEY, b int GENERATED ALWAYS AS (c * 2) STORED);