On Mon, 2 Oct 2023 at 01:01, Tom Lane <t...@sss.pgh.pa.us> wrote:
>
> Erwin Brandstetter <brsaw...@gmail.com> writes:
> > On Mon, 2 Oct 2023 at 00:33, Dean Rasheed <dean.a.rash...@gmail.com> wrote:
> >> The only place that exposes the eref's made-up relation name is the
> >> existing query deparsing code in ruleutils.c, which uniquifies it and
> >> generates SQL spec-compliant output. For example:
>
> > I ran into one other place: error messages.
> > SELECT unnamed_subquery.a
> > FROM (SELECT 1 AS a)
> > ERROR: There is an entry for table "unnamed_subquery", but it cannot be
> > referenced from this part of the query.invalid reference to FROM-clause
> > entry for table "unnamed_subquery"
>
> Yeah, that's exposing more of the implementation than we really want.
>
Note that this isn't a new issue, specific to unnamed subqueries. The
same thing happens for unnamed joins:
create table foo(a int);
create table bar(a int);
select unnamed_join.a from foo join bar using (a);
ERROR: invalid reference to FROM-clause entry for table "unnamed_join"
LINE 1: select unnamed_join.a from foo join bar using (a);
^
DETAIL: There is an entry for table "unnamed_join", but it cannot be
referenced from this part of the query.
And there's a similar problem with VALUES RTEs:
insert into foo values (1),(2) returning "*VALUES*".a;
ERROR: invalid reference to FROM-clause entry for table "*VALUES*"
LINE 1: insert into foo values (1),(2) returning "*VALUES*".a;
^
DETAIL: There is an entry for table "*VALUES*", but it cannot be
referenced from this part of the query.
> I'm inclined to think we should avoid letting "unnamed_subquery"
> appear in the parse tree, too. It might not be a good idea to
> try to leave the eref field null, but could we set it to an
> empty string instead, that is
>
> - eref = alias ? copyObject(alias) : makeAlias("unnamed_subquery", NIL);
> + eref = alias ? copyObject(alias) : makeAlias("", NIL);
>
> and then let ruleutils replace that with "unnamed_subquery"?
Hmm, I think that there would be other side-effects if we did that --
at least doing it for VALUES RTEs would also require additional
changes to retain current EXPLAIN output. I think perhaps it would be
better to try for a more targeted fix of the parser error reporting.
In searchRangeTableForRel() we try to find any RTE that could possibly
match the RangeVar, but certain kinds of RTE don't naturally have
names, and if they also haven't been given aliases, then they can't
possibly match anywhere in the query (and thus it's misleading to
report that they can't be referred to from specific places).
So I think perhaps it's better to just have searchRangeTableForRel()
exclude these kinds of RTE, if they haven't been given an alias.
Regards,
Dean
diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c
new file mode 100644
index 864ea9b..0afe177
--- a/src/backend/parser/parse_relation.c
+++ b/src/backend/parser/parse_relation.c
@@ -395,6 +395,20 @@ searchRangeTableForRel(ParseState *pstat
{
RangeTblEntry *rte = (RangeTblEntry *) lfirst(l);
+ /*
+ * Ignore RTEs that do not automatically take their names from
+ * database objects, and have not been supplied with aliases
+ * (e.g., unnamed joins). Such RTEs cannot possibly match the
+ * RangeVar, and cannot be referenced from anywhere in the query.
+ * Thus, it would be misleading to complain that they can't be
+ * referenced from particular parts of the query.
+ */
+ if (!rte->alias &&
+ (rte->rtekind == RTE_SUBQUERY ||
+ rte->rtekind == RTE_JOIN ||
+ rte->rtekind == RTE_VALUES))
+ continue;
+
if (rte->rtekind == RTE_RELATION &&
OidIsValid(relId) &&
rte->relid == relId)
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
new file mode 100644
index 9b8638f..2bbf3bf
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -2895,6 +2895,26 @@ select bar.*, unnamed_join.* from
ERROR: FOR UPDATE cannot be applied to a join
LINE 3: for update of bar;
^
+-- Test error reporting of references to internal aliases
+select unnamed_join.* from
+ t1 join t2 on (t1.a = t2.a);
+ERROR: missing FROM-clause entry for table "unnamed_join"
+LINE 1: select unnamed_join.* from
+ ^
+select unnamed_join.* from
+ (select * from t1 join t2 using (a)) ss join t3 on (t3.x = ss.a);
+ERROR: missing FROM-clause entry for table "unnamed_join"
+LINE 1: select unnamed_join.* from
+ ^
+select unnamed_subquery.* from
+ (select * from t1);
+ERROR: missing FROM-clause entry for table "unnamed_subquery"
+LINE 1: select unnamed_subquery.* from
+ ^
+insert into t1 values (1, 10), (2, 20) returning "*VALUES*".*;
+ERROR: missing FROM-clause entry for table "*VALUES*"
+LINE 1: insert into t1 values (1, 10), (2, 20) returning "*VALUES*"....
+ ^
--
-- regression test for 8.1 merge right join bug
--
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
new file mode 100644
index 3e5032b..4d4ebc3
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -669,6 +669,19 @@ select bar.*, unnamed_join.* from
(t1 join t2 using (a) as foo) as bar, t3 as unnamed_join
for update of bar;
+-- Test error reporting of references to internal aliases
+
+select unnamed_join.* from
+ t1 join t2 on (t1.a = t2.a);
+
+select unnamed_join.* from
+ (select * from t1 join t2 using (a)) ss join t3 on (t3.x = ss.a);
+
+select unnamed_subquery.* from
+ (select * from t1);
+
+insert into t1 values (1, 10), (2, 20) returning "*VALUES*".*;
+
--
-- regression test for 8.1 merge right join bug
--
