Re: ERROR: attribute number 6 exceeds number of columns 5 - Mailing list pgsql-hackers

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 (