Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitaeb9ae6

Browse files
committed
Disable physical tlist if any Var would need multiple sortgroupref labels.
As part of upper planner pathification (commit3fc6e2d) I redidcreateplan.c's approach to the physical-tlist optimization, in which scannodes are allowed to return exactly the underlying table's columns so asto save doing a projection step at runtime. The logic was intentionallymore aggressive than before about applying the optimization, which isgenerally a good thing, but Andres Freund found a case in which it gottoo aggressive. Namely, if any column is referenced more than once inthe parent plan node's sorting or grouping column list, we can't optimizebecause then that column would need to have more than one ressortgroupreflabel, and we only have space for one.Add logic to detect this situation in use_physical_tlist(), and also addsome error checking in apply_pathtarget_labeling_to_tlist(), which thisexample proves was being overly cavalier about whether what it was doingmade any sense.The added test case exposes the problem only because we do not eliminateduplicate grouping keys. That might be something to fix someday, but itdoesn't seem like appropriate post-beta work.Report: <20160526021235.w4nq7k3gnheg7vit@alap3.anarazel.de>
1 parentd7ef357 commitaeb9ae6

File tree

4 files changed

+65
-8
lines changed

4 files changed

+65
-8
lines changed

‎src/backend/optimizer/plan/createplan.c

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -787,10 +787,14 @@ use_physical_tlist(PlannerInfo *root, Path *path, int flags)
787787
* to emit any sort/group columns that are not simple Vars. (If they are
788788
* simple Vars, they should appear in the physical tlist, and
789789
* apply_pathtarget_labeling_to_tlist will take care of getting them
790-
* labeled again.)
790+
* labeled again.)We also have to check that no two sort/group columns
791+
* are the same Var, else that element of the physical tlist would need
792+
* conflicting ressortgroupref labels.
791793
*/
792794
if ((flags&CP_LABEL_TLIST)&&path->pathtarget->sortgrouprefs)
793795
{
796+
Bitmapset*sortgroupatts=NULL;
797+
794798
i=0;
795799
foreach(lc,path->pathtarget->exprs)
796800
{
@@ -799,7 +803,14 @@ use_physical_tlist(PlannerInfo *root, Path *path, int flags)
799803
if (path->pathtarget->sortgrouprefs[i])
800804
{
801805
if (expr&&IsA(expr,Var))
802-
/* okay */ ;
806+
{
807+
intattno= ((Var*)expr)->varattno;
808+
809+
attno-=FirstLowInvalidHeapAttributeNumber;
810+
if (bms_is_member(attno,sortgroupatts))
811+
return false;
812+
sortgroupatts=bms_add_member(sortgroupatts,attno);
813+
}
803814
else
804815
return false;
805816
}

‎src/backend/optimizer/util/tlist.c

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -736,17 +736,28 @@ apply_pathtarget_labeling_to_tlist(List *tlist, PathTarget *target)
736736
* this allows us to deal with some cases where a set-returning
737737
* function has been inlined, so that we now have more knowledge
738738
* about what it returns than we did when the original Var was
739-
* created. Otherwise, use regular equal() to see if there's a
740-
* matching TLE. (In current usage, only the Var case is actually
741-
* needed; but it seems best to have sane behavior here for
742-
* non-Vars too.)
739+
* created. Otherwise, use regular equal() to find the matching
740+
* TLE. (In current usage, only the Var case is actually needed;
741+
* but it seems best to have sane behavior here for non-Vars too.)
743742
*/
744743
if (expr&&IsA(expr,Var))
745744
tle=tlist_member_match_var((Var*)expr,tlist);
746745
else
747746
tle=tlist_member((Node*)expr,tlist);
748-
if (tle)
749-
tle->ressortgroupref=target->sortgrouprefs[i];
747+
748+
/*
749+
* Complain if noplace for the sortgrouprefs label, or if we'd
750+
* have to label a column twice. (The case where it already has
751+
* the desired label probably can't happen, but we may as well
752+
* allow for it.)
753+
*/
754+
if (!tle)
755+
elog(ERROR,"ORDER/GROUP BY expression not found in targetlist");
756+
if (tle->ressortgroupref!=0&&
757+
tle->ressortgroupref!=target->sortgrouprefs[i])
758+
elog(ERROR,"targetlist item has multiple sortgroupref labels");
759+
760+
tle->ressortgroupref=target->sortgrouprefs[i];
750761
}
751762
i++;
752763
}

‎src/test/regress/expected/select_distinct.out

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,30 @@ SELECT DISTINCT p.age FROM person* p ORDER BY age using >;
124124
8
125125
(20 rows)
126126

127+
--
128+
-- Check mentioning same column more than once
129+
--
130+
EXPLAIN (VERBOSE, COSTS OFF)
131+
SELECT count(*) FROM
132+
(SELECT DISTINCT two, four, two FROM tenk1) ss;
133+
QUERY PLAN
134+
--------------------------------------------------------
135+
Aggregate
136+
Output: count(*)
137+
-> HashAggregate
138+
Output: tenk1.two, tenk1.four, tenk1.two
139+
Group Key: tenk1.two, tenk1.four, tenk1.two
140+
-> Seq Scan on public.tenk1
141+
Output: tenk1.two, tenk1.four, tenk1.two
142+
(7 rows)
143+
144+
SELECT count(*) FROM
145+
(SELECT DISTINCT two, four, two FROM tenk1) ss;
146+
count
147+
-------
148+
4
149+
(1 row)
150+
127151
--
128152
-- Also, some tests of IS DISTINCT FROM, which doesn't quite deserve its
129153
-- very own regression file.

‎src/test/regress/sql/select_distinct.sql

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,17 @@ SELECT DISTINCT two, string4, ten
3434
--
3535
SELECT DISTINCTp.ageFROM person* pORDER BY age using>;
3636

37+
--
38+
-- Check mentioning same column more than once
39+
--
40+
41+
EXPLAIN (VERBOSE, COSTS OFF)
42+
SELECTcount(*)FROM
43+
(SELECT DISTINCT two, four, twoFROM tenk1) ss;
44+
45+
SELECTcount(*)FROM
46+
(SELECT DISTINCT two, four, twoFROM tenk1) ss;
47+
3748
--
3849
-- Also, some tests of IS DISTINCT FROM, which doesn't quite deserve its
3950
-- very own regression file.

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp