1
0
mirror of https://github.com/postgres/postgres.git synced 2025-07-15 19:21:59 +03:00

Allow empty target list in SELECT.

This fixes a problem noted as a followup to bug #8648: if a query has a
semantically-empty target list, e.g. SELECT * FROM zero_column_table,
ruleutils.c will dump it as a syntactically-empty target list, which was
not allowed.  There doesn't seem to be any reliable way to fix this by
hacking ruleutils (note in particular that the originally zero-column table
might since have had columns added to it); and even if we had such a fix,
it would do nothing for existing dump files that might contain bad syntax.
The best bet seems to be to relax the syntactic restriction.

Also, add parse-analysis errors for SELECT DISTINCT with no columns (after
*-expansion) and RETURNING with no columns.  These cases previously
produced unexpected behavior because the parsed Query looked like it had
no DISTINCT or RETURNING clause, respectively.  If anyone ever offers
a plausible use-case for this, we could work a bit harder on making the
situation distinguishable.

Arguably this is a bug fix that should be back-patched, but I'm worried
that there may be client apps or PLs that expect "SELECT ;" to throw a
syntax error.  The issue doesn't seem important enough to risk changing
behavior in minor releases.
This commit is contained in:
Tom Lane
2013-12-14 20:23:26 -05:00
parent c03ad5602f
commit 1b4f7f93b4
6 changed files with 73 additions and 33 deletions

View File

@ -2018,6 +2018,19 @@ transformReturningList(ParseState *pstate, List *returningList)
/* transform RETURNING identically to a SELECT targetlist */
rlist = transformTargetList(pstate, returningList, EXPR_KIND_RETURNING);
/*
* Complain if the nonempty tlist expanded to nothing (which is possible
* if it contains only a star-expansion of a zero-column table). If we
* allow this, the parsed Query will look like it didn't have RETURNING,
* with results that would probably surprise the user.
*/
if (rlist == NIL)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("RETURNING must have at least one column"),
parser_errposition(pstate,
exprLocation(linitial(returningList)))));
/* mark column origins */
markTargetListOrigins(pstate, rlist);

View File

@ -334,7 +334,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
name_list from_clause from_list opt_array_bounds
qualified_name_list any_name any_name_list
any_operator expr_list attrs
target_list insert_column_list set_target_list
target_list opt_target_list insert_column_list set_target_list
set_clause_list set_clause multiple_set_clause
ctext_expr_list ctext_row def_list indirection opt_indirection
reloption_list group_clause TriggerFuncArgs select_limit
@ -9259,7 +9259,7 @@ select_clause:
* However, this is not checked by the grammar; parse analysis must check it.
*/
simple_select:
SELECT opt_distinct target_list
SELECT opt_distinct opt_target_list
into_clause from_clause where_clause
group_clause having_clause window_clause
{
@ -12215,6 +12215,10 @@ ctext_row: '(' ctext_expr_list ')' { $$ = $2; }
*
*****************************************************************************/
opt_target_list: target_list { $$ = $1; }
| /* EMPTY */ { $$ = NIL; }
;
target_list:
target_el { $$ = list_make1($1); }
| target_list ',' target_el { $$ = lappend($1, $3); }

View File

@ -2011,6 +2011,20 @@ transformDistinctClause(ParseState *pstate,
true);
}
/*
* Complain if we found nothing to make DISTINCT. Returning an empty list
* would cause the parsed Query to look like it didn't have DISTINCT, with
* results that would probably surprise the user. Note: this case is
* presently impossible for aggregates because of grammar restrictions,
* but we check anyway.
*/
if (result == NIL)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
is_agg ?
errmsg("an aggregate with DISTINCT must have at least one argument") :
errmsg("SELECT DISTINCT must have at least one column")));
return result;
}
@ -2115,6 +2129,11 @@ transformDistinctOnClause(ParseState *pstate, List *distinctlist,
true);
}
/*
* An empty result list is impossible here because of grammar restrictions.
*/
Assert(result != NIL);
return result;
}