From cc47bc21c49a0941f7664b6707a5f9a52b7aa1ce Mon Sep 17 00:00:00 2001
From: David Rowley <dgrowley@gmail.com>
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 ] <replaceable class="parameter">grouping_element</rep
     class="parameter">expression</replaceable> used inside a
     <replaceable class="parameter">grouping_element</replaceable>
     can be an input column name, or the name or ordinal number of an
-    output column (<command>SELECT</command> list item), or an arbitrary
-    expression formed from input-column values.  In case of ambiguity,
-    a <literal>GROUP BY</literal> name will be interpreted as an
-    input-column name rather than an output column name.
+    output column (<command>SELECT</command> list item) expressed as an
+    integer literal, or an arbitrary expression formed from input-column
+    values.  In case of ambiguity, a <literal>GROUP BY</literal> name will be
+    interpreted as an input-column name rather than an output column name.
    </para>
 
    <para>
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

