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

Commit57664ed

Browse files
committed
Wrap appendrel member outputs in PlaceHolderVars in additional cases.
Add PlaceHolderVar wrappers as needed to make UNION ALL sub-select outputexpressions appear non-constant and distinct from each other. This makesthe world safe for add_child_rel_equivalences to do what it does. Before,it was possible for that function to add identical expressions to differentEquivalenceClasses, which logically should imply merging such ECs, whichwould be wrong; or to improperly add a constant to an EquivalenceClass,drastically changing its behavior. Per report from Teodor Sigaev.The only currently known consequence of this bug is "MergeAppend child'stargetlist doesn't match MergeAppend" planner failures in 9.1 and later.I am suspicious that there may be other failure modes that could affectolder release branches; but in the absence of any hard evidence, I'llrefrain from back-patching further than 9.1.
1 parent3b81617 commit57664ed

File tree

6 files changed

+134
-22
lines changed

6 files changed

+134
-22
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ query_planner(PlannerInfo *root, List *tlist,
176176
*/
177177
build_base_rel_tlists(root,tlist);
178178

179-
find_placeholders_in_jointree(root);
179+
find_placeholders_in_query(root);
180180

181181
joinlist=deconstruct_jointree(root);
182182

‎src/backend/optimizer/prep/prepjointree.c

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -784,22 +784,15 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte,
784784
parse->havingQual=pullup_replace_vars(parse->havingQual,&rvcontext);
785785

786786
/*
787-
* Replace references in the translated_vars lists of appendrels. When
788-
* pulling up an appendrel member, we do not need PHVs in the list of the
789-
* parent appendrel --- there isn't any outer join between. Elsewhere, use
790-
* PHVs for safety. (This analysis could be made tighter but it seems
791-
* unlikely to be worth much trouble.)
787+
* Replace references in the translated_vars lists of appendrels, too.
788+
* We do it this way because we must preserve the AppendRelInfo structs.
792789
*/
793790
foreach(lc,root->append_rel_list)
794791
{
795792
AppendRelInfo*appinfo= (AppendRelInfo*)lfirst(lc);
796-
boolsave_need_phvs=rvcontext.need_phvs;
797793

798-
if (appinfo==containing_appendrel)
799-
rvcontext.need_phvs= false;
800794
appinfo->translated_vars= (List*)
801795
pullup_replace_vars((Node*)appinfo->translated_vars,&rvcontext);
802-
rvcontext.need_phvs=save_need_phvs;
803796
}
804797

805798
/*
@@ -1407,14 +1400,31 @@ pullup_replace_vars_callback(Var *var,
14071400
if (newnode&&IsA(newnode,Var)&&
14081401
((Var*)newnode)->varlevelsup==0)
14091402
{
1410-
/* Simple Vars always escape being wrapped */
1411-
wrap= false;
1403+
/*
1404+
* Simple Vars normally escape being wrapped. However, in
1405+
* wrap_non_vars mode (ie, we are dealing with an appendrel
1406+
* member), we must ensure that each tlist entry expands to a
1407+
* distinct expression, else we may have problems with
1408+
* improperly placing identical entries into different
1409+
* EquivalenceClasses. Therefore, we wrap a Var in a
1410+
* PlaceHolderVar if it duplicates any earlier entry in the
1411+
* tlist (ie, we've got "SELECT x, x, ..."). Since each PHV
1412+
* is distinct, this fixes the ambiguity. We can use
1413+
* tlist_member to detect whether there's an earlier
1414+
* duplicate.
1415+
*/
1416+
wrap= (rcon->wrap_non_vars&&
1417+
tlist_member(newnode,rcon->targetlist)!=tle);
14121418
}
14131419
elseif (newnode&&IsA(newnode,PlaceHolderVar)&&
14141420
((PlaceHolderVar*)newnode)->phlevelsup==0)
14151421
{
1416-
/* No need to wrap a PlaceHolderVar with another one, either */
1417-
wrap= false;
1422+
/*
1423+
* No need to directly wrap a PlaceHolderVar with another one,
1424+
* either, unless we need to prevent duplication.
1425+
*/
1426+
wrap= (rcon->wrap_non_vars&&
1427+
tlist_member(newnode,rcon->targetlist)!=tle);
14181428
}
14191429
elseif (rcon->wrap_non_vars)
14201430
{

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

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -104,28 +104,41 @@ find_placeholder_info(PlannerInfo *root, PlaceHolderVar *phv,
104104
}
105105

106106
/*
107-
*find_placeholders_in_jointree
108-
*Search thejointree for PlaceHolderVars, and build PlaceHolderInfos
107+
*find_placeholders_in_query
108+
*Search thequery for PlaceHolderVars, and build PlaceHolderInfos
109109
*
110-
* We don't need to look at the targetlist because build_base_rel_tlists()
111-
* will already have made entries for any PHVs in the tlist.
110+
* We need to examine the jointree, but not the targetlist, because
111+
* build_base_rel_tlists() will already have made entries for any PHVs
112+
* in the targetlist.
113+
*
114+
* We also need to search for PHVs in AppendRelInfo translated_vars
115+
* lists. In most cases, translated_vars entries aren't directly referenced
116+
* elsewhere, but we need to create PlaceHolderInfo entries for them to
117+
* support set_rel_width() calculations for the appendrel child relations.
112118
*/
113119
void
114-
find_placeholders_in_jointree(PlannerInfo*root)
120+
find_placeholders_in_query(PlannerInfo*root)
115121
{
116122
/* We need do nothing if the query contains no PlaceHolderVars */
117123
if (root->glob->lastPHId!=0)
118124
{
119-
/*Start recursion at top of jointree */
125+
/*Recursively search the jointree */
120126
Assert(root->parse->jointree!=NULL&&
121127
IsA(root->parse->jointree,FromExpr));
122128
(void)find_placeholders_recurse(root, (Node*)root->parse->jointree);
129+
130+
/*
131+
* Also search the append_rel_list for translated vars that are PHVs.
132+
* Barring finding them elsewhere in the query, they do not need any
133+
* ph_may_need bits, only to be present in the PlaceHolderInfo list.
134+
*/
135+
mark_placeholders_in_expr(root, (Node*)root->append_rel_list,NULL);
123136
}
124137
}
125138

126139
/*
127140
* find_placeholders_recurse
128-
* One recursion level offind_placeholders_in_jointree.
141+
* One recursion level ofjointree search for find_placeholders_in_query.
129142
*
130143
* jtnode is the current jointree node to examine.
131144
*

‎src/include/optimizer/placeholder.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ extern PlaceHolderVar *make_placeholder_expr(PlannerInfo *root, Expr *expr,
2121
Relidsphrels);
2222
externPlaceHolderInfo*find_placeholder_info(PlannerInfo*root,
2323
PlaceHolderVar*phv,boolcreate_new_ph);
24-
externvoidfind_placeholders_in_jointree(PlannerInfo*root);
24+
externvoidfind_placeholders_in_query(PlannerInfo*root);
2525
externvoidmark_placeholder_maybe_needed(PlannerInfo*root,
2626
PlaceHolderInfo*phinfo,Relidsrelids);
2727
externvoidupdate_placeholder_eval_levels(PlannerInfo*root,

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

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1242,3 +1242,61 @@ NOTICE: drop cascades to 3 other objects
12421242
DETAIL: drop cascades to table matest1
12431243
drop cascades to table matest2
12441244
drop cascades to table matest3
1245+
--
1246+
-- Test merge-append for UNION ALL append relations
1247+
-- Check handling of duplicated, constant, or volatile targetlist items
1248+
--
1249+
set enable_seqscan = off;
1250+
set enable_indexscan = on;
1251+
set enable_bitmapscan = off;
1252+
explain (costs off)
1253+
SELECT thousand, tenthous FROM tenk1
1254+
UNION ALL
1255+
SELECT thousand, thousand FROM tenk1
1256+
ORDER BY thousand, tenthous;
1257+
QUERY PLAN
1258+
-----------------------------------------------------------------------
1259+
Result
1260+
-> Merge Append
1261+
Sort Key: public.tenk1.thousand, public.tenk1.tenthous
1262+
-> Index Only Scan using tenk1_thous_tenthous on tenk1
1263+
-> Sort
1264+
Sort Key: public.tenk1.thousand, public.tenk1.thousand
1265+
-> Index Only Scan using tenk1_thous_tenthous on tenk1
1266+
(7 rows)
1267+
1268+
explain (costs off)
1269+
SELECT thousand, tenthous FROM tenk1
1270+
UNION ALL
1271+
SELECT 42, 42 FROM tenk1
1272+
ORDER BY thousand, tenthous;
1273+
QUERY PLAN
1274+
-----------------------------------------------------------------------
1275+
Result
1276+
-> Merge Append
1277+
Sort Key: public.tenk1.thousand, public.tenk1.tenthous
1278+
-> Index Only Scan using tenk1_thous_tenthous on tenk1
1279+
-> Sort
1280+
Sort Key: (42), (42)
1281+
-> Index Only Scan using tenk1_thous_tenthous on tenk1
1282+
(7 rows)
1283+
1284+
explain (costs off)
1285+
SELECT thousand, tenthous FROM tenk1
1286+
UNION ALL
1287+
SELECT thousand, random()::integer FROM tenk1
1288+
ORDER BY thousand, tenthous;
1289+
QUERY PLAN
1290+
-----------------------------------------------------------------------
1291+
Result
1292+
-> Merge Append
1293+
Sort Key: public.tenk1.thousand, public.tenk1.tenthous
1294+
-> Index Only Scan using tenk1_thous_tenthous on tenk1
1295+
-> Sort
1296+
Sort Key: public.tenk1.thousand, ((random())::integer)
1297+
-> Index Only Scan using tenk1_thous_tenthous on tenk1
1298+
(7 rows)
1299+
1300+
reset enable_seqscan;
1301+
reset enable_indexscan;
1302+
reset enable_bitmapscan;

‎src/test/regress/sql/inherit.sql

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -406,3 +406,34 @@ select * from matest0 order by 1-id;
406406
reset enable_seqscan;
407407

408408
droptable matest0 cascade;
409+
410+
--
411+
-- Test merge-append for UNION ALL append relations
412+
-- Check handling of duplicated, constant, or volatile targetlist items
413+
--
414+
415+
set enable_seqscan= off;
416+
set enable_indexscan=on;
417+
set enable_bitmapscan= off;
418+
419+
explain (costs off)
420+
SELECT thousand, tenthousFROM tenk1
421+
UNION ALL
422+
SELECT thousand, thousandFROM tenk1
423+
ORDER BY thousand, tenthous;
424+
425+
explain (costs off)
426+
SELECT thousand, tenthousFROM tenk1
427+
UNION ALL
428+
SELECT42,42FROM tenk1
429+
ORDER BY thousand, tenthous;
430+
431+
explain (costs off)
432+
SELECT thousand, tenthousFROM tenk1
433+
UNION ALL
434+
SELECT thousand, random()::integerFROM tenk1
435+
ORDER BY thousand, tenthous;
436+
437+
reset enable_seqscan;
438+
reset enable_indexscan;
439+
reset enable_bitmapscan;

0 commit comments

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