From cc47bc21c49a0941f7664b6707a5f9a52b7aa1ce Mon Sep 17 00:00:00 2001 From: David Rowley Date: Mon, 11 Sep 2023 17:10:41 +1200 Subject: [PATCH v1 2/2] Allow any constant types in GROUP BY clause In v14 and earlier, only integer Consts were allowed in the GROUP BY clause. Boolean constants made it through as these had no native node type and were expressed as TypeCasts. As of 941460fcf, Booleans have their own node type which resulted in "GROUP BY true" no longer working in v15 and v16. Here adjust the code to remove the special Boolean case that was added to fix the v15/v16 regression and just allow any constant types in the GROUP BY. Integers, of course, remain a special case to reference the ordinal position of the column in the target list to group by. Also, clarify in the documentation that references made in the GROUP BY to output columns must be done so using integer literals. This should lessen the chances of any surprises that "GROUP BY 1.0" doesn't group by the 1st column in the target list. Discussion: https://postgr.es/m/CAApHDvrSUvb1ODvNcYGHz4O6WEsPChhwmSkJcs_3y5pniN2p+A@mail.gmail.com --- doc/src/sgml/ref/select.sgml | 8 ++--- src/backend/parser/parse_clause.c | 52 +++++++++++++------------------ 2 files changed, 26 insertions(+), 34 deletions(-) diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml index 0ee0cc7e64..323561cb0f 100644 --- a/doc/src/sgml/ref/select.sgml +++ b/doc/src/sgml/ref/select.sgml @@ -804,10 +804,10 @@ GROUP BY [ ALL | DISTINCT ] grouping_elementexpression used inside a grouping_element can be an input column name, or the name or ordinal number of an - output column (SELECT list item), or an arbitrary - expression formed from input-column values. In case of ambiguity, - a GROUP BY name will be interpreted as an - input-column name rather than an output column name. + output column (SELECT list item) expressed as an + integer literal, or an arbitrary expression formed from input-column + values. In case of ambiguity, a GROUP BY name will be + interpreted as an input-column name rather than an output column name. diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c index 9effe6ae7c..a988e0202c 100644 --- a/src/backend/parser/parse_clause.c +++ b/src/backend/parser/parse_clause.c @@ -2109,46 +2109,38 @@ findTargetlistEntrySQL92(ParseState *pstate, Node *node, List **tlist, if (IsA(node, A_Const)) { A_Const *aconst = castNode(A_Const, node); - int targetlist_pos = 0; - int target_pos; /* - * Pre-v15 versions supported boolean constants in the GROUP BY clause. - * Here we make a special case to allow these after boolean types were given - * their own parse node type in v15. + * Handle integer constants as the 1-based position in the query's + * targetlist. Other constant types are treated as expressions. */ - if (IsA(&aconst->val, Boolean)) - return findTargetlistEntrySQL99(pstate, node, tlist, exprKind); - - if (!IsA(&aconst->val, Integer)) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - /* translator: %s is name of a SQL construct, eg ORDER BY */ - errmsg("non-integer constant in %s", - ParseExprKindName(exprKind)), - parser_errposition(pstate, aconst->location))); - - target_pos = intVal(&aconst->val); - foreach(tl, *tlist) + if (IsA(&aconst->val, Integer)) { - TargetEntry *tle = (TargetEntry *) lfirst(tl); + int targetlist_pos = 0; + int target_pos; - if (!tle->resjunk) + target_pos = intVal(&aconst->val); + foreach(tl, *tlist) { - if (++targetlist_pos == target_pos) + TargetEntry *tle = (TargetEntry *) lfirst(tl); + + if (!tle->resjunk) { - /* return the unique match, after suitable validation */ - checkTargetlistEntrySQL92(pstate, tle, exprKind); - return tle; + if (++targetlist_pos == target_pos) + { + /* return the unique match, after suitable validation */ + checkTargetlistEntrySQL92(pstate, tle, exprKind); + return tle; + } } } + ereport(ERROR, + (errcode(ERRCODE_INVALID_COLUMN_REFERENCE), + /* translator: %s is name of a SQL construct, eg ORDER BY */ + errmsg("%s position %d is not in select list", + ParseExprKindName(exprKind), target_pos), + parser_errposition(pstate, aconst->location))); } - ereport(ERROR, - (errcode(ERRCODE_INVALID_COLUMN_REFERENCE), - /* translator: %s is name of a SQL construct, eg ORDER BY */ - errmsg("%s position %d is not in select list", - ParseExprKindName(exprKind), target_pos), - parser_errposition(pstate, aconst->location))); } /* -- 2.40.1.windows.1