Make setrefs.c match by ressortgroupref even for plain Vars.

Previously, we skipped using search_indexed_tlist_for_sortgroupref()
if the tlist expression being sought in the child plan node was merely
a Var.  This is purely an optimization, based on the theory that
search_indexed_tlist_for_var() is faster, and one copy of a Var should
be as good as another.  However, the GROUPING SETS patch broke the
latter assumption: grouping columns containing the "same" Var can
sometimes have different outputs, as shown in the test case added here.
So do it the hard way whenever a ressortgroupref marking exists.

(If this seems like a bottleneck, we could imagine building a tlist index
data structure for ressortgroupref values, as we do for Vars.  But I'll
let that idea go until there's some evidence it's worthwhile.)

Back-patch to 9.6.  The problem also exists in 9.5 where GROUPING SETS
came in, but this patch is insufficient to resolve the problem in 9.5:
there is some obscure dependency on the upper-planner-pathification
work that happened in 9.6.  Given that this is such a weird corner case,
and no end users have complained about it, it doesn't seem worth the work
to develop a fix for 9.5.

Patch by me, per a report from Heikki Linnakangas.  (This does not fix
Heikki's original complaint, just the follow-on one.)

Discussion: https://postgr.es/m/aefc657e-edb2-64d5-6df1-a0828f6e9104@iki.fi

diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index d91bc3b30de5c80f9112f52003cb794de72688fe..ec376c05650daec1230a9b0fe6c749ddd6cc84d7 100644 (file)
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -1687,8 +1687,8 @@ set_upper_references(PlannerInfo *root, Plan *plan, int rtoffset)
                TargetEntry *tle = (TargetEntry *) lfirst(l);
                Node       *newexpr;
 
-               /* If it's a non-Var sort/group item, first try to match by sortref */
-               if (tle->ressortgroupref != 0 && !IsA(tle->expr, Var))
+               /* If it's a sort/group item, first try to match by sortref */
+               if (tle->ressortgroupref != 0)
                {
                        newexpr = (Node *)
                                search_indexed_tlist_for_sortgroupref((Node *) tle->expr,
@@ -2049,7 +2049,6 @@ search_indexed_tlist_for_non_var(Node *node,
 
 /*
  * search_indexed_tlist_for_sortgroupref --- find a sort/group expression
- *             (which is assumed not to be just a Var)
  *
  * If a match is found, return a Var constructed to reference the tlist item.
  * If no match, return NULL.

diff --git a/src/test/regress/expected/groupingsets.out b/src/test/regress/expected/groupingsets.out
index 260ccd52c87b03831133d1679e28221c9f0aca84..091aada090038c71fd49ae329a132723386fd5d0 100644 (file)
--- a/src/test/regress/expected/groupingsets.out
+++ b/src/test/regress/expected/groupingsets.out
@@ -352,6 +352,35 @@ select a, d, grouping(a,b,c)
  2 | 2 |        2
 (4 rows)
 
+-- check that distinct grouping columns are kept separate
+-- even if they are equal()
+explain (costs off)
+select g as alias1, g as alias2
+  from generate_series(1,3) g
+ group by alias1, rollup(alias2);
+                   QUERY PLAN                   
+------------------------------------------------
+ GroupAggregate
+   Group Key: g, g
+   Group Key: g
+   ->  Sort
+         Sort Key: g
+         ->  Function Scan on generate_series g
+(6 rows)
+
+select g as alias1, g as alias2
+  from generate_series(1,3) g
+ group by alias1, rollup(alias2);
+ alias1 | alias2 
+--------+--------
+      1 |      1
+      1 |       
+      2 |      2
+      2 |       
+      3 |      3
+      3 |       
+(6 rows)
+
 -- simple rescan tests
 select a, b, sum(v.x)
   from (values (1),(2)) v(x), gstest_data(v.x)

diff --git a/src/test/regress/sql/groupingsets.sql b/src/test/regress/sql/groupingsets.sql
index 71cc0ec90077c16ee62c38323fb9195b32a80308..a9938d57b7d4258a73b45d602443f6f75fdfa6e2 100644 (file)
--- a/src/test/regress/sql/groupingsets.sql
+++ b/src/test/regress/sql/groupingsets.sql
@@ -130,6 +130,17 @@ select a, d, grouping(a,b,c)
   from gstest3
  group by grouping sets ((a,b), (a,c));
 
+-- check that distinct grouping columns are kept separate
+-- even if they are equal()
+explain (costs off)
+select g as alias1, g as alias2
+  from generate_series(1,3) g
+ group by alias1, rollup(alias2);
+
+select g as alias1, g as alias2
+  from generate_series(1,3) g
+ group by alias1, rollup(alias2);
+
 -- simple rescan tests
 
 select a, b, sum(v.x)