aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2017-10-26 12:17:40 -0400
committerTom Lane <tgl@sss.pgh.pa.us>2017-10-26 12:17:40 -0400
commit6a81ba1d4d26b46636d652a3a56143c159da899c (patch)
treeb35a8f4777775cd92f7e27f5de84ed662c40e5f5
parenta9d4625f485abd458f8fcc263f0003235430401e (diff)
downloadpostgresql-6a81ba1d4d26b46636d652a3a56143c159da899c.tar.gz
postgresql-6a81ba1d4d26b46636d652a3a56143c159da899c.zip
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
-rw-r--r--src/backend/optimizer/plan/setrefs.c5
-rw-r--r--src/test/regress/expected/groupingsets.out29
-rw-r--r--src/test/regress/sql/groupingsets.sql11
3 files changed, 42 insertions, 3 deletions
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index b0c9e944597..90c0bb4a195 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -1746,8 +1746,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(tle->expr,
@@ -2108,7 +2108,6 @@ search_indexed_tlist_for_non_var(Expr *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 fd618afe603..833d5151743 100644
--- a/src/test/regress/expected/groupingsets.out
+++ b/src/test/regress/expected/groupingsets.out
@@ -360,6 +360,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 564ebc9b051..2b4ab692c4f 100644
--- a/src/test/regress/sql/groupingsets.sql
+++ b/src/test/regress/sql/groupingsets.sql
@@ -141,6 +141,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)