diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2017-10-26 12:17:40 -0400 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2017-10-26 12:17:40 -0400 |
commit | 37b4e0fe9964fc23d6a38973eaf67b287ac199ca (patch) | |
tree | 21fccf546a0798b18a95bb00a32f55e3c2562c3d /src | |
parent | ea8480a2493e396715e2fa0709c137b513a680aa (diff) | |
download | postgresql-37b4e0fe9964fc23d6a38973eaf67b287ac199ca.tar.gz postgresql-37b4e0fe9964fc23d6a38973eaf67b287ac199ca.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
Diffstat (limited to 'src')
-rw-r--r-- | src/backend/optimizer/plan/setrefs.c | 5 | ||||
-rw-r--r-- | src/test/regress/expected/groupingsets.out | 29 | ||||
-rw-r--r-- | src/test/regress/sql/groupingsets.sql | 11 |
3 files changed, 42 insertions, 3 deletions
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c index d91bc3b30de..ec376c05650 100644 --- 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 260ccd52c87..091aada0900 100644 --- 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 71cc0ec9007..a9938d57b7d 100644 --- 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) |