Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1oPp5u-0004SU-SR for pgsql-bugs@arkaria.postgresql.org; Sun, 21 Aug 2022 17:50:43 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1oPp5t-0002bF-Pm for pgsql-bugs@arkaria.postgresql.org; Sun, 21 Aug 2022 17:50:41 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1oPp5t-0002b6-Ei for pgsql-bugs@lists.postgresql.org; Sun, 21 Aug 2022 17:50:41 +0000 Received: from sss.pgh.pa.us ([66.207.139.130]) by magus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1oPp5r-0005sK-A4 for pgsql-bugs@lists.postgresql.org; Sun, 21 Aug 2022 17:50:41 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.15.2/8.15.2) with ESMTP id 27LHoZ1d4158935; Sun, 21 Aug 2022 13:50:35 -0400 From: Tom Lane To: Bruce Momjian cc: "David G. Johnston" , "lxndrkrlv@gmail.com" , "pgsql-bugs@lists.postgresql.org" Subject: Re: BUG #17233: Incorrect behavior of DELETE command with bad subquery in WHERE clause In-reply-to: References: <17233-afb9d806aaa64b17@postgresql.org> <1409812.1634481258@sss.pgh.pa.us> <2792105.1660859528@sss.pgh.pa.us> Comments: In-reply-to Bruce Momjian message dated "Fri, 19 Aug 2022 10:21:00 -0400" MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <4158658.1661104161.0@sss.pgh.pa.us> Date: Sun, 21 Aug 2022 13:50:35 -0400 Message-ID: <4158934.1661104235@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <4158658.1661104161.1@sss.pgh.pa.us> Bruce Momjian writes: > To improve things, it would be good if we could determine if LATERAL > will really fix the error, or at least detect one of the cases above we > have a clearer way to suggest a fix. Here's a proposed patch that tries to determine this by looking at ParseNamespaceItem flags. I'm not sure it's totally bulletproof, but it's likely good enough for a HINT. I felt that the conditional-expression nests in the existing ereport calls were nearly unintelligible already, so I rearranged the logic to duplicate portions of the ereports instead. That could be debated perhaps. Also, as written some paths through errorMissingColumn will invoke the findNSItemForRTE search twice. I'm not too fussed about that: it's a pretty cheap search and anyway nobody should be bothering to shave microseconds off an error reporting path. regards, tom lane ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="improve-missing-column-error-hints-1.patch"; charset="us-ascii" Content-ID: <4158658.1661104161.2@sss.pgh.pa.us> Content-Description: improve-missing-column-error-hints-1.patch Content-Transfer-Encoding: quoted-printable diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/pars= e_relation.c index f6b740df0a..e1591c6fb1 100644 --- a/src/backend/parser/parse_relation.c +++ b/src/backend/parser/parse_relation.c @@ -81,6 +81,8 @@ static void expandTupleDesc(TupleDesc tupdesc, Alias *er= ef, int location, bool include_dropped, List **colnames, List **colvars); static int specialAttNum(const char *attname); +static bool rte_visible_if_lateral(ParseState *pstate, RangeTblEntry *rte= ); +static bool rte_visible_if_qualified(ParseState *pstate, RangeTblEntry *r= te); static bool isQueryUsingTempRelation_walker(Node *node, void *context); = = @@ -3604,17 +3606,27 @@ errorMissingRTE(ParseState *pstate, RangeVar *rela= tion) badAlias =3D rte->eref->aliasname; } = - if (rte) + /* If it looks like the user forgot to use an alias, hint about that */ + if (badAlias) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_TABLE), errmsg("invalid reference to FROM-clause entry for table \"%s\"", relation->relname), - (badAlias ? - errhint("Perhaps you meant to reference the table alias \"%s\".", - badAlias) : - errhint("There is an entry for table \"%s\", but it cannot be refer= enced from this part of the query.", - rte->eref->aliasname)), + errhint("Perhaps you meant to reference the table alias \"%s\".", + badAlias), parser_errposition(pstate, relation->location))); + /* Hint about case where we found an (inaccessible) exact match */ + else if (rte) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_TABLE), + errmsg("invalid reference to FROM-clause entry for table \"%s\"", + relation->relname), + errdetail("There is an entry for table \"%s\", but it cannot be refe= renced from this part of the query.", + rte->eref->aliasname), + rte_visible_if_lateral(pstate, rte) ? + errhint("To reference that table, you must mark this subquery with L= ATERAL.") : 0, + parser_errposition(pstate, relation->location))); + /* Else, we have nothing to offer but the bald statement of error */ else ereport(ERROR, (errcode(ERRCODE_UNDEFINED_TABLE), @@ -3639,9 +3651,6 @@ errorMissingColumn(ParseState *pstate, /* * Search the entire rtable looking for possible matches. If we find on= e, * emit a hint about it. - * - * TODO: improve this code (and also errorMissingRTE) to mention using - * LATERAL if appropriate. */ state =3D searchRangeTableForCol(pstate, relname, colname, location); = @@ -3660,21 +3669,38 @@ errorMissingColumn(ParseState *pstate, = if (!state->rsecond) { - /* - * Handle case where there is zero or one column suggestions to hint, - * including exact matches referenced but not visible. - */ - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_COLUMN), - relname ? - errmsg("column %s.%s does not exist", relname, colname) : - errmsg("column \"%s\" does not exist", colname), - state->rfirst ? closestfirst ? - errhint("Perhaps you meant to reference the column \"%s.%s\".", - state->rfirst->eref->aliasname, closestfirst) : - errhint("There is a column named \"%s\" in table \"%s\", but it cann= ot be referenced from this part of the query.", - colname, state->rfirst->eref->aliasname) : 0, - parser_errposition(pstate, location))); + /* If we found no match at all, we have little to report */ + if (!state->rfirst) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_COLUMN), + relname ? + errmsg("column %s.%s does not exist", relname, colname) : + errmsg("column \"%s\" does not exist", colname), + parser_errposition(pstate, location))); + /* Handle case where we have a single alternative spelling to offer */ + else if (closestfirst) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_COLUMN), + relname ? + errmsg("column %s.%s does not exist", relname, colname) : + errmsg("column \"%s\" does not exist", colname), + errhint("Perhaps you meant to reference the column \"%s.%s\".", + state->rfirst->eref->aliasname, closestfirst), + parser_errposition(pstate, location))); + /* We found an exact match but it's inaccessible for some reason */ + else + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_COLUMN), + relname ? + errmsg("column %s.%s does not exist", relname, colname) : + errmsg("column \"%s\" does not exist", colname), + errdetail("There is a column named \"%s\" in table \"%s\", but it c= annot be referenced from this part of the query.", + colname, state->rfirst->eref->aliasname), + rte_visible_if_lateral(pstate, state->rfirst) ? + errhint("To reference that column, you must mark this subquery with= LATERAL.") : + (!relname && rte_visible_if_qualified(pstate, state->rfirst)) ? + errhint("To reference that column, you must use a table-qualified n= ame.") : 0, + parser_errposition(pstate, location))); } else { @@ -3696,6 +3722,71 @@ errorMissingColumn(ParseState *pstate, } } = +/* + * Find ParseNamespaceItem for RTE, if it's visible at all. + * We assume an RTE couldn't appear more than once in the namespace lists= . + */ +static ParseNamespaceItem * +findNSItemForRTE(ParseState *pstate, RangeTblEntry *rte) +{ + while (pstate !=3D NULL) + { + ListCell *l; + + foreach(l, pstate->p_namespace) + { + ParseNamespaceItem *nsitem =3D (ParseNamespaceItem *) lfirst(l); + + if (nsitem->p_rte =3D=3D rte) + return nsitem; + } + pstate =3D pstate->parentParseState; + } + return NULL; +} + +/* + * Would this RTE be visible, if only the user had written LATERAL? + * + * This is a helper for deciding whether to issue a HINT about LATERAL. + * As such, it doesn't need to be 100% accurate; the HINT could be useful + * even if it's not quite right. Hence, we don't delve into fine points + * about whether a found nsitem has the appropriate one of p_rel_visible = or + * p_cols_visible set. + */ +static bool +rte_visible_if_lateral(ParseState *pstate, RangeTblEntry *rte) +{ + ParseNamespaceItem *nsitem; + + /* If LATERAL *is* active, we're clearly barking up the wrong tree */ + if (pstate->p_lateral_active) + return false; + nsitem =3D findNSItemForRTE(pstate, rte); + if (nsitem) + { + /* Found it, report whether it's LATERAL-only */ + return nsitem->p_lateral_only && nsitem->p_lateral_ok; + } + return false; +} + +/* + * Would columns in this RTE be visible if qualified? + */ +static bool +rte_visible_if_qualified(ParseState *pstate, RangeTblEntry *rte) +{ + ParseNamespaceItem *nsitem =3D findNSItemForRTE(pstate, rte); + + if (nsitem) + { + /* Found it, report whether it's relation-only */ + return nsitem->p_rel_visible && !nsitem->p_cols_visible; + } + return false; +} + = /* * Examine a fully-parsed query, and return true iff any relation underly= ing diff --git a/src/test/regress/expected/insert_conflict.out b/src/test/regr= ess/expected/insert_conflict.out index 66d8633e3e..9e9e3bd00c 100644 --- a/src/test/regress/expected/insert_conflict.out +++ b/src/test/regress/expected/insert_conflict.out @@ -242,7 +242,7 @@ insert into insertconflicttest values (1, 'Apple') on = conflict (key) do update s ERROR: invalid reference to FROM-clause entry for table "excluded" LINE 1: ...y) do update set fruit =3D excluded.fruit RETURNING excluded.f= ... ^ -HINT: There is an entry for table "excluded", but it cannot be reference= d from this part of the query. +DETAIL: There is an entry for table "excluded", but it cannot be referen= ced from this part of the query. -- Only suggest .* column when inference element misspelled: insert into insertconflicttest values (1, 'Apple') on conflict (keyy) do = update set fruit =3D excluded.fruit; ERROR: column "keyy" does not exist diff --git a/src/test/regress/expected/join.out b/src/test/regress/expecte= d/join.out index 2ed2e542a4..a515399f5e 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -1638,7 +1638,7 @@ SELECT * FROM (J1_TBL JOIN J2_TBL USING (i)) AS x WH= ERE J1_TBL.t =3D 'one'; -- er ERROR: invalid reference to FROM-clause entry for table "j1_tbl" LINE 1: ... * FROM (J1_TBL JOIN J2_TBL USING (i)) AS x WHERE J1_TBL.t =3D= ... ^ -HINT: There is an entry for table "j1_tbl", but it cannot be referenced = from this part of the query. +DETAIL: There is an entry for table "j1_tbl", but it cannot be reference= d from this part of the query. SELECT * FROM J1_TBL JOIN J2_TBL USING (i) AS x WHERE x.i =3D 1; -- ok i | j | t | k = ---+---+-----+---- @@ -4975,7 +4975,7 @@ select * from ERROR: invalid reference to FROM-clause entry for table "y" LINE 2: ...bl x join (int4_tbl x cross join int4_tbl y) j on q1 =3D y.f1; ^ -HINT: There is an entry for table "y", but it cannot be referenced from = this part of the query. +DETAIL: There is an entry for table "y", but it cannot be referenced fro= m this part of the query. select * from int8_tbl x join (int4_tbl x cross join int4_tbl y(ff)) j on q1 =3D f1; = -- ok q1 | q2 | f1 | ff = @@ -5003,6 +5003,13 @@ ERROR: column "uunique1" does not exist LINE 1: select uunique1 from ^ HINT: Perhaps you meant to reference the column "t1.unique1" or the colu= mn "t2.unique1". +select ctid from + tenk1 t1 join tenk2 t2 on t1.two =3D t2.two; -- error, need qualificati= on +ERROR: column "ctid" does not exist +LINE 1: select ctid from + ^ +DETAIL: There is a column named "ctid" in table "t1", but it cannot be r= eferenced from this part of the query. +HINT: To reference that column, you must use a table-qualified name. -- -- Take care to reference the correct RTE -- @@ -6036,22 +6043,26 @@ select f1,g from int4_tbl a, (select f1 as g) ss; ERROR: column "f1" does not exist LINE 1: select f1,g from int4_tbl a, (select f1 as g) ss; ^ -HINT: There is a column named "f1" in table "a", but it cannot be refere= nced from this part of the query. +DETAIL: There is a column named "f1" in table "a", but it cannot be refe= renced from this part of the query. +HINT: To reference that column, you must mark this subquery with LATERAL= . select f1,g from int4_tbl a, (select a.f1 as g) ss; ERROR: invalid reference to FROM-clause entry for table "a" LINE 1: select f1,g from int4_tbl a, (select a.f1 as g) ss; ^ -HINT: There is an entry for table "a", but it cannot be referenced from = this part of the query. +DETAIL: There is an entry for table "a", but it cannot be referenced fro= m this part of the query. +HINT: To reference that table, you must mark this subquery with LATERAL. select f1,g from int4_tbl a cross join (select f1 as g) ss; ERROR: column "f1" does not exist LINE 1: select f1,g from int4_tbl a cross join (select f1 as g) ss; ^ -HINT: There is a column named "f1" in table "a", but it cannot be refere= nced from this part of the query. +DETAIL: There is a column named "f1" in table "a", but it cannot be refe= renced from this part of the query. +HINT: To reference that column, you must mark this subquery with LATERAL= . select f1,g from int4_tbl a cross join (select a.f1 as g) ss; ERROR: invalid reference to FROM-clause entry for table "a" LINE 1: select f1,g from int4_tbl a cross join (select a.f1 as g) ss... ^ -HINT: There is an entry for table "a", but it cannot be referenced from = this part of the query. +DETAIL: There is an entry for table "a", but it cannot be referenced fro= m this part of the query. +HINT: To reference that table, you must mark this subquery with LATERAL. -- SQL:2008 says the left table is in scope but illegal to access here select f1,g from int4_tbl a right join lateral generate_series(0, a.f1) g= on true; ERROR: invalid reference to FROM-clause entry for table "a" @@ -6081,12 +6092,12 @@ update xx1 set x2 =3D f1 from (select * from int4_= tbl where f1 =3D x1) ss; ERROR: column "x1" does not exist LINE 1: ... set x2 =3D f1 from (select * from int4_tbl where f1 =3D x1) s= s; ^ -HINT: There is a column named "x1" in table "xx1", but it cannot be refe= renced from this part of the query. +DETAIL: There is a column named "x1" in table "xx1", but it cannot be re= ferenced from this part of the query. update xx1 set x2 =3D f1 from (select * from int4_tbl where f1 =3D xx1.x1= ) ss; ERROR: invalid reference to FROM-clause entry for table "xx1" LINE 1: ...t x2 =3D f1 from (select * from int4_tbl where f1 =3D xx1.x1) = ss... ^ -HINT: There is an entry for table "xx1", but it cannot be referenced fro= m this part of the query. +DETAIL: There is an entry for table "xx1", but it cannot be referenced f= rom this part of the query. -- can't do it even with LATERAL: update xx1 set x2 =3D f1 from lateral (select * from int4_tbl where f1 =3D= x1) ss; ERROR: invalid reference to FROM-clause entry for table "xx1" @@ -6101,12 +6112,12 @@ delete from xx1 using (select * from int4_tbl wher= e f1 =3D x1) ss; ERROR: column "x1" does not exist LINE 1: ...te from xx1 using (select * from int4_tbl where f1 =3D x1) ss; ^ -HINT: There is a column named "x1" in table "xx1", but it cannot be refe= renced from this part of the query. +DETAIL: There is a column named "x1" in table "xx1", but it cannot be re= ferenced from this part of the query. delete from xx1 using (select * from int4_tbl where f1 =3D xx1.x1) ss; ERROR: invalid reference to FROM-clause entry for table "xx1" LINE 1: ...from xx1 using (select * from int4_tbl where f1 =3D xx1.x1) ss= ... ^ -HINT: There is an entry for table "xx1", but it cannot be referenced fro= m this part of the query. +DETAIL: There is an entry for table "xx1", but it cannot be referenced f= rom this part of the query. delete from xx1 using lateral (select * from int4_tbl where f1 =3D x1) ss= ; ERROR: invalid reference to FROM-clause entry for table "xx1" LINE 1: ...xx1 using lateral (select * from int4_tbl where f1 =3D x1) ss; diff --git a/src/test/regress/expected/merge.out b/src/test/regress/expect= ed/merge.out index 729ae2eb06..5af344e4e3 100644 --- a/src/test/regress/expected/merge.out +++ b/src/test/regress/expected/merge.out @@ -197,7 +197,7 @@ WHEN NOT MATCHED THEN ERROR: invalid reference to FROM-clause entry for table "t" LINE 2: USING (SELECT * FROM source WHERE t.tid > sid) s ^ -HINT: There is an entry for table "t", but it cannot be referenced from = this part of the query. +DETAIL: There is an entry for table "t", but it cannot be referenced fro= m this part of the query. -- -- initial tests -- @@ -618,7 +618,7 @@ WHEN NOT MATCHED THEN ERROR: invalid reference to FROM-clause entry for table "t" LINE 5: INSERT (tid, balance) VALUES (t.tid, s.delta); ^ -HINT: There is an entry for table "t", but it cannot be referenced from = this part of the query. +DETAIL: There is an entry for table "t", but it cannot be referenced fro= m this part of the query. -- and again with a constant ON clause BEGIN; MERGE INTO target t @@ -629,7 +629,7 @@ WHEN NOT MATCHED THEN ERROR: invalid reference to FROM-clause entry for table "t" LINE 5: INSERT (tid, balance) VALUES (t.tid, s.delta); ^ -HINT: There is an entry for table "t", but it cannot be referenced from = this part of the query. +DETAIL: There is an entry for table "t", but it cannot be referenced fro= m this part of the query. SELECT * FROM target ORDER BY tid; ERROR: current transaction is aborted, commands ignored until end of tra= nsaction block ROLLBACK; @@ -722,7 +722,7 @@ WHEN NOT MATCHED AND t.balance =3D 100 THEN ERROR: invalid reference to FROM-clause entry for table "t" LINE 3: WHEN NOT MATCHED AND t.balance =3D 100 THEN ^ -HINT: There is an entry for table "t", but it cannot be referenced from = this part of the query. +DETAIL: There is an entry for table "t", but it cannot be referenced fro= m this part of the query. SELECT * FROM wq_target; ERROR: current transaction is aborted, commands ignored until end of tra= nsaction block ROLLBACK; diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expect= ed/rules.out index 7ec3d2688f..66323c323f 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -1194,7 +1194,8 @@ do instead insert into rules_foo2 values (f1); ERROR: column "f1" does not exist LINE 2: do instead insert into rules_foo2 values (f1); ^ -HINT: There is a column named "f1" in table "old", but it cannot be refe= renced from this part of the query. +DETAIL: There is a column named "f1" in table "old", but it cannot be re= ferenced from this part of the query. +HINT: To reference that column, you must use a table-qualified name. -- this is the correct way: create rule rules_foorule as on insert to rules_foo where f1 < 100 do instead insert into rules_foo2 values (new.f1); diff --git a/src/test/regress/expected/union.out b/src/test/regress/expect= ed/union.out index 7ac4a9380e..220f0c1629 100644 --- a/src/test/regress/expected/union.out +++ b/src/test/regress/expected/union.out @@ -910,7 +910,7 @@ SELECT q1 FROM int8_tbl EXCEPT SELECT q2 FROM int8_tbl= ORDER BY q2 LIMIT 1; ERROR: column "q2" does not exist LINE 1: ... int8_tbl EXCEPT SELECT q2 FROM int8_tbl ORDER BY q2 LIMIT 1..= . ^ -HINT: There is a column named "q2" in table "*SELECT* 2", but it cannot = be referenced from this part of the query. +DETAIL: There is a column named "q2" in table "*SELECT* 2", but it canno= t be referenced from this part of the query. -- But this should work: SELECT q1 FROM int8_tbl EXCEPT (((SELECT q2 FROM int8_tbl ORDER BY q2 LIM= IT 1))) ORDER BY 1; q1 = diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql index 27e7e741a1..3504e31260 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -1803,6 +1803,8 @@ select t2.uunique1 from tenk1 t1 join tenk2 t2 on t1.two =3D t2.two; -- error, prefer "t2" sugg= estion select uunique1 from tenk1 t1 join tenk2 t2 on t1.two =3D t2.two; -- error, suggest both at = once +select ctid from + tenk1 t1 join tenk2 t2 on t1.two =3D t2.two; -- error, need qualificati= on = -- -- Take care to reference the correct RTE ------- =_aaaaaaaaaa0--