pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #16958: "Invalid reference to FROM-clause entry for table" when qualifying columns in "on conflict .. where"
6+ messages / 5 participants
[nested] [flat]
Commits from this thread:
2021-04-13 6c0373ab77 Allow table-qualified variable names in ON CONFLICT ... WHERE.
* BUG #16958: "Invalid reference to FROM-clause entry for table" when qualifying columns in "on conflict .. where"
@ 2021-04-09 21:31 PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 6+ messages in thread
From: PG Bug reporting form @ 2021-04-09 21:31 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: lukas.eder@gmail.com
The following bug has been logged on the website:
Bug reference: 16958
Logged by: Lukas Eder
Email address: lukas.eder@gmail.com
PostgreSQL version: 13.2
Operating system: Docker on Windows
Description:
I'm using Docker on Windows:
-------------------------------------------------------------------
select version();
-------------------------------------------------------------------
PostgreSQL 13.2 (Debian 13.2-1.pgdg100+1) on x86_64-pc-linux-gnu, compiled
by gcc (Debian 8.3.0-6) 8.3.0, 64-bit
Consider the following SQL script:
-------------------------------------------------------------------
create table t (
a int,
b int,
c int
);
create unique index i on t(a)
where t.b is null;
insert into t (a, b)
values (
1,
1
)
on conflict (a)
where t.b is null -- Error here
do update
set
c = 1
returning t.a, t.b, t.c;
drop table t;
-------------------------------------------------------------------
There's an error reported:
SQL Error [42P01]: ERROR: invalid reference to FROM-clause entry for table
"t"
Hint: There is an entry for table "t", but it cannot be referenced from
this part of the query.
Position: 71
Notice the qualification of the t.b column in the "on conflict .. where"
clause. I don't understand why b cannot be qualified at this location. It
can be qualified in the index definition, looks like a bug to me.
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: BUG #16958: "Invalid reference to FROM-clause entry for table" when qualifying columns in "on conflict .. where"
@ 2021-04-10 07:57 Pantelis Theodosiou <ypercube@gmail.com>
parent: PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 6+ messages in thread
From: Pantelis Theodosiou @ 2021-04-10 07:57 UTC (permalink / raw)
To: Lukas Eder <lukas.eder@gmail.com>; pgsql-bugs@lists.postgresql.org
On Fri, Apr 9, 2021 at 11:00 PM PG Bug reporting form <
noreply@postgresql.org> wrote:
> The following bug has been logged on the website:
>
> Bug reference: 16958
> Logged by: Lukas Eder
> Email address: lukas.eder@gmail.com
> PostgreSQL version: 13.2
> Operating system: Docker on Windows
> Description:
>
> I'm using Docker on Windows:
>
> -------------------------------------------------------------------
> select version();
> -------------------------------------------------------------------
> PostgreSQL 13.2 (Debian 13.2-1.pgdg100+1) on x86_64-pc-linux-gnu, compiled
> by gcc (Debian 8.3.0-6) 8.3.0, 64-bit
>
> Consider the following SQL script:
> -------------------------------------------------------------------
> create table t (
> a int,
> b int,
> c int
> );
>
> create unique index i on t(a)
> where t.b is null;
>
> insert into t (a, b)
> values (
> 1,
> 1
> )
> on conflict (a)
> where t.b is null -- Error here
> do update
> set
> c = 1
> returning t.a, t.b, t.c;
>
> drop table t;
> -------------------------------------------------------------------
>
> There's an error reported:
>
> SQL Error [42P01]: ERROR: invalid reference to FROM-clause entry for table
> "t"
> Hint: There is an entry for table "t", but it cannot be referenced from
> this part of the query.
> Position: 71
>
> Notice the qualification of the t.b column in the "on conflict .. where"
> clause. I don't understand why b cannot be qualified at this location. It
> can be qualified in the index definition, looks like a bug to me.
>
You don't need and shouldn't prefix the column with the table name in that
part. This should work:
...
on conflict (a)
where b is null
do update
...
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: BUG #16958: "Invalid reference to FROM-clause entry for table" when qualifying columns in "on conflict .. where"
@ 2021-04-10 10:28 Vik Fearing <vik@postgresfriends.org>
parent: Pantelis Theodosiou <ypercube@gmail.com>
0 siblings, 1 reply; 6+ messages in thread
From: Vik Fearing @ 2021-04-10 10:28 UTC (permalink / raw)
To: Pantelis Theodosiou <ypercube@gmail.com>; Lukas Eder <lukas.eder@gmail.com>; pgsql-bugs@lists.postgresql.org
On 4/10/21 9:57 AM, Pantelis Theodosiou wrote:
> On Fri, Apr 9, 2021 at 11:00 PM PG Bug reporting form <
> noreply@postgresql.org> wrote:
>
>> insert into t (a, b)
>> values (
>> 1,
>> 1
>> )
>> on conflict (a)
>> where t.b is null -- Error here
>> do update
>> set
>> c = 1
>> returning t.a, t.b, t.c;
>> There's an error reported:
>>
>> SQL Error [42P01]: ERROR: invalid reference to FROM-clause entry for table
>> "t"
>> Hint: There is an entry for table "t", but it cannot be referenced from
>> this part of the query.
>> Position: 71
>>
>> Notice the qualification of the t.b column in the "on conflict .. where"
>> clause. I don't understand why b cannot be qualified at this location. It
>> can be qualified in the index definition, looks like a bug to me.
>>
>
> You don't need and shouldn't prefix the column with the table name in that
> part. This should work:
>
> ...
> on conflict (a)
> where b is null
> do update
> ...
Not needing to and not being able to are two completely different
things. I believe tablename qualification should be allowed here even
if it isn't necessary.
--
Vik Fearing
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: BUG #16958: "Invalid reference to FROM-clause entry for table" when qualifying columns in "on conflict .. where"
@ 2021-04-10 16:28 Tom Lane <tgl@sss.pgh.pa.us>
parent: Vik Fearing <vik@postgresfriends.org>
0 siblings, 1 reply; 6+ messages in thread
From: Tom Lane @ 2021-04-10 16:28 UTC (permalink / raw)
To: Vik Fearing <vik@postgresfriends.org>; +Cc: Peter Geoghegan <pg@bowt.ie>; Pantelis Theodosiou <ypercube@gmail.com>; Lukas Eder <lukas.eder@gmail.com>; pgsql-bugs@lists.postgresql.org
Vik Fearing <vik@postgresfriends.org> writes:
> On 4/10/21 9:57 AM, Pantelis Theodosiou wrote:
>> On Fri, Apr 9, 2021 at 11:00 PM PG Bug reporting form <
>> noreply@postgresql.org> wrote:
>>> SQL Error [42P01]: ERROR: invalid reference to FROM-clause entry for table "t"
>>> Hint: There is an entry for table "t", but it cannot be referenced from
>>> this part of the query.
>>>
>>> Notice the qualification of the t.b column in the "on conflict .. where"
>>> clause. I don't understand why b cannot be qualified at this location. It
>>> can be qualified in the index definition, looks like a bug to me.
>> You don't need and shouldn't prefix the column with the table name in that
>> part. This should work:
> Not needing to and not being able to are two completely different
> things. I believe tablename qualification should be allowed here even
> if it isn't necessary.
It does seem like a pointless prohibition, but the comment about it in
the source code implies it was intentional. Peter, do you remember
why?
Anyway, if the prohibition isn't necessary, the attached one-liner
gets most of the way to removing it. I'm not quite satisfied with
this though:
regression=# create table t(a int primary key, b int, c int);
CREATE TABLE
regression=# insert into t (a,b) values(1,1) on conflict(a)
where excluded.b is null do update set c=excluded.c+1;
ERROR: missing FROM-clause entry for table "excluded"
LINE 2: where excluded.b is null do update set c=excluded.c+1;
^
Seems like we want the same cannot-be-referenced HINT here as in
the complaint, so just nuking the whole namespace as the code
currently does ought to be revisited.
regards, tom lane
Attachments:
allow-table-qual-in-ON-CONFLICT-wip.patch (text/x-diff, 843B) download
diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index af80aa4593..4402be6845 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -3225,13 +3225,13 @@ transformOnConflictArbiter(ParseState *pstate,
List *save_namespace;
/*
- * While we process the arbiter expressions, accept only non-qualified
- * references to the target table. Hide any other relations.
+ * While we process the arbiter expressions, accept only references to
+ * the target table. Hide any other relations.
*/
save_namespace = pstate->p_namespace;
pstate->p_namespace = NIL;
addNSItemToQuery(pstate, pstate->p_target_nsitem,
- false, false, true);
+ false, true, true);
if (infer->indexElems)
*arbiterExpr = resolve_unique_index_expr(pstate, infer,
* Re: BUG #16958: "Invalid reference to FROM-clause entry for table" when qualifying columns in "on conflict .. where"
@ 2021-04-10 16:54 Peter Geoghegan <pg@bowt.ie>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 6+ messages in thread
From: Peter Geoghegan @ 2021-04-10 16:54 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Vik Fearing <vik@postgresfriends.org>; Pantelis Theodosiou <ypercube@gmail.com>; Lukas Eder <lukas.eder@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Sat, Apr 10, 2021 at 9:28 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> It does seem like a pointless prohibition, but the comment about it in
> the source code implies it was intentional. Peter, do you remember
> why?
No. The intention was to make it like CREATE INDEX. Apparently CREATE
INDEX allows the columns to be qualified, though, so that explanation
doesn't justify it.
There might have been a concern about users being confused about the
difference between what the INSERT docs call 'index_predicate' and
what they call 'condition' in the synopsis.
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: BUG #16958: "Invalid reference to FROM-clause entry for table" when qualifying columns in "on conflict .. where"
@ 2021-04-11 00:20 Tom Lane <tgl@sss.pgh.pa.us>
parent: Peter Geoghegan <pg@bowt.ie>
0 siblings, 0 replies; 6+ messages in thread
From: Tom Lane @ 2021-04-11 00:20 UTC (permalink / raw)
To: Peter Geoghegan <pg@bowt.ie>; +Cc: Vik Fearing <vik@postgresfriends.org>; Pantelis Theodosiou <ypercube@gmail.com>; Lukas Eder <lukas.eder@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Peter Geoghegan <pg@bowt.ie> writes:
> On Sat, Apr 10, 2021 at 9:28 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> It does seem like a pointless prohibition, but the comment about it in
>> the source code implies it was intentional. Peter, do you remember
>> why?
> No. The intention was to make it like CREATE INDEX. Apparently CREATE
> INDEX allows the columns to be qualified, though, so that explanation
> doesn't justify it.
OK. Here's a more fleshed-out patch that makes an effort to get rid of
duplicate namespace-munging.
I found that creating the EXCLUDED RTE soon enough to draw the complaint
I wanted about bogus references also caused this interesting change in
the regression test results:
ERROR: column "keyy" does not exist
LINE 1: ...nsertconflicttest values (1, 'Apple') on conflict (keyy) do ...
^
-HINT: Perhaps you meant to reference the column "insertconflicttest.key".
+HINT: Perhaps you meant to reference the column "insertconflicttest.key" or the column "excluded.key".
-- Have useful HINT for EXCLUDED.* RTE within UPDATE:
This seems to me like an independent bug: why is the misspelled-column-
name hint machinery including inaccessible columns in its results?
That seems much more likely to be confusing than helpful. But if there
is actually a defensible reason for doing that, then this seems like
an okay change.
regards, tom lane
Attachments:
allow-table-qual-in-ON-CONFLICT-1.patch (text/x-diff, 9.0K) download
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 9f13880d19..862f18a92f 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -869,25 +869,27 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
attr_num - FirstLowInvalidHeapAttributeNumber);
}
- /* Process ON CONFLICT, if any. */
- if (stmt->onConflictClause)
- qry->onConflict = transformOnConflictClause(pstate,
- stmt->onConflictClause);
-
/*
- * If we have a RETURNING clause, we need to add the target relation to
- * the query namespace before processing it, so that Var references in
- * RETURNING will work. Also, remove any namespace entries added in a
+ * If we have any clauses yet to process, set the query namespace to
+ * contain only the target relation, removing any entries added in a
* sub-SELECT or VALUES list.
*/
- if (stmt->returningList)
+ if (stmt->onConflictClause || stmt->returningList)
{
pstate->p_namespace = NIL;
addNSItemToQuery(pstate, pstate->p_target_nsitem,
false, true, true);
+ }
+
+ /* Process ON CONFLICT, if any. */
+ if (stmt->onConflictClause)
+ qry->onConflict = transformOnConflictClause(pstate,
+ stmt->onConflictClause);
+
+ /* Process RETURNING, if any. */
+ if (stmt->returningList)
qry->returningList = transformReturningList(pstate,
stmt->returningList);
- }
/* done building the range table and jointree */
qry->rtable = pstate->p_rtable;
@@ -1014,6 +1016,7 @@ static OnConflictExpr *
transformOnConflictClause(ParseState *pstate,
OnConflictClause *onConflictClause)
{
+ ParseNamespaceItem *exclNSItem = NULL;
List *arbiterElems;
Node *arbiterWhere;
Oid arbiterConstraint;
@@ -1023,29 +1026,17 @@ transformOnConflictClause(ParseState *pstate,
List *exclRelTlist = NIL;
OnConflictExpr *result;
- /* Process the arbiter clause, ON CONFLICT ON (...) */
- transformOnConflictArbiter(pstate, onConflictClause, &arbiterElems,
- &arbiterWhere, &arbiterConstraint);
-
- /* Process DO UPDATE */
+ /*
+ * If this is ON CONFLICT ... UPDATE, first create the range table entry
+ * for the EXCLUDED pseudo relation, so that that will be present while
+ * processing arbiter expressions. (You can't actually reference it from
+ * there, but this provides a useful error message if you try.)
+ */
if (onConflictClause->action == ONCONFLICT_UPDATE)
{
Relation targetrel = pstate->p_target_relation;
- ParseNamespaceItem *exclNSItem;
RangeTblEntry *exclRte;
- /*
- * All INSERT expressions have been parsed, get ready for potentially
- * existing SET statements that need to be processed like an UPDATE.
- */
- pstate->p_is_insert = false;
-
- /*
- * Add range table entry for the EXCLUDED pseudo relation. relkind is
- * set to composite to signal that we're not dealing with an actual
- * relation, and no permission checks are required on it. (We'll
- * check the actual target relation, instead.)
- */
exclNSItem = addRangeTableEntryForRelation(pstate,
targetrel,
RowExclusiveLock,
@@ -1054,6 +1045,11 @@ transformOnConflictClause(ParseState *pstate,
exclRte = exclNSItem->p_rte;
exclRelIndex = exclNSItem->p_rtindex;
+ /*
+ * relkind is set to composite to signal that we're not dealing with
+ * an actual relation, and no permission checks are required on it.
+ * (We'll check the actual target relation, instead.)
+ */
exclRte->relkind = RELKIND_COMPOSITE_TYPE;
exclRte->requiredPerms = 0;
/* other permissions fields in exclRte are already empty */
@@ -1061,14 +1057,27 @@ transformOnConflictClause(ParseState *pstate,
/* Create EXCLUDED rel's targetlist for use by EXPLAIN */
exclRelTlist = BuildOnConflictExcludedTargetlist(targetrel,
exclRelIndex);
+ }
+
+ /* Process the arbiter clause, ON CONFLICT ON (...) */
+ transformOnConflictArbiter(pstate, onConflictClause, &arbiterElems,
+ &arbiterWhere, &arbiterConstraint);
+
+ /* Process DO UPDATE */
+ if (onConflictClause->action == ONCONFLICT_UPDATE)
+ {
+ /*
+ * Expressions in the UPDATE targetlist need to be handled like UPDATE
+ * not INSERT. We don't need to save/restore this because all INSERT
+ * expressions have been parsed already.
+ */
+ pstate->p_is_insert = false;
/*
- * Add EXCLUDED and the target RTE to the namespace, so that they can
- * be used in the UPDATE subexpressions.
+ * Add the EXCLUDED pseudo relation to the query namespace, making it
+ * available in the UPDATE subexpressions.
*/
addNSItemToQuery(pstate, exclNSItem, false, true, true);
- addNSItemToQuery(pstate, pstate->p_target_nsitem,
- false, true, true);
/*
* Now transform the UPDATE subexpressions.
@@ -1079,6 +1088,14 @@ transformOnConflictClause(ParseState *pstate,
onConflictWhere = transformWhereClause(pstate,
onConflictClause->whereClause,
EXPR_KIND_WHERE, "WHERE");
+
+ /*
+ * Remove the EXCLUDED pseudo relation from the query namespace, since
+ * it's not supposed to be available in RETURNING. (Maybe someday we
+ * could allow that, and drop this step.)
+ */
+ Assert((ParseNamespaceItem *) llast(pstate->p_namespace) == exclNSItem);
+ pstate->p_namespace = list_delete_last(pstate->p_namespace);
}
/* Finally, build ON CONFLICT DO [NOTHING | UPDATE] expression */
diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index af80aa4593..89d95d3e94 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -3222,17 +3222,6 @@ transformOnConflictArbiter(ParseState *pstate,
/* ON CONFLICT DO NOTHING does not require an inference clause */
if (infer)
{
- List *save_namespace;
-
- /*
- * While we process the arbiter expressions, accept only non-qualified
- * references to the target table. Hide any other relations.
- */
- save_namespace = pstate->p_namespace;
- pstate->p_namespace = NIL;
- addNSItemToQuery(pstate, pstate->p_target_nsitem,
- false, false, true);
-
if (infer->indexElems)
*arbiterExpr = resolve_unique_index_expr(pstate, infer,
pstate->p_target_relation);
@@ -3245,8 +3234,6 @@ transformOnConflictArbiter(ParseState *pstate,
*arbiterWhere = transformExpr(pstate, infer->whereClause,
EXPR_KIND_INDEX_PREDICATE);
- pstate->p_namespace = save_namespace;
-
/*
* If the arbiter is specified by constraint name, get the constraint
* OID and mark the constrained columns as requiring SELECT privilege,
diff --git a/src/test/regress/expected/insert_conflict.out b/src/test/regress/expected/insert_conflict.out
index a54a51e5c7..66d8633e3e 100644
--- a/src/test/regress/expected/insert_conflict.out
+++ b/src/test/regress/expected/insert_conflict.out
@@ -248,7 +248,7 @@ insert into insertconflicttest values (1, 'Apple') on conflict (keyy) do update
ERROR: column "keyy" does not exist
LINE 1: ...nsertconflicttest values (1, 'Apple') on conflict (keyy) do ...
^
-HINT: Perhaps you meant to reference the column "insertconflicttest.key".
+HINT: Perhaps you meant to reference the column "insertconflicttest.key" or the column "excluded.key".
-- Have useful HINT for EXCLUDED.* RTE within UPDATE:
insert into insertconflicttest values (1, 'Apple') on conflict (key) do update set fruit = excluded.fruitt;
ERROR: column excluded.fruitt does not exist
@@ -373,7 +373,7 @@ drop index fruit_index;
create unique index partial_key_index on insertconflicttest(key) where fruit like '%berry';
-- Succeeds
insert into insertconflicttest values (23, 'Blackberry') on conflict (key) where fruit like '%berry' do update set fruit = excluded.fruit;
-insert into insertconflicttest values (23, 'Blackberry') on conflict (key) where fruit like '%berry' and fruit = 'inconsequential' do nothing;
+insert into insertconflicttest as t values (23, 'Blackberry') on conflict (key) where fruit like '%berry' and t.fruit = 'inconsequential' do nothing;
-- fails
insert into insertconflicttest values (23, 'Blackberry') on conflict (key) do update set fruit = excluded.fruit;
ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification
diff --git a/src/test/regress/sql/insert_conflict.sql b/src/test/regress/sql/insert_conflict.sql
index 43691cd335..23d5778b82 100644
--- a/src/test/regress/sql/insert_conflict.sql
+++ b/src/test/regress/sql/insert_conflict.sql
@@ -214,7 +214,7 @@ create unique index partial_key_index on insertconflicttest(key) where fruit lik
-- Succeeds
insert into insertconflicttest values (23, 'Blackberry') on conflict (key) where fruit like '%berry' do update set fruit = excluded.fruit;
-insert into insertconflicttest values (23, 'Blackberry') on conflict (key) where fruit like '%berry' and fruit = 'inconsequential' do nothing;
+insert into insertconflicttest as t values (23, 'Blackberry') on conflict (key) where fruit like '%berry' and t.fruit = 'inconsequential' do nothing;
-- fails
insert into insertconflicttest values (23, 'Blackberry') on conflict (key) do update set fruit = excluded.fruit;
end of thread, other threads:[~2021-04-11 00:20 UTC | newest]
Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-04-09 21:31 BUG #16958: "Invalid reference to FROM-clause entry for table" when qualifying columns in "on conflict .. where" PG Bug reporting form <noreply@postgresql.org>
2021-04-10 07:57 ` Pantelis Theodosiou <ypercube@gmail.com>
2021-04-10 10:28 ` Vik Fearing <vik@postgresfriends.org>
2021-04-10 16:28 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-04-10 16:54 ` Peter Geoghegan <pg@bowt.ie>
2021-04-11 00:20 ` Tom Lane <tgl@sss.pgh.pa.us>
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