I wrote:
> So I believe that the real problem here is that the executor is
> evaluating GENERATED expressions at the wrong time. It's evaluating
> them against the pre-conversion tuples when it should be evaluating
> them against the post-conversion tuples. We need to go fix that,
> rather than inserting arbitrary restrictions in the DDL code.
I looked at that more closely, and realized that blaming the executor
is wrong: the real issue is that ALTER TABLE itself supposes that it
need only evaluate expressions against the old tuple. That's easy
to fix with a bit more code though. I propose the attached.
(Note that this should also allow relaxing the existing implementation
restriction against changing types of columns that GENERATED columns
depend on: all we have to do is re-parse the generation expression
and schedule it for evaluation. I've not looked into that, and it
doesn't seem like a bug fix anyway.)
regards, tom lane
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 1c4394a..421bc28 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -192,13 +192,15 @@ typedef struct NewConstraint
* Phase 3 copy (this could be either a new column with a non-null default, or
* a column that we're changing the type of). Columns without such an entry
* are just copied from the old table during ATRewriteTable. Note that the
- * expr is an expression over *old* table values.
+ * expr is an expression over *old* table values, except when is_generated
+ * is true; then it is an expression over columns of the *new* tuple.
*/
typedef struct NewColumnValue
{
AttrNumber attnum; /* which column */
Expr *expr; /* expression to compute */
ExprState *exprstate; /* execution state */
+ bool is_generated; /* is it a GENERATED expression? */
} NewColumnValue;
/*
@@ -4961,7 +4963,9 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode)
/*
* Process supplied expressions to replace selected columns.
- * Expression inputs come from the old tuple.
+ *
+ * First, evaluate expressions whose inputs come from the old
+ * tuple.
*/
econtext->ecxt_scantuple = oldslot;
@@ -4969,6 +4973,9 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode)
{
NewColumnValue *ex = lfirst(l);
+ if (ex->is_generated)
+ continue;
+
newslot->tts_values[ex->attnum - 1]
= ExecEvalExpr(ex->exprstate,
econtext,
@@ -4978,6 +4985,26 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode)
ExecStoreVirtualTuple(newslot);
/*
+ * Now, evaluate any expressions whose inputs come from the
+ * new tuple. We assume these columns won't reference each
+ * other, so that there's no ordering dependency.
+ */
+ econtext->ecxt_scantuple = newslot;
+
+ foreach(l, tab->newvals)
+ {
+ NewColumnValue *ex = lfirst(l);
+
+ if (!ex->is_generated)
+ continue;
+
+ newslot->tts_values[ex->attnum - 1]
+ = ExecEvalExpr(ex->exprstate,
+ econtext,
+ &newslot->tts_isnull[ex->attnum - 1]);
+ }
+
+ /*
* Constraints might reference the tableoid column, so
* initialize t_tableOid before evaluating them.
*/
@@ -5892,6 +5919,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
newval = (NewColumnValue *) palloc0(sizeof(NewColumnValue));
newval->attnum = attribute.attnum;
newval->expr = expression_planner(defval);
+ newval->is_generated = (colDef->generated != '\0');
tab->newvals = lappend(tab->newvals, newval);
}
@@ -10379,6 +10407,7 @@ ATPrepAlterColumnType(List **wqueue,
newval = (NewColumnValue *) palloc0(sizeof(NewColumnValue));
newval->attnum = attnum;
newval->expr = (Expr *) transform;
+ newval->is_generated = false;
tab->newvals = lappend(tab->newvals, newval);
if (ATColumnChangeRequiresRewrite(transform, attnum))
diff --git a/src/test/regress/expected/generated.out b/src/test/regress/expected/generated.out
index f62c93f..3ef10fa 100644
--- a/src/test/regress/expected/generated.out
+++ b/src/test/regress/expected/generated.out
@@ -593,6 +593,31 @@ ERROR: cannot use generated column "b" in column generation expression
DETAIL: A generated column cannot reference another generated column.
ALTER TABLE gtest25 ADD COLUMN x int GENERATED ALWAYS AS (z * 4) STORED; -- error
ERROR: column "z" does not exist
+ALTER TABLE gtest25 ADD COLUMN c int DEFAULT 42,
+ ADD COLUMN x int GENERATED ALWAYS AS (c * 4) STORED;
+ALTER TABLE gtest25 ADD COLUMN d int DEFAULT 101;
+ALTER TABLE gtest25 ALTER COLUMN d SET DATA TYPE float8,
+ ADD COLUMN y float8 GENERATED ALWAYS AS (d * 4) STORED;
+SELECT * FROM gtest25 ORDER BY a;
+ a | b | c | x | d | y
+---+----+----+-----+-----+-----
+ 3 | 9 | 42 | 168 | 101 | 404
+ 4 | 12 | 42 | 168 | 101 | 404
+(2 rows)
+
+\d gtest25
+ Table "public.gtest25"
+ Column | Type | Collation | Nullable | Default
+--------+------------------+-----------+----------+------------------------------------------------------
+ a | integer | | not null |
+ b | integer | | | generated always as (a * 3) stored
+ c | integer | | | 42
+ x | integer | | | generated always as (c * 4) stored
+ d | double precision | | | 101
+ y | double precision | | | generated always as (d * 4::double precision) stored
+Indexes:
+ "gtest25_pkey" PRIMARY KEY, btree (a)
+
-- ALTER TABLE ... ALTER COLUMN
CREATE TABLE gtest27 (
a int,
diff --git a/src/test/regress/sql/generated.sql b/src/test/regress/sql/generated.sql
index 6a56ae2..f75d760 100644
--- a/src/test/regress/sql/generated.sql
+++ b/src/test/regress/sql/generated.sql
@@ -316,6 +316,13 @@ ALTER TABLE gtest25 ADD COLUMN b int GENERATED ALWAYS AS (a * 3) STORED;
SELECT * FROM gtest25 ORDER BY a;
ALTER TABLE gtest25 ADD COLUMN x int GENERATED ALWAYS AS (b * 4) STORED; -- error
ALTER TABLE gtest25 ADD COLUMN x int GENERATED ALWAYS AS (z * 4) STORED; -- error
+ALTER TABLE gtest25 ADD COLUMN c int DEFAULT 42,
+ ADD COLUMN x int GENERATED ALWAYS AS (c * 4) STORED;
+ALTER TABLE gtest25 ADD COLUMN d int DEFAULT 101;
+ALTER TABLE gtest25 ALTER COLUMN d SET DATA TYPE float8,
+ ADD COLUMN y float8 GENERATED ALWAYS AS (d * 4) STORED;
+SELECT * FROM gtest25 ORDER BY a;
+\d gtest25
-- ALTER TABLE ... ALTER COLUMN
CREATE TABLE gtest27 (
