Thread: ERROR: attribute number 6 exceeds number of columns 5
Run the attached script and you'll get:
psql -f error.sql -d test
psql:error.sql:37: ERROR: attribute number 6 exceeds number of columns 5
Splitting up the alter-table like this makes it work:
alter table access
add column start_timestamp timestamp not null DEFAULT CURRENT_TIMESTAMP,
add column end_timestamp timestamp
;
alter table access add column tsrange TSRANGE NOT NULL GENERATED ALWAYS AS (tsrange(start_timestamp, end_timestamp, '[)')) STORED
;
add column start_timestamp timestamp not null DEFAULT CURRENT_TIMESTAMP,
add column end_timestamp timestamp
;
alter table access add column tsrange TSRANGE NOT NULL GENERATED ALWAYS AS (tsrange(start_timestamp, end_timestamp, '[)')) STORED
;
--
Andreas Joseph Krogh
Attachment
Andreas Joseph Krogh <andreas@visena.com> writes:
> Run the attached script and you'll get:
> psql -f error.sql -d test
> psql:error.sql:37: ERROR: attribute number 6 exceeds number of columns 5
Hmm, interesting. IMO, that *should* have thrown an error, but of
course not that one. The ADD COLUMN operations are all processed
in parallel, so it's not okay for one of them to have a GENERATED
expression that refers to another one of the new columns. But you
should have gotten a "no such column" type of error, not a run-time
cross-check failure.
regards, tom lane
At Tue, 26 Nov 2019 10:49:11 -0500, Tom Lane <tgl@sss.pgh.pa.us> wrote in
> Andreas Joseph Krogh <andreas@visena.com> writes:
> > Run the attached script and you'll get:
>
> > psql -f error.sql -d test
> > psql:error.sql:37: ERROR: attribute number 6 exceeds number of columns 5
>
> Hmm, interesting. IMO, that *should* have thrown an error, but of
> course not that one. The ADD COLUMN operations are all processed
> in parallel, so it's not okay for one of them to have a GENERATED
> expression that refers to another one of the new columns. But you
> should have gotten a "no such column" type of error, not a run-time
> cross-check failure.
Something like this works?
ALTER TABLE gtest25 ADD COLUMN x int, ADD COLUMN y int GENERATED ALWAYS AS (x * 4) STORED;
ERROR: column "x" does not exist
DETAIL: An expression cannot reference columns added in the same command.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
From 92787bcef1b190696f71238879bcd3a9cf51d42b Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Wed, 27 Nov 2019 11:16:07 +0900
Subject: [PATCH] Emit meaningful message for reference to concurrently added
column.
In ALTER TABLE ADD COLUMN, expressions cannot refer to columns
concurrently added by other subcommands. Currently we get an improper
message like "attribute number 6 exceeds number of columns 5" for the
case. Fix it.
---
src/backend/commands/tablecmds.c | 28 ++++++++++++++++++++++++-
src/test/regress/expected/generated.out | 3 +++
src/test/regress/sql/generated.sql | 1 +
3 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 5440eb9015..f75ca74c6b 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -5891,10 +5891,36 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
if (defval)
{
NewColumnValue *newval;
+ List *varlist;
+ ListCell *lc;
+ Expr *newexpr;
+
+ newexpr = expression_planner(defval);
+
+ /*
+ * The expression may reference columns added by concurrent
+ * subcommands, which leads to internal error on table
+ * rewrite. Check them out here to emit meaningful error message.
+ */
+ varlist = pull_var_clause((Node *) newexpr, 0);
+ foreach (lc, varlist)
+ {
+ Var *var = (Var *) lfirst(lc);
+
+ /* System columns are rejected ealier. */
+ Assert(var->varattno > 0);
+
+ if (var->varattno > tab->oldDesc->natts)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
+ errmsg ("column \"%s\" does not exist",
+ NameStr(TupleDescAttr(rel->rd_att, var->varattno - 1)->attname)),
+ errdetail("An expression cannot reference columns added in the same command.")));
+ }
newval = (NewColumnValue *) palloc0(sizeof(NewColumnValue));
newval->attnum = attribute.attnum;
- newval->expr = expression_planner(defval);
+ newval->expr = newexpr;
tab->newvals = lappend(tab->newvals, newval);
}
diff --git a/src/test/regress/expected/generated.out b/src/test/regress/expected/generated.out
index f62c93f468..b5e58991bd 100644
--- a/src/test/regress/expected/generated.out
+++ b/src/test/regress/expected/generated.out
@@ -593,6 +593,9 @@ 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 x int, ADD COLUMN y int GENERATED ALWAYS AS (x * 4) STORED; -- error
+ERROR: column "x" does not exist
+DETAIL: An expression cannot reference columns added in the same command.
-- 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 6a56ae260f..3b00f81b54 100644
--- a/src/test/regress/sql/generated.sql
+++ b/src/test/regress/sql/generated.sql
@@ -316,6 +316,7 @@ 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 x int, ADD COLUMN y int GENERATED ALWAYS AS (x * 4) STORED; -- error
-- ALTER TABLE ... ALTER COLUMN
CREATE TABLE gtest27 (
--
2.23.0
Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:
> At Tue, 26 Nov 2019 10:49:11 -0500, Tom Lane <tgl@sss.pgh.pa.us> wrote in
>> Hmm, interesting. IMO, that *should* have thrown an error, but of
>> course not that one. The ADD COLUMN operations are all processed
>> in parallel, so it's not okay for one of them to have a GENERATED
>> expression that refers to another one of the new columns. But you
>> should have gotten a "no such column" type of error, not a run-time
>> cross-check failure.
> Something like this works?
I started to look at this, but it felt a bit brute-force to me.
After awhile I began to think that my offhand comment above was
wrong --- why *shouldn't* this case work? When we insert or
update a tuple, we expect that GENERATED columns should be
computed based on the new tuple values, so why is the executor
evidently evaluating them based on the old tuple?
That thought soon led me to realize that there's an adjacent
bug that this patch fails to fix:
regression=# create table foo (f1 int);
CREATE TABLE
regression=# insert into foo values(1),(2);
INSERT 0 2
regression=# alter table foo alter column f1 type float8, add column f2 int generated always as (f1 * 2) stored;
ERROR: attribute 1 of type foo has wrong type
DETAIL: Table has type integer, but query expects double precision.
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.
regards, tom lane
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 (