From 0c051c90082da0b7e5bcaf9aabcbd4f361137cdc Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 7 Jan 2014 15:25:16 -0500 Subject: [PATCH] Fix LATERAL references to target table of UPDATE/DELETE. I failed to think much about UPDATE/DELETE when implementing LATERAL :-(. The implemented behavior ended up being that subqueries in the FROM or USING clause (respectively) could access the update/delete target table as though it were a lateral reference; which seems fine if they said LATERAL, but certainly ought to draw an error if they didn't. Fix it so you get a suitable error when you omit LATERAL. Per report from Emre Hasegeli. --- src/backend/parser/analyze.c | 21 +++++++++++++ src/backend/parser/parse_clause.c | 4 +++ src/backend/parser/parse_relation.c | 4 +++ src/test/regress/expected/join.out | 48 +++++++++++++++++++++++++++++ src/test/regress/sql/join.sql | 18 +++++++++++ 5 files changed, 95 insertions(+) diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index 60cce37845..6560b86e4b 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -342,6 +342,7 @@ static Query * transformDeleteStmt(ParseState *pstate, DeleteStmt *stmt) { Query *qry = makeNode(Query); + ParseNamespaceItem *nsitem; Node *qual; qry->commandType = CMD_DELETE; @@ -360,8 +361,15 @@ transformDeleteStmt(ParseState *pstate, DeleteStmt *stmt) true, ACL_DELETE); + /* grab the namespace item made by setTargetTable */ + nsitem = (ParseNamespaceItem *) llast(pstate->p_namespace); + + /* there's no DISTINCT in DELETE */ qry->distinctClause = NIL; + /* subqueries in USING can see the result relation only via LATERAL */ + nsitem->p_lateral_only = true; + /* * The USING clause is non-standard SQL syntax, and is equivalent in * functionality to the FROM list that can be specified for UPDATE. The @@ -370,6 +378,9 @@ transformDeleteStmt(ParseState *pstate, DeleteStmt *stmt) */ transformFromClause(pstate, stmt->usingClause); + /* remaining clauses can see the result relation normally */ + nsitem->p_lateral_only = false; + qual = transformWhereClause(pstate, stmt->whereClause, EXPR_KIND_WHERE, "WHERE"); @@ -1889,6 +1900,7 @@ static Query * transformUpdateStmt(ParseState *pstate, UpdateStmt *stmt) { Query *qry = makeNode(Query); + ParseNamespaceItem *nsitem; RangeTblEntry *target_rte; Node *qual; ListCell *origTargetList; @@ -1910,12 +1922,21 @@ transformUpdateStmt(ParseState *pstate, UpdateStmt *stmt) true, ACL_UPDATE); + /* grab the namespace item made by setTargetTable */ + nsitem = (ParseNamespaceItem *) llast(pstate->p_namespace); + + /* subqueries in FROM can see the result relation only via LATERAL */ + nsitem->p_lateral_only = true; + /* * the FROM clause is non-standard SQL syntax. We used to be able to do * this with REPLACE in POSTQUEL so we keep the feature. */ transformFromClause(pstate, stmt->fromClause); + /* remaining clauses can see the result relation normally */ + nsitem->p_lateral_only = false; + qry->targetList = transformTargetList(pstate, stmt->targetList, EXPR_KIND_UPDATE_SOURCE); diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c index 05ddb8c3e7..51db595641 100644 --- a/src/backend/parser/parse_clause.c +++ b/src/backend/parser/parse_clause.c @@ -204,6 +204,10 @@ setTargetTable(ParseState *pstate, RangeVar *relation, /* * If UPDATE/DELETE, add table to joinlist and namespace. + * + * Note: some callers know that they can find the new ParseNamespaceItem + * at the end of the pstate->p_namespace list. This is a bit ugly but not + * worth complicating this function's signature for. */ if (alsoSource) addRTEtoQuery(pstate, rte, true, true, true); diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c index cd8d75e23d..a7f563b1b1 100644 --- a/src/backend/parser/parse_relation.c +++ b/src/backend/parser/parse_relation.c @@ -1730,6 +1730,10 @@ isLockedRefname(ParseState *pstate, const char *refname) * and/or namespace list. (We assume caller has checked for any * namespace conflicts.) The RTE is always marked as unconditionally * visible, that is, not LATERAL-only. + * + * Note: some callers know that they can find the new ParseNamespaceItem + * at the end of the pstate->p_namespace list. This is a bit ugly but not + * worth complicating this function's signature for. */ void addRTEtoQuery(ParseState *pstate, RangeTblEntry *rte, diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index 83cbde82c6..9adfac5ef6 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -4103,3 +4103,51 @@ select 1 from tenk1 a, lateral (select max(a.unique1) from int4_tbl b) ss; ERROR: aggregate functions are not allowed in FROM clause of their own query level LINE 1: select 1 from tenk1 a, lateral (select max(a.unique1) from i... ^ +-- check behavior of LATERAL in UPDATE/DELETE +create temp table xx1 as select f1 as x1, -f1 as x2 from int4_tbl; +select * from xx1; + x1 | x2 +-------------+------------- + 0 | 0 + 123456 | -123456 + -123456 | 123456 + 2147483647 | -2147483647 + -2147483647 | 2147483647 +(5 rows) + +-- error, can't do this without LATERAL: +update xx1 set x2 = f1 from (select * from int4_tbl where f1 = x1) ss; +ERROR: column "x1" does not exist +LINE 1: ... set x2 = f1 from (select * from int4_tbl where f1 = x1) ss; + ^ +HINT: There is a column named "x1" in table "xx1", but it cannot be referenced from this part of the query. +update xx1 set x2 = f1 from (select * from int4_tbl where f1 = xx1.x1) ss; +ERROR: invalid reference to FROM-clause entry for table "xx1" +LINE 1: ...t x2 = f1 from (select * from int4_tbl where f1 = xx1.x1) ss... + ^ +HINT: There is an entry for table "xx1", but it cannot be referenced from this part of the query. +-- OK: +update xx1 set x2 = f1 from lateral (select * from int4_tbl where f1 = x1) ss; +select * from xx1; + x1 | x2 +-------------+------------- + 0 | 0 + 123456 | 123456 + -123456 | -123456 + 2147483647 | 2147483647 + -2147483647 | -2147483647 +(5 rows) + +-- error: +delete from xx1 using (select * from int4_tbl where f1 = x1) ss; +ERROR: column "x1" does not exist +LINE 1: ...te from xx1 using (select * from int4_tbl where f1 = x1) ss; + ^ +HINT: There is a column named "x1" in table "xx1", but it cannot be referenced from this part of the query. +-- OK: +delete from xx1 using lateral (select * from int4_tbl where f1 = x1) ss; +select * from xx1; + x1 | x2 +----+---- +(0 rows) + diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql index 2168c55722..e2bf915be0 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -1147,3 +1147,21 @@ select * from int8_tbl x cross join (int4_tbl x cross join lateral (select x.f1) ss); -- LATERAL can be used to put an aggregate into the FROM clause of its query select 1 from tenk1 a, lateral (select max(a.unique1) from int4_tbl b) ss; + +-- check behavior of LATERAL in UPDATE/DELETE + +create temp table xx1 as select f1 as x1, -f1 as x2 from int4_tbl; +select * from xx1; + +-- error, can't do this without LATERAL: +update xx1 set x2 = f1 from (select * from int4_tbl where f1 = x1) ss; +update xx1 set x2 = f1 from (select * from int4_tbl where f1 = xx1.x1) ss; +-- OK: +update xx1 set x2 = f1 from lateral (select * from int4_tbl where f1 = x1) ss; +select * from xx1; + +-- error: +delete from xx1 using (select * from int4_tbl where f1 = x1) ss; +-- OK: +delete from xx1 using lateral (select * from int4_tbl where f1 = x1) ss; +select * from xx1;