On Wed, 6 Jul 2022 at 15:09, Dean Rasheed <dean.a.rash...@gmail.com> wrote:
>
> I'll post an update in a little while, but first, I found a bug, which
> revealed a pre-existing bug in transformLockingClause(). I'll start a
> new thread for that, since it'd be good to get that resolved
> independently of this patch.
>
Attached is an update with the following changes:
* Docs updated as suggested.
* transformLockingClause() updated to skip subquery and values rtes
without aliases.
* eref->aliasname changed to "unnamed_subquery" for subqueries without aliases.
Regards,
Dean
diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml
new file mode 100644
index 80bb8ad..410c80e
--- a/doc/src/sgml/ref/select.sgml
+++ b/doc/src/sgml/ref/select.sgml
@@ -51,7 +51,7 @@ SELECT [ ALL | DISTINCT [ ON ( <replacea
[ ONLY ] <replaceable class="parameter">table_name</replaceable> [ * ] [ [ AS ] <replaceable class="parameter">alias</replaceable> [ ( <replaceable class="parameter">column_alias</replaceable> [, ...] ) ] ]
[ TABLESAMPLE <replaceable class="parameter">sampling_method</replaceable> ( <replaceable class="parameter">argument</replaceable> [, ...] ) [ REPEATABLE ( <replaceable class="parameter">seed</replaceable> ) ] ]
- [ LATERAL ] ( <replaceable class="parameter">select</replaceable> ) [ AS ] <replaceable class="parameter">alias</replaceable> [ ( <replaceable class="parameter">column_alias</replaceable> [, ...] ) ]
+ [ LATERAL ] ( <replaceable class="parameter">select</replaceable> ) [ [ AS ] <replaceable class="parameter">alias</replaceable> [ ( <replaceable class="parameter">column_alias</replaceable> [, ...] ) ] ]
<replaceable class="parameter">with_query_name</replaceable> [ [ AS ] <replaceable class="parameter">alias</replaceable> [ ( <replaceable class="parameter">column_alias</replaceable> [, ...] ) ] ]
[ LATERAL ] <replaceable class="parameter">function_name</replaceable> ( [ <replaceable class="parameter">argument</replaceable> [, ...] ] )
[ WITH ORDINALITY ] [ [ AS ] <replaceable class="parameter">alias</replaceable> [ ( <replaceable class="parameter">column_alias</replaceable> [, ...] ) ] ]
@@ -490,8 +490,8 @@ TABLE [ ONLY ] <replaceable class="param
output were created as a temporary table for the duration of
this single <command>SELECT</command> command. Note that the
sub-<command>SELECT</command> must be surrounded by
- parentheses, and an alias <emphasis>must</emphasis> be
- provided for it. A
+ parentheses, and an alias can be provided in the same way as for a
+ table. A
<link linkend="sql-values"><command>VALUES</command></link> command
can also be used here.
</para>
@@ -2041,6 +2041,16 @@ SELECT 2+2;
</para>
</refsect2>
+ <refsect2>
+ <title>Omitting sub-<command>SELECT</command> aliases in <literal>FROM</literal></title>
+
+ <para>
+ According to the SQL standard, a sub-<command>SELECT</command> in the
+ <literal>FROM</literal> list must have an alias. In
+ <productname>PostgreSQL</productname>, this alias may be omitted.
+ </para>
+ </refsect2>
+
<refsect2>
<title><literal>ONLY</literal> and Inheritance</title>
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
new file mode 100644
index 8ed2c4b..6688c2a
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -3291,7 +3291,7 @@ transformLockingClause(ParseState *pstat
foreach(rt, qry->rtable)
{
RangeTblEntry *rte = (RangeTblEntry *) lfirst(rt);
- char *rtename;
+ char *rtename = rte->eref->aliasname;
++i;
if (!rte->inFromCl)
@@ -3302,15 +3302,22 @@ transformLockingClause(ParseState *pstat
* name and needs to be skipped (otherwise it might hide a
* base relation with the same name), except if it has a USING
* alias, which *is* visible.
+ *
+ * Subquery and values RTEs without aliases are never visible
+ * as relation names and must always be skipped.
*/
- if (rte->rtekind == RTE_JOIN && rte->alias == NULL)
+ if (rte->alias == NULL)
{
- if (rte->join_using_alias == NULL)
+ if (rte->rtekind == RTE_JOIN)
+ {
+ if (rte->join_using_alias == NULL)
+ continue;
+ rtename = rte->join_using_alias->aliasname;
+ }
+ else if (rte->rtekind == RTE_SUBQUERY ||
+ rte->rtekind == RTE_VALUES)
continue;
- rtename = rte->join_using_alias->aliasname;
}
- else
- rtename = rte->eref->aliasname;
if (strcmp(rtename, thisrel->relname) == 0)
{
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
new file mode 100644
index 0523013..0f8d056
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -13346,33 +13346,6 @@ table_ref: relation_expr opt_alias_claus
n->lateral = false;
n->subquery = $1;
n->alias = $2;
- /*
- * The SQL spec does not permit a subselect
- * (<derived_table>) without an alias clause,
- * so we don't either. This avoids the problem
- * of needing to invent a unique refname for it.
- * That could be surmounted if there's sufficient
- * popular demand, but for now let's just implement
- * the spec and see if anyone complains.
- * However, it does seem like a good idea to emit
- * an error message that's better than "syntax error".
- */
- if ($2 == NULL)
- {
- if (IsA($1, SelectStmt) &&
- ((SelectStmt *) $1)->valuesLists)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("VALUES in FROM must have an alias"),
- errhint("For example, FROM (VALUES ...) [AS] foo."),
- parser_errposition(@1)));
- else
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("subquery in FROM must have an alias"),
- errhint("For example, FROM (SELECT ...) [AS] foo."),
- parser_errposition(@1)));
- }
$$ = (Node *) n;
}
| LATERAL_P select_with_parens opt_alias_clause
@@ -13382,23 +13355,6 @@ table_ref: relation_expr opt_alias_claus
n->lateral = true;
n->subquery = $2;
n->alias = $3;
- /* same comment as above */
- if ($3 == NULL)
- {
- if (IsA($2, SelectStmt) &&
- ((SelectStmt *) $2)->valuesLists)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("VALUES in FROM must have an alias"),
- errhint("For example, FROM (VALUES ...) [AS] foo."),
- parser_errposition(@2)));
- else
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("subquery in FROM must have an alias"),
- errhint("For example, FROM (SELECT ...) [AS] foo."),
- parser_errposition(@2)));
- }
$$ = (Node *) n;
}
| joined_table
diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
new file mode 100644
index c655d18..5a18107
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -404,16 +404,6 @@ transformRangeSubselect(ParseState *psta
Query *query;
/*
- * We require user to supply an alias for a subselect, per SQL92. To relax
- * this, we'd have to be prepared to gin up a unique alias for an
- * unlabeled subselect. (This is just elog, not ereport, because the
- * grammar should have enforced it already. It'd probably be better to
- * report the error here, but we don't have a good error location here.)
- */
- if (r->alias == NULL)
- elog(ERROR, "subquery in FROM must have an alias");
-
- /*
* Set p_expr_kind to show this parse level is recursing to a subselect.
* We can't be nested within any expression, so don't need save-restore
* logic here.
@@ -430,10 +420,14 @@ transformRangeSubselect(ParseState *psta
pstate->p_lateral_active = r->lateral;
/*
- * Analyze and transform the subquery.
+ * Analyze and transform the subquery. Note that if the subquery doesn't
+ * have an alias, it can't be explicitly selected for locking, but locking
+ * might still be required (if there is an all-tables locking clause).
*/
query = parse_sub_analyze(r->subquery, pstate, NULL,
- isLockedRefname(pstate, r->alias->aliasname),
+ isLockedRefname(pstate,
+ r->alias == NULL ? NULL :
+ r->alias->aliasname),
true);
/* Restore state */
diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c
new file mode 100644
index 926dcbf..4e63556
--- a/src/backend/parser/parse_relation.c
+++ b/src/backend/parser/parse_relation.c
@@ -1577,7 +1577,11 @@ addRangeTableEntryForRelation(ParseState
* Then, construct and return a ParseNamespaceItem for the new RTE.
*
* This is much like addRangeTableEntry() except that it makes a subquery RTE.
- * Note that an alias clause *must* be supplied.
+ *
+ * If the subquery does not have an alias, the auto-generated relation name in
+ * the returned ParseNamespaceItem will be marked as not visible, and so only
+ * unqualified references to the subquery columns will be allowed, and the
+ * relation name will not conflict with others in the pstate's namespace list.
*/
ParseNamespaceItem *
addRangeTableEntryForSubquery(ParseState *pstate,
@@ -1587,7 +1591,6 @@ addRangeTableEntryForSubquery(ParseState
bool inFromCl)
{
RangeTblEntry *rte = makeNode(RangeTblEntry);
- char *refname = alias->aliasname;
Alias *eref;
int numaliases;
List *coltypes,
@@ -1595,6 +1598,7 @@ addRangeTableEntryForSubquery(ParseState
*colcollations;
int varattno;
ListCell *tlistitem;
+ ParseNamespaceItem *nsitem;
Assert(pstate != NULL);
@@ -1602,7 +1606,7 @@ addRangeTableEntryForSubquery(ParseState
rte->subquery = subquery;
rte->alias = alias;
- eref = copyObject(alias);
+ eref = alias ? copyObject(alias) : makeAlias("unnamed_subquery", NIL);
numaliases = list_length(eref->colnames);
/* fill in any unspecified alias columns, and extract column type info */
@@ -1634,7 +1638,7 @@ addRangeTableEntryForSubquery(ParseState
ereport(ERROR,
(errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
errmsg("table \"%s\" has %d columns available but %d columns specified",
- refname, varattno, numaliases)));
+ eref->aliasname, varattno, numaliases)));
rte->eref = eref;
@@ -1665,8 +1669,15 @@ addRangeTableEntryForSubquery(ParseState
* Build a ParseNamespaceItem, but don't add it to the pstate's namespace
* list --- caller must do that if appropriate.
*/
- return buildNSItemFromLists(rte, list_length(pstate->p_rtable),
- coltypes, coltypmods, colcollations);
+ nsitem = buildNSItemFromLists(rte, list_length(pstate->p_rtable),
+ coltypes, coltypmods, colcollations);
+
+ /*
+ * Mark it visible as a relation name only if it had a user-written alias.
+ */
+ nsitem->p_rel_visible = (alias != NULL);
+
+ return nsitem;
}
/*
@@ -2520,6 +2531,10 @@ addRangeTableEntryForENR(ParseState *pst
* This is used when we have not yet done transformLockingClause, but need
* to know the correct lock to take during initial opening of relations.
*
+ * Note that refname may be NULL (for a subquery without an alias), in which
+ * case the relation can't be locked by name, but it might still be locked if
+ * a locking clause requests that all tables be locked.
+ *
* Note: we pay no attention to whether it's FOR UPDATE vs FOR SHARE,
* since the table-level lock is the same either way.
*/
@@ -2544,7 +2559,7 @@ isLockedRefname(ParseState *pstate, cons
/* all tables used in query */
return true;
}
- else
+ else if (refname != NULL)
{
/* just the named tables */
ListCell *l2;
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
new file mode 100644
index 26cf4fa..513bf0f
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -11725,7 +11725,11 @@ get_from_clause_item(Node *jtnode, Query
else if (rte->rtekind == RTE_SUBQUERY ||
rte->rtekind == RTE_VALUES)
{
- /* Alias is syntactically required for SUBQUERY and VALUES */
+ /*
+ * For a subquery, always print alias. This makes the output SQL
+ * spec-compliant, even though we allow the alias to be omitted on
+ * input.
+ */
printalias = true;
}
else if (rte->rtekind == RTE_CTE)
diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h
new file mode 100644
index cf9c759..962ebf6
--- a/src/include/parser/parse_node.h
+++ b/src/include/parser/parse_node.h
@@ -246,8 +246,11 @@ struct ParseState
* visible for qualified references) but it does hide their columns
* (unqualified references to the columns refer to the JOIN, not the member
* tables, so we must not complain that such a reference is ambiguous).
- * Various special RTEs such as NEW/OLD for rules may also appear with only
- * one flag set.
+ * Conversely, a subquery without an alias does not hide the columns selected
+ * by the subquery, but it does hide the auto-generated relation name (so the
+ * subquery columns are visible for unqualified references only). Various
+ * special RTEs such as NEW/OLD for rules may also appear with only one flag
+ * set.
*
* While processing the FROM clause, namespace items may appear with
* p_lateral_only set, meaning they are visible only to LATERAL
diff --git a/src/interfaces/ecpg/preproc/ecpg.addons b/src/interfaces/ecpg/preproc/ecpg.addons
new file mode 100644
index 94f7d4a..e94da2a
--- a/src/interfaces/ecpg/preproc/ecpg.addons
+++ b/src/interfaces/ecpg/preproc/ecpg.addons
@@ -449,12 +449,6 @@ ECPG: into_clauseINTOOptTempTableName bl
$$= cat2_str(mm_strdup("into"), $2);
}
| ecpg_into { $$ = EMPTY; }
-ECPG: table_refselect_with_parensopt_alias_clause addon
- if ($2 == NULL)
- mmerror(PARSE_ERROR, ET_ERROR, "subquery in FROM must have an alias");
-ECPG: table_refLATERAL_Pselect_with_parensopt_alias_clause addon
- if ($3 == NULL)
- mmerror(PARSE_ERROR, ET_ERROR, "subquery in FROM must have an alias");
ECPG: TypenameSimpleTypenameopt_array_bounds block
{ $$ = cat2_str($1, $2.str); }
ECPG: TypenameSETOFSimpleTypenameopt_array_bounds block
diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out
new file mode 100644
index 45c75ee..63d26d4
--- a/src/test/regress/expected/subselect.out
+++ b/src/test/regress/expected/subselect.out
@@ -30,6 +30,12 @@ SELECT * FROM ((SELECT 1 AS x)) ss;
1
(1 row)
+SELECT * FROM ((SELECT 1 AS x)), ((SELECT * FROM ((SELECT 2 AS y))));
+ x | y
+---+---
+ 1 | 2
+(1 row)
+
(SELECT 2) UNION SELECT 2;
?column?
----------
@@ -196,6 +202,69 @@ SELECT f1 AS "Correlated Field"
3
(5 rows)
+-- Subselects without aliases
+SELECT count FROM (SELECT COUNT(DISTINCT name) FROM road);
+ count
+-------
+ 2911
+(1 row)
+
+SELECT COUNT(*) FROM (SELECT DISTINCT name FROM road);
+ count
+-------
+ 2911
+(1 row)
+
+SELECT * FROM (SELECT * FROM int4_tbl), (VALUES (123456)) WHERE f1 = column1;
+ f1 | column1
+--------+---------
+ 123456 | 123456
+(1 row)
+
+CREATE VIEW view_unnamed_ss AS
+SELECT * FROM (SELECT * FROM (SELECT abs(f1) AS a1 FROM int4_tbl)),
+ (SELECT * FROM int8_tbl)
+ WHERE a1 < 10 AND q1 > a1 ORDER BY q1, q2;
+SELECT * FROM view_unnamed_ss;
+ a1 | q1 | q2
+----+------------------+-------------------
+ 0 | 123 | 456
+ 0 | 123 | 4567890123456789
+ 0 | 4567890123456789 | -4567890123456789
+ 0 | 4567890123456789 | 123
+ 0 | 4567890123456789 | 4567890123456789
+(5 rows)
+
+\sv view_unnamed_ss
+CREATE OR REPLACE VIEW public.view_unnamed_ss AS
+ SELECT unnamed_subquery.a1,
+ unnamed_subquery_1.q1,
+ unnamed_subquery_1.q2
+ FROM ( SELECT unnamed_subquery_2.a1
+ FROM ( SELECT abs(int4_tbl.f1) AS a1
+ FROM int4_tbl) unnamed_subquery_2) unnamed_subquery,
+ ( SELECT int8_tbl.q1,
+ int8_tbl.q2
+ FROM int8_tbl) unnamed_subquery_1
+ WHERE unnamed_subquery.a1 < 10 AND unnamed_subquery_1.q1 > unnamed_subquery.a1
+ ORDER BY unnamed_subquery_1.q1, unnamed_subquery_1.q2
+DROP VIEW view_unnamed_ss;
+-- Test matching of locking clause to correct alias
+CREATE VIEW view_unnamed_ss_locking AS
+SELECT * FROM (SELECT * FROM int4_tbl), int8_tbl AS unnamed_subquery
+ WHERE f1 = q1
+ FOR UPDATE OF unnamed_subquery;
+\sv view_unnamed_ss_locking
+CREATE OR REPLACE VIEW public.view_unnamed_ss_locking AS
+ SELECT unnamed_subquery.f1,
+ unnamed_subquery_1.q1,
+ unnamed_subquery_1.q2
+ FROM ( SELECT int4_tbl.f1
+ FROM int4_tbl) unnamed_subquery,
+ int8_tbl unnamed_subquery_1
+ WHERE unnamed_subquery.f1 = unnamed_subquery_1.q1
+ FOR UPDATE OF unnamed_subquery_1
+DROP VIEW view_unnamed_ss_locking;
--
-- Use some existing tables in the regression test
--
diff --git a/src/test/regress/sql/subselect.sql b/src/test/regress/sql/subselect.sql
new file mode 100644
index 94ba91f..4027670
--- a/src/test/regress/sql/subselect.sql
+++ b/src/test/regress/sql/subselect.sql
@@ -13,6 +13,8 @@ SELECT 1 AS zero WHERE 1 IN (SELECT 2);
SELECT * FROM (SELECT 1 AS x) ss;
SELECT * FROM ((SELECT 1 AS x)) ss;
+SELECT * FROM ((SELECT 1 AS x)), ((SELECT * FROM ((SELECT 2 AS y))));
+
(SELECT 2) UNION SELECT 2;
((SELECT 2)) UNION SELECT 2;
@@ -80,6 +82,35 @@ SELECT f1 AS "Correlated Field"
WHERE (f1, f2) IN (SELECT f2, CAST(f3 AS int4) FROM SUBSELECT_TBL
WHERE f3 IS NOT NULL);
+-- Subselects without aliases
+
+SELECT count FROM (SELECT COUNT(DISTINCT name) FROM road);
+SELECT COUNT(*) FROM (SELECT DISTINCT name FROM road);
+
+SELECT * FROM (SELECT * FROM int4_tbl), (VALUES (123456)) WHERE f1 = column1;
+
+CREATE VIEW view_unnamed_ss AS
+SELECT * FROM (SELECT * FROM (SELECT abs(f1) AS a1 FROM int4_tbl)),
+ (SELECT * FROM int8_tbl)
+ WHERE a1 < 10 AND q1 > a1 ORDER BY q1, q2;
+
+SELECT * FROM view_unnamed_ss;
+
+\sv view_unnamed_ss
+
+DROP VIEW view_unnamed_ss;
+
+-- Test matching of locking clause to correct alias
+
+CREATE VIEW view_unnamed_ss_locking AS
+SELECT * FROM (SELECT * FROM int4_tbl), int8_tbl AS unnamed_subquery
+ WHERE f1 = q1
+ FOR UPDATE OF unnamed_subquery;
+
+\sv view_unnamed_ss_locking
+
+DROP VIEW view_unnamed_ss_locking;
+
--
-- Use some existing tables in the regression test
--
