Re: BUG #17233: Incorrect behavior of DELETE command with bad subquery in WHERE clause

Bruce Momjian <bruce@momjian.us> 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

diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_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 *eref,
                             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 *rte);
 static bool isQueryUsingTempRelation_walker(Node *node, void *context);


@@ -3604,17 +3606,27 @@ errorMissingRTE(ParseState *pstate, RangeVar *relation)
             badAlias = 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 referenced 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 referenced 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 LATERAL.") : 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 one,
      * emit a hint about it.
-     *
-     * TODO: improve this code (and also errorMissingRTE) to mention using
-     * LATERAL if appropriate.
      */
     state = 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 cannot be referenced from this part
ofthe 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 cannot be referenced from this
partof 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 name.") : 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 != NULL)
+    {
+        ListCell   *l;
+
+        foreach(l, pstate->p_namespace)
+        {
+            ParseNamespaceItem *nsitem = (ParseNamespaceItem *) lfirst(l);
+
+            if (nsitem->p_rte == rte)
+                return nsitem;
+        }
+        pstate = 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 = 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 = 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 underlying
diff --git a/src/test/regress/expected/insert_conflict.out b/src/test/regress/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 = excluded.fruit RETURNING excluded.f...
                                                              ^
-HINT:  There is an entry for table "excluded", but it cannot be referenced from this part of the query.
+DETAIL:  There is an entry for table "excluded", but it cannot be referenced from this part of the query.
 -- Only suggest <table>.* column when inference element misspelled:
 insert into insertconflicttest values (1, 'Apple') on conflict (keyy) do update set fruit = excluded.fruit;
 ERROR:  column "keyy" does not exist
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/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 WHERE J1_TBL.t = '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 =...
                                                              ^
-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 referenced from this part of the query.
 SELECT * FROM J1_TBL JOIN J2_TBL USING (i) AS x WHERE x.i = 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 = 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 from this part of the query.
 select * from
   int8_tbl x join (int4_tbl x cross join int4_tbl y(ff)) j on q1 = 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 column "t2.unique1".
+select ctid from
+  tenk1 t1 join tenk2 t2 on t1.two = t2.two; -- error, need qualification
+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 referenced 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 referenced from this part of the query.
+DETAIL:  There is a column named "f1" in table "a", but it cannot be referenced 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 from 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 referenced from this part of the query.
+DETAIL:  There is a column named "f1" in table "a", but it cannot be referenced 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 from 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 = 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.
+DETAIL:  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.
+DETAIL:  There is an entry for table "xx1", but it cannot be referenced from this part of the query.
 -- can't do it even with LATERAL:
 update xx1 set x2 = f1 from lateral (select * from int4_tbl where f1 = x1) ss;
 ERROR:  invalid reference to FROM-clause entry for table "xx1"
@@ -6101,12 +6112,12 @@ 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.
+DETAIL:  There is a column named "x1" in table "xx1", but it cannot be referenced from this part of the query.
 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.
+DETAIL:  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;
 ERROR:  invalid reference to FROM-clause entry for table "xx1"
 LINE 1: ...xx1 using lateral (select * from int4_tbl where f1 = x1) ss;
diff --git a/src/test/regress/expected/merge.out b/src/test/regress/expected/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 from 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 from 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 from this part of the query.
 SELECT * FROM target ORDER BY tid;
 ERROR:  current transaction is aborted, commands ignored until end of transaction block
 ROLLBACK;
@@ -722,7 +722,7 @@ WHEN NOT MATCHED AND t.balance = 100 THEN
 ERROR:  invalid reference to FROM-clause entry for table "t"
 LINE 3: WHEN NOT MATCHED AND t.balance = 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 from this part of the query.
 SELECT * FROM wq_target;
 ERROR:  current transaction is aborted, commands ignored until end of transaction block
 ROLLBACK;
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/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 referenced from this part of the query.
+DETAIL:  There is a column named "f1" in table "old", but it cannot be referenced 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/expected/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 cannot 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 LIMIT 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 = t2.two; -- error, prefer "t2" suggestion
 select uunique1 from
   tenk1 t1 join tenk2 t2 on t1.two = t2.two; -- error, suggest both at once
+select ctid from
+  tenk1 t1 join tenk2 t2 on t1.two = t2.two; -- error, need qualification

 --
 -- Take care to reference the correct RTE