Skip to content

Commit cde35cf

Browse files
committed
Fix eclass_useful_for_merging to give valid results for appendrel children.
Formerly, this function would always return "true" for an appendrel child relation, because it would think that the appendrel parent was a potential join target for the child. In principle that should only lead to some inefficiency in planning, but fuzz testing by Andreas Seltenreich disclosed that it could lead to "could not find pathkey item to sort" planner errors in odd corner cases. Specifically, we would think that all columns of a child table's multicolumn index were interesting pathkeys, causing us to generate a MergeAppend path that sorts by all the columns. However, if any of those columns weren't actually used above the level of the appendrel, they would not get added to that rel's targetlist, which would result in being unable to resolve the MergeAppend's sort keys against its targetlist during createplan.c. Backpatch to 9.3. In older versions, columns of an appendrel get added to its targetlist even if they're not mentioned above the scan level, so that the failure doesn't occur. It might be worth back-patching this fix to older versions anyway, but I'll refrain for the moment.
1 parent c9351f0 commit cde35cf

File tree

5 files changed

+69
-5
lines changed

5 files changed

+69
-5
lines changed

src/backend/optimizer/path/equivclass.c

+11-3
Original file line numberDiff line numberDiff line change
@@ -2285,9 +2285,11 @@ has_relevant_eclass_joinclause(PlannerInfo *root, RelOptInfo *rel1)
22852285
* from actually being generated.
22862286
*/
22872287
bool
2288-
eclass_useful_for_merging(EquivalenceClass *eclass,
2288+
eclass_useful_for_merging(PlannerInfo *root,
2289+
EquivalenceClass *eclass,
22892290
RelOptInfo *rel)
22902291
{
2292+
Relids relids;
22912293
ListCell *lc;
22922294

22932295
Assert(!eclass->ec_merged);
@@ -2305,8 +2307,14 @@ eclass_useful_for_merging(EquivalenceClass *eclass,
23052307
* possibly-overoptimistic heuristic.
23062308
*/
23072309

2310+
/* If specified rel is a child, we must consider the topmost parent rel */
2311+
if (rel->reloptkind == RELOPT_OTHER_MEMBER_REL)
2312+
relids = find_childrel_top_parent(root, rel)->relids;
2313+
else
2314+
relids = rel->relids;
2315+
23082316
/* If rel already includes all members of eclass, no point in searching */
2309-
if (bms_is_subset(eclass->ec_relids, rel->relids))
2317+
if (bms_is_subset(eclass->ec_relids, relids))
23102318
return false;
23112319

23122320
/* To join, we need a member not in the given rel */
@@ -2317,7 +2325,7 @@ eclass_useful_for_merging(EquivalenceClass *eclass,
23172325
if (cur_em->em_is_child)
23182326
continue; /* ignore children here */
23192327

2320-
if (!bms_overlap(cur_em->em_relids, rel->relids))
2328+
if (!bms_overlap(cur_em->em_relids, relids))
23212329
return true;
23222330
}
23232331

src/backend/optimizer/path/pathkeys.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -1373,7 +1373,7 @@ pathkeys_useful_for_merging(PlannerInfo *root, RelOptInfo *rel, List *pathkeys)
13731373
* surely possible to generate a mergejoin clause using them.
13741374
*/
13751375
if (rel->has_eclass_joins &&
1376-
eclass_useful_for_merging(pathkey->pk_eclass, rel))
1376+
eclass_useful_for_merging(root, pathkey->pk_eclass, rel))
13771377
matched = true;
13781378
else
13791379
{

src/include/optimizer/paths.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,8 @@ extern bool have_relevant_eclass_joinclause(PlannerInfo *root,
146146
RelOptInfo *rel1, RelOptInfo *rel2);
147147
extern bool has_relevant_eclass_joinclause(PlannerInfo *root,
148148
RelOptInfo *rel1);
149-
extern bool eclass_useful_for_merging(EquivalenceClass *eclass,
149+
extern bool eclass_useful_for_merging(PlannerInfo *root,
150+
EquivalenceClass *eclass,
150151
RelOptInfo *rel);
151152
extern bool is_redundant_derived_clause(RestrictInfo *rinfo, List *clauselist);
152153

src/test/regress/expected/inherit.out

+34
Original file line numberDiff line numberDiff line change
@@ -1307,6 +1307,40 @@ DETAIL: drop cascades to table matest1
13071307
drop cascades to table matest2
13081308
drop cascades to table matest3
13091309
--
1310+
-- Check that use of an index with an extraneous column doesn't produce
1311+
-- a plan with extraneous sorting
1312+
--
1313+
create table matest0 (a int, b int, c int, d int);
1314+
create table matest1 () inherits(matest0);
1315+
create index matest0i on matest0 (b, c);
1316+
create index matest1i on matest1 (b, c);
1317+
set enable_nestloop = off; -- we want a plan with two MergeAppends
1318+
explain (costs off)
1319+
select t1.* from matest0 t1, matest0 t2
1320+
where t1.b = t2.b and t2.c = t2.d
1321+
order by t1.b limit 10;
1322+
QUERY PLAN
1323+
-------------------------------------------------------------------
1324+
Limit
1325+
-> Merge Join
1326+
Merge Cond: (t1.b = t2.b)
1327+
-> Merge Append
1328+
Sort Key: t1.b
1329+
-> Index Scan using matest0i on matest0 t1
1330+
-> Index Scan using matest1i on matest1 t1_1
1331+
-> Materialize
1332+
-> Merge Append
1333+
Sort Key: t2.b
1334+
-> Index Scan using matest0i on matest0 t2
1335+
Filter: (c = d)
1336+
-> Index Scan using matest1i on matest1 t2_1
1337+
Filter: (c = d)
1338+
(14 rows)
1339+
1340+
reset enable_nestloop;
1341+
drop table matest0 cascade;
1342+
NOTICE: drop cascades to table matest1
1343+
--
13101344
-- Test merge-append for UNION ALL append relations
13111345
--
13121346
set enable_seqscan = off;

src/test/regress/sql/inherit.sql

+21
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,27 @@ reset enable_seqscan;
402402

403403
drop table matest0 cascade;
404404

405+
--
406+
-- Check that use of an index with an extraneous column doesn't produce
407+
-- a plan with extraneous sorting
408+
--
409+
410+
create table matest0 (a int, b int, c int, d int);
411+
create table matest1 () inherits(matest0);
412+
create index matest0i on matest0 (b, c);
413+
create index matest1i on matest1 (b, c);
414+
415+
set enable_nestloop = off; -- we want a plan with two MergeAppends
416+
417+
explain (costs off)
418+
select t1.* from matest0 t1, matest0 t2
419+
where t1.b = t2.b and t2.c = t2.d
420+
order by t1.b limit 10;
421+
422+
reset enable_nestloop;
423+
424+
drop table matest0 cascade;
425+
405426
--
406427
-- Test merge-append for UNION ALL append relations
407428
--

0 commit comments

Comments
 (0)