From 158b7fa6a34006bdc70b515e14e120d3e896589b Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 11 Jan 2014 19:03:12 -0500 Subject: [PATCH] Disallow LATERAL references to the target table of an UPDATE/DELETE. On second thought, commit 0c051c90082da0b7e5bcaf9aabcbd4f361137cdc was over-hasty: rather than allowing this case, we ought to reject it for now. That leaves the field clear for a future feature that allows the target table to be re-specified in the FROM (or USING) clause, which will enable left-joining the target table to something else. We can then also allow LATERAL references to such an explicitly re-specified target table. But allowing them right now will create ambiguities or worse for such a feature, and it isn't something we documented 9.3 as supporting. While at it, add a convenience subroutine to avoid having several copies of the ereport for disalllowed-LATERAL-reference cases. --- src/backend/parser/analyze.c | 12 ++++-- src/backend/parser/parse_relation.c | 62 +++++++++++++++++------------ src/include/parser/parse_node.h | 3 +- src/test/regress/expected/join.out | 48 +++++++++------------- src/test/regress/sql/join.sql | 13 +++--- 5 files changed, 72 insertions(+), 66 deletions(-) diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index 0d6e661ec4..7225bb62ab 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -367,8 +367,9 @@ transformDeleteStmt(ParseState *pstate, DeleteStmt *stmt) /* there's no DISTINCT in DELETE */ qry->distinctClause = NIL; - /* subqueries in USING can see the result relation only via LATERAL */ + /* subqueries in USING cannot access the result relation */ nsitem->p_lateral_only = true; + nsitem->p_lateral_ok = false; /* * The USING clause is non-standard SQL syntax, and is equivalent in @@ -378,8 +379,9 @@ transformDeleteStmt(ParseState *pstate, DeleteStmt *stmt) */ transformFromClause(pstate, stmt->usingClause); - /* remaining clauses can see the result relation normally */ + /* remaining clauses can reference the result relation normally */ nsitem->p_lateral_only = false; + nsitem->p_lateral_ok = true; qual = transformWhereClause(pstate, stmt->whereClause, EXPR_KIND_WHERE, "WHERE"); @@ -1925,8 +1927,9 @@ transformUpdateStmt(ParseState *pstate, UpdateStmt *stmt) /* grab the namespace item made by setTargetTable */ nsitem = (ParseNamespaceItem *) llast(pstate->p_namespace); - /* subqueries in FROM can see the result relation only via LATERAL */ + /* subqueries in FROM cannot access the result relation */ nsitem->p_lateral_only = true; + nsitem->p_lateral_ok = false; /* * the FROM clause is non-standard SQL syntax. We used to be able to do @@ -1934,8 +1937,9 @@ transformUpdateStmt(ParseState *pstate, UpdateStmt *stmt) */ transformFromClause(pstate, stmt->fromClause); - /* remaining clauses can see the result relation normally */ + /* remaining clauses can reference the result relation normally */ nsitem->p_lateral_only = false; + nsitem->p_lateral_ok = true; qry->targetList = transformTargetList(pstate, stmt->targetList, EXPR_KIND_UPDATE_SOURCE); diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c index c904ea317e..8760952dfe 100644 --- a/src/backend/parser/parse_relation.c +++ b/src/backend/parser/parse_relation.c @@ -37,6 +37,8 @@ static RangeTblEntry *scanNameSpaceForRefname(ParseState *pstate, const char *refname, int location); static RangeTblEntry *scanNameSpaceForRelid(ParseState *pstate, Oid relid, int location); +static void check_lateral_ref_ok(ParseState *pstate, ParseNamespaceItem *nsitem, + int location); static void markRTEForSelectPriv(ParseState *pstate, RangeTblEntry *rte, int rtindex, AttrNumber col); static void expandRelation(Oid relid, Alias *eref, @@ -170,14 +172,7 @@ scanNameSpaceForRefname(ParseState *pstate, const char *refname, int location) errmsg("table reference \"%s\" is ambiguous", refname), parser_errposition(pstate, location))); - /* SQL:2008 demands this be an error, not an invisible item */ - if (nsitem->p_lateral_only && !nsitem->p_lateral_ok) - ereport(ERROR, - (errcode(ERRCODE_INVALID_COLUMN_REFERENCE), - errmsg("invalid reference to FROM-clause entry for table \"%s\"", - refname), - errdetail("The combining JOIN type must be INNER or LEFT for a LATERAL reference."), - parser_errposition(pstate, location))); + check_lateral_ref_ok(pstate, nsitem, location); result = rte; } } @@ -221,14 +216,7 @@ scanNameSpaceForRelid(ParseState *pstate, Oid relid, int location) errmsg("table reference %u is ambiguous", relid), parser_errposition(pstate, location))); - /* SQL:2008 demands this be an error, not an invisible item */ - if (nsitem->p_lateral_only && !nsitem->p_lateral_ok) - ereport(ERROR, - (errcode(ERRCODE_INVALID_COLUMN_REFERENCE), - errmsg("invalid reference to FROM-clause entry for table \"%s\"", - rte->eref->aliasname), - errdetail("The combining JOIN type must be INNER or LEFT for a LATERAL reference."), - parser_errposition(pstate, location))); + check_lateral_ref_ok(pstate, nsitem, location); result = rte; } } @@ -410,6 +398,37 @@ checkNameSpaceConflicts(ParseState *pstate, List *namespace1, } } +/* + * Complain if a namespace item is currently disallowed as a LATERAL reference. + * This enforces both SQL:2008's rather odd idea of what to do with a LATERAL + * reference to the wrong side of an outer join, and our own prohibition on + * referencing the target table of an UPDATE or DELETE as a lateral reference + * in a FROM/USING clause. + * + * Convenience subroutine to avoid multiple copies of a rather ugly ereport. + */ +static void +check_lateral_ref_ok(ParseState *pstate, ParseNamespaceItem *nsitem, + int location) +{ + if (nsitem->p_lateral_only && !nsitem->p_lateral_ok) + { + /* SQL:2008 demands this be an error, not an invisible item */ + RangeTblEntry *rte = nsitem->p_rte; + char *refname = rte->eref->aliasname; + + ereport(ERROR, + (errcode(ERRCODE_INVALID_COLUMN_REFERENCE), + errmsg("invalid reference to FROM-clause entry for table \"%s\"", + refname), + (rte == pstate->p_target_rangetblentry) ? + errhint("There is an entry for table \"%s\", but it cannot be referenced from this part of the query.", + refname) : + errdetail("The combining JOIN type must be INNER or LEFT for a LATERAL reference."), + parser_errposition(pstate, location))); + } +} + /* * given an RTE, return RT index (starting with 1) of the entry, * and optionally get its nesting depth (0 = current). If sublevels_up @@ -632,15 +651,8 @@ colNameToVar(ParseState *pstate, char *colname, bool localonly, (errcode(ERRCODE_AMBIGUOUS_COLUMN), errmsg("column reference \"%s\" is ambiguous", colname), - parser_errposition(orig_pstate, location))); - /* SQL:2008 demands this be an error, not an invisible item */ - if (nsitem->p_lateral_only && !nsitem->p_lateral_ok) - ereport(ERROR, - (errcode(ERRCODE_INVALID_COLUMN_REFERENCE), - errmsg("invalid reference to FROM-clause entry for table \"%s\"", - rte->eref->aliasname), - errdetail("The combining JOIN type must be INNER or LEFT for a LATERAL reference."), - parser_errposition(orig_pstate, location))); + parser_errposition(pstate, location))); + check_lateral_ref_ok(pstate, nsitem, location); result = newresult; } } diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h index 8ce183e8cc..85598e8783 100644 --- a/src/include/parser/parse_node.h +++ b/src/include/parser/parse_node.h @@ -186,7 +186,8 @@ struct ParseState * inside such a subexpression at the moment.) If p_lateral_ok is not set, * it's an error to actually use such a namespace item. One might think it * would be better to just exclude such items from visibility, but the wording - * of SQL:2008 requires us to do it this way. + * of SQL:2008 requires us to do it this way. We also use p_lateral_ok to + * forbid LATERAL references to an UPDATE/DELETE target table. * * At no time should a namespace list contain two entries that conflict * according to the rules in checkNameSpaceConflicts; but note that those diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index 9adfac5ef6..74bc8ead26 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -4105,17 +4105,7 @@ 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: +-- error, can't do this: 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; @@ -4126,28 +4116,28 @@ 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: +-- can't do it even with LATERAL: 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: +ERROR: invalid reference to FROM-clause entry for table "xx1" +LINE 1: ...= f1 from lateral (select * from int4_tbl where f1 = x1) ss; + ^ +HINT: There is an entry for table "xx1", but it cannot be referenced from this part of the query. +-- we might in future allow something like this, but for now it's an error: +update xx1 set x2 = f1 from xx1, lateral (select * from int4_tbl where f1 = x1) ss; +ERROR: table name "xx1" specified more than once +-- also errors: 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 (select * from int4_tbl where f1 = xx1.x1) ss; +ERROR: invalid reference to FROM-clause entry for table "xx1" +LINE 1: ...from xx1 using (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. delete from xx1 using lateral (select * from int4_tbl where f1 = x1) ss; -select * from xx1; - x1 | x2 -----+---- -(0 rows) - +ERROR: invalid reference to FROM-clause entry for table "xx1" +LINE 1: ...xx1 using lateral (select * from int4_tbl where f1 = x1) ss; + ^ +HINT: There is an entry for table "xx1", but it cannot be referenced from this part of the query. diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql index e2bf915be0..409e0b13d6 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -1151,17 +1151,16 @@ 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: +-- error, can't do this: 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: +-- can't do it even with LATERAL: update xx1 set x2 = f1 from lateral (select * from int4_tbl where f1 = x1) ss; -select * from xx1; +-- we might in future allow something like this, but for now it's an error: +update xx1 set x2 = f1 from xx1, lateral (select * from int4_tbl where f1 = x1) ss; --- error: +-- also errors: delete from xx1 using (select * from int4_tbl where f1 = x1) ss; --- OK: +delete from xx1 using (select * from int4_tbl where f1 = xx1.x1) ss; delete from xx1 using lateral (select * from int4_tbl where f1 = x1) ss; -select * from xx1;