aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
Diffstat (limited to 'src')
-rw-r--r--src/backend/parser/parse_clause.c89
-rw-r--r--src/test/regress/expected/numerology.out12
-rw-r--r--src/test/regress/sql/numerology.sql5
3 files changed, 68 insertions, 38 deletions
diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index 53d9b25f11b..96a005ff0d9 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
- * $Header: /cvsroot/pgsql/src/backend/parser/parse_clause.c,v 1.56 2000/03/14 23:06:32 thomas Exp $
+ * $Header: /cvsroot/pgsql/src/backend/parser/parse_clause.c,v 1.57 2000/03/15 23:31:04 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -719,9 +719,9 @@ parseFromClause(ParseState *pstate, List *frmList)
* list as a "resjunk" node.
*
* node the ORDER BY, GROUP BY, or DISTINCT ON expression to be matched
- * tlist the existing target list (NB: this cannot be NIL, which is a
- * good thing since we'd be unable to append to it...)
- * clause identifies clause type (mainly for error messages).
+ * tlist the existing target list (NB: this will never be NIL, which is a
+ * good thing since we'd be unable to append to it if it were...)
+ * clause identifies clause type being processed.
*/
static TargetEntry *
findTargetlistEntry(ParseState *pstate, Node *node, List *tlist, int clause)
@@ -733,7 +733,7 @@ findTargetlistEntry(ParseState *pstate, Node *node, List *tlist, int clause)
/*----------
* Handle two special cases as mandated by the SQL92 spec:
*
- * 1. ORDER BY ColumnName
+ * 1. Bare ColumnName (no qualifier or subscripts)
* For a bare identifier, we search for a matching column name
* in the existing target list. Multiple matches are an error
* unless they refer to identical values; for example,
@@ -741,49 +741,76 @@ findTargetlistEntry(ParseState *pstate, Node *node, List *tlist, int clause)
* but not SELECT a AS b, b FROM table ORDER BY b
* If no match is found, we fall through and treat the identifier
* as an expression.
- * We do NOT attempt this match for GROUP BY, since it is clearly
- * contrary to the spec to use an output column name in preference
- * to an underlying column name in GROUP BY. DISTINCT ON isn't in
- * the standard, so we can do what we like there; we choose to make
- * it work like GROUP BY.
+ * For GROUP BY, it is incorrect to match the grouping item against
+ * targetlist entries: according to SQL92, an identifier in GROUP BY
+ * is a reference to a column name exposed by FROM, not to a target
+ * list column. However, many implementations (including pre-7.0
+ * PostgreSQL) accept this anyway. So for GROUP BY, we look first
+ * to see if the identifier matches any FROM column name, and only
+ * try for a targetlist name if it doesn't. This ensures that we
+ * adhere to the spec in the case where the name could be both.
+ * DISTINCT ON isn't in the standard, so we can do what we like there;
+ * we choose to make it work like ORDER BY, on the rather flimsy
+ * grounds that ordinary DISTINCT works on targetlist entries.
*
- * 2. ORDER BY/GROUP BY/DISTINCT ON IntegerConstant
+ * 2. IntegerConstant
* This means to use the n'th item in the existing target list.
* Note that it would make no sense to order/group/distinct by an
* actual constant, so this does not create a conflict with our
* extension to order/group by an expression.
- * I believe that GROUP BY column-number is not sanctioned by SQL92,
- * but since the standard has no other behavior defined for this
- * syntax, we may as well continue to support our past behavior.
+ * GROUP BY column-number is not allowed by SQL92, but since
+ * the standard has no other behavior defined for this syntax,
+ * we may as well accept this common extension.
*
- * Note that pre-existing resjunk targets must not be used in either case.
+ * Note that pre-existing resjunk targets must not be used in either case,
+ * since the user didn't write them in his SELECT list.
+ *
+ * If neither special case applies, fall through to treat the item as
+ * an expression.
*----------
*/
- if (clause == ORDER_CLAUSE &&
- IsA(node, Ident) && ((Ident *) node)->indirection == NIL)
+ if (IsA(node, Ident) && ((Ident *) node)->indirection == NIL)
{
char *name = ((Ident *) node)->name;
- foreach(tl, tlist)
+
+ if (clause == GROUP_CLAUSE)
{
- TargetEntry *tle = (TargetEntry *) lfirst(tl);
- Resdom *resnode = tle->resdom;
+ /*
+ * In GROUP BY, we must prefer a match against a FROM-clause
+ * column to one against the targetlist. Look to see if there is
+ * a matching column. If so, fall through to let transformExpr()
+ * do the rest. NOTE: if name could refer ambiguously to more
+ * than one column name exposed by FROM, colnameRangeTableEntry
+ * will elog(ERROR). That's just what we want here.
+ */
+ if (colnameRangeTableEntry(pstate, name) != NULL)
+ name = NULL;
+ }
- if (!resnode->resjunk &&
- strcmp(resnode->resname, name) == 0)
+ if (name != NULL)
+ {
+ foreach(tl, tlist)
{
- if (target_result != NULL)
+ TargetEntry *tle = (TargetEntry *) lfirst(tl);
+ Resdom *resnode = tle->resdom;
+
+ if (!resnode->resjunk &&
+ strcmp(resnode->resname, name) == 0)
{
- if (! equal(target_result->expr, tle->expr))
- elog(ERROR, "%s '%s' is ambiguous",
- clauseText[clause], name);
+ if (target_result != NULL)
+ {
+ if (! equal(target_result->expr, tle->expr))
+ elog(ERROR, "%s '%s' is ambiguous",
+ clauseText[clause], name);
+ }
+ else
+ target_result = tle;
+ /* Stay in loop to check for ambiguity */
}
- else
- target_result = tle;
- /* Stay in loop to check for ambiguity */
}
+ if (target_result != NULL)
+ return target_result; /* return the first match */
}
- if (target_result != NULL)
- return target_result; /* return the first match */
}
if (IsA(node, A_Const))
{
diff --git a/src/test/regress/expected/numerology.out b/src/test/regress/expected/numerology.out
index 8e13a9e6acb..c5ad36fdd32 100644
--- a/src/test/regress/expected/numerology.out
+++ b/src/test/regress/expected/numerology.out
@@ -96,14 +96,18 @@ SELECT f1 AS two, max(f3) AS max_float, min(f3) as min_float
2 | 0 | -1.2345678901234e+200
(2 rows)
--- Postgres used to accept this, but it is clearly against SQL92 to
--- interpret GROUP BY arguments as result column names; they should
--- be source column names *only*. An error is expected.
+-- GROUP BY a result column name is not legal per SQL92, but we accept it
+-- anyway (if the name is not the name of any column exposed by FROM).
SELECT f1 AS two, max(f3) AS max_float, min(f3) AS min_float
FROM TEMP_GROUP
GROUP BY two
ORDER BY two, max_float, min_float;
-ERROR: Attribute 'two' not found
+ two | max_float | min_float
+-----+----------------------+-----------------------
+ 1 | 1.2345678901234e+200 | 0
+ 2 | 0 | -1.2345678901234e+200
+(2 rows)
+
SELECT f1 AS two, (max(f3) + 1) AS max_plus_1, (min(f3) - 1) AS min_minus_1
FROM TEMP_GROUP
GROUP BY f1
diff --git a/src/test/regress/sql/numerology.sql b/src/test/regress/sql/numerology.sql
index b30f008bffb..2220fdba385 100644
--- a/src/test/regress/sql/numerology.sql
+++ b/src/test/regress/sql/numerology.sql
@@ -70,9 +70,8 @@ SELECT f1 AS two, max(f3) AS max_float, min(f3) as min_float
GROUP BY f1
ORDER BY two, max_float, min_float;
--- Postgres used to accept this, but it is clearly against SQL92 to
--- interpret GROUP BY arguments as result column names; they should
--- be source column names *only*. An error is expected.
+-- GROUP BY a result column name is not legal per SQL92, but we accept it
+-- anyway (if the name is not the name of any column exposed by FROM).
SELECT f1 AS two, max(f3) AS max_float, min(f3) AS min_float
FROM TEMP_GROUP
GROUP BY two
generated by cgit v1.2.3 (git 2.25.1) at 2025-06-16 18:07:01 +0000