Allow ORDER BY/GROUP BY/etc items to match targetlist items regardless of

tglsfdc · tglsfdc · commit fba999cb2c8a · 2010-07-18T19:37:49.000Z
any implicit casting previously applied to the targetlist item.  This is
reasonable because the implicit cast, by definition, wasn't written by the
user; so we are preserving the expected behavior that ORDER BY items match
textually equivalent tlist items.  The case never arose before because there
couldn't be any implicit casting of a top-level SELECT item before we process
ORDER BY etc.  But now it can arise in the context of aggregates containing
ORDER BY clauses, since the "targetlist" is the already-casted list of
arguments for the aggregate.  The net effect is that the datatype used for
ORDER BY/DISTINCT purposes is the aggregate's declared input type, not that
of the original input column; which is a bit debatable but not horrendous,
and to do otherwise would require major rework that doesn't seem justified.

Per bug #5564 from Daniel Grace.  Back-patch to 9.0 where aggregate ORDER BY
was implemented.
diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/parser/parse_clause.c,v 1.198 2010/02/26 02:00:50 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/parser/parse_clause.c,v 1.199 2010/07/18 19:37:48 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -21,6 +21,7 @@
 #include "commands/defrem.h"
 #include "nodes/makefuncs.h"
 #include "nodes/nodeFuncs.h"
+#include "optimizer/clauses.h"
 #include "optimizer/tlist.h"
 #include "optimizer/var.h"
 #include "parser/analyze.h"
@@ -1430,8 +1431,20 @@ findTargetlistEntrySQL99(ParseState *pstate, Node *node, List **tlist)
 	foreach(tl, *tlist)
 	{
 		TargetEntry *tle = (TargetEntry *) lfirst(tl);
+		Node	   *texpr;
 
-		if (equal(expr, tle->expr))
+		/*
+		 * Ignore any implicit cast on the existing tlist expression.
+		 *
+		 * This essentially allows the ORDER/GROUP/etc item to adopt the same
+		 * datatype previously selected for a textually-equivalent tlist item.
+		 * There can't be any implicit cast at top level in an ordinary SELECT
+		 * tlist at this stage, but the case does arise with ORDER BY in an
+		 * aggregate function.
+		 */
+		texpr = strip_implicit_coercions((Node *) tle->expr);
+
+		if (equal(expr, texpr))
 			return tle;
 	}
 
diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out
@@ -830,3 +830,24 @@ select string_agg(a,',') from (values(null),(null)) g(a);
  
 (1 row)
 
+-- check some implicit casting cases, as per bug #5564
+select string_agg(distinct f1 order by f1) from varchar_tbl;  -- ok
+ string_agg 
+------------
+ aababcd
+(1 row)
+
+select string_agg(distinct f1::text order by f1) from varchar_tbl;  -- not ok
+ERROR:  in an aggregate with DISTINCT, ORDER BY expressions must appear in argument list
+LINE 1: select string_agg(distinct f1::text order by f1) from varcha...
+                                                     ^
+select string_agg(distinct f1 order by f1::text) from varchar_tbl;  -- not ok
+ERROR:  in an aggregate with DISTINCT, ORDER BY expressions must appear in argument list
+LINE 1: select string_agg(distinct f1 order by f1::text) from varcha...
+                                               ^
+select string_agg(distinct f1::text order by f1::text) from varchar_tbl;  -- ok
+ string_agg 
+------------
+ aababcd
+(1 row)
+
diff --git a/src/test/regress/sql/aggregates.sql b/src/test/regress/sql/aggregates.sql
@@ -362,3 +362,9 @@ select string_agg(a,',') from (values('aaaa'),('bbbb'),('cccc')) g(a);
 select string_agg(a,',') from (values('aaaa'),(null),('bbbb'),('cccc')) g(a);
 select string_agg(a,',') from (values(null),(null),('bbbb'),('cccc')) g(a);
 select string_agg(a,',') from (values(null),(null)) g(a);
+
+-- check some implicit casting cases, as per bug #5564
+select string_agg(distinct f1 order by f1) from varchar_tbl;  -- ok
+select string_agg(distinct f1::text order by f1) from varchar_tbl;  -- not ok
+select string_agg(distinct f1 order by f1::text) from varchar_tbl;  -- not ok
+select string_agg(distinct f1::text order by f1::text) from varchar_tbl;  -- ok
