From: Tom Lane <tgl@sss.pgh.pa.us>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: andreas@visena.com, Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: ERROR: attribute number 6 exceeds number of columns 5
Date: Tue, 07 Jan 2020 12:47:00 -0500
Message-ID: <13814.1578419220@sss.pgh.pa.us> (raw)
In-Reply-To: <18651.1578415999@sss.pgh.pa.us>
References: <VisenaEmail.200.231b0a41523275d0.16ea7f800c7@tc7-visena>
<6352.1574783351@sss.pgh.pa.us>
<20191127.120034.20463388950832898.horikyota.ntt@gmail.com>
<18651.1578415999@sss.pgh.pa.us>
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
Attachments:
[text/x-diff] fix-alter-type-for-generated-columns.patch (5.6K, ../13814.1578419220@sss.pgh.pa.us/2-fix-alter-type-for-generated-columns.patch)
download | inline diff:diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex 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.outindex 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.sqlindex 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 (
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, horikyota.ntt@gmail.com, peter.eisentraut@2ndquadrant.com, pgsql-hackers@lists.postgresql.org
Subject: Re: ERROR: attribute number 6 exceeds number of columns 5
In-Reply-To: <13814.1578419220@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox