Skip to content

Commit f069c91

Browse files
committed
Fix possible crash in partition-wise join.
The previous code assumed that we'd always succeed in creating child-joins for a joinrel for which partition-wise join was considered, but that's not guaranteed, at least in the case where dummy rels are involved. Ashutosh Bapat, with some wordsmithing by me. Discussion: http://postgr.es/m/CAFjFpRf8=uyMYYfeTBjWDMs1tR5t--FgOe2vKZPULxxdYQ4RNw@mail.gmail.com
1 parent 1eb5d43 commit f069c91

File tree

6 files changed

+43
-55
lines changed

6 files changed

+43
-55
lines changed

src/backend/optimizer/path/allpaths.c

+4-14
Original file line numberDiff line numberDiff line change
@@ -3425,20 +3425,8 @@ generate_partition_wise_join_paths(PlannerInfo *root, RelOptInfo *rel)
34253425
if (!IS_JOIN_REL(rel))
34263426
return;
34273427

3428-
/*
3429-
* If we've already proven this join is empty, we needn't consider any
3430-
* more paths for it.
3431-
*/
3432-
if (IS_DUMMY_REL(rel))
3433-
return;
3434-
3435-
/*
3436-
* We've nothing to do if the relation is not partitioned. An outer join
3437-
* relation which had an empty inner relation in every pair will have the
3438-
* rest of the partitioning properties set except the child-join
3439-
* RelOptInfos. See try_partition_wise_join() for more details.
3440-
*/
3441-
if (rel->nparts <= 0 || rel->part_rels == NULL)
3428+
/* We've nothing to do if the relation is not partitioned. */
3429+
if (!IS_PARTITIONED_REL(rel))
34423430
return;
34433431

34443432
/* Guard against stack overflow due to overly deep partition hierarchy. */
@@ -3452,6 +3440,8 @@ generate_partition_wise_join_paths(PlannerInfo *root, RelOptInfo *rel)
34523440
{
34533441
RelOptInfo *child_rel = part_rels[cnt_parts];
34543442

3443+
Assert(child_rel != NULL);
3444+
34553445
/* Add partition-wise join paths for partitioned child-joins. */
34563446
generate_partition_wise_join_paths(root, child_rel);
34573447

src/backend/optimizer/path/joinrels.c

-16
Original file line numberDiff line numberDiff line change
@@ -1318,17 +1318,6 @@ try_partition_wise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
13181318
if (!IS_PARTITIONED_REL(joinrel))
13191319
return;
13201320

1321-
/*
1322-
* set_rel_pathlist() may not create paths in children of an empty
1323-
* partitioned table and so we can not add paths to child-joins. So, deem
1324-
* such a join as unpartitioned. When a partitioned relation is deemed
1325-
* empty because all its children are empty, dummy path will be set in
1326-
* each of the children. In such a case we could still consider the join
1327-
* as partitioned, but it might not help much.
1328-
*/
1329-
if (IS_DUMMY_REL(rel1) || IS_DUMMY_REL(rel2))
1330-
return;
1331-
13321321
/*
13331322
* Since this join relation is partitioned, all the base relations
13341323
* participating in this join must be partitioned and so are all the
@@ -1360,11 +1349,6 @@ try_partition_wise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
13601349

13611350
nparts = joinrel->nparts;
13621351

1363-
/* Allocate space to hold child-joins RelOptInfos, if not already done. */
1364-
if (!joinrel->part_rels)
1365-
joinrel->part_rels =
1366-
(RelOptInfo **) palloc0(sizeof(RelOptInfo *) * nparts);
1367-
13681352
/*
13691353
* Create child-join relations for this partitioned join, if those don't
13701354
* exist. Add paths to child-joins for a pair of child relations

src/backend/optimizer/util/relnode.c

+4-1
Original file line numberDiff line numberDiff line change
@@ -1662,11 +1662,14 @@ build_joinrel_partition_info(RelOptInfo *joinrel, RelOptInfo *outer_rel,
16621662
*/
16631663
joinrel->part_scheme = part_scheme;
16641664
joinrel->boundinfo = outer_rel->boundinfo;
1665-
joinrel->nparts = outer_rel->nparts;
16661665
partnatts = joinrel->part_scheme->partnatts;
16671666
joinrel->partexprs = (List **) palloc0(sizeof(List *) * partnatts);
16681667
joinrel->nullable_partexprs =
16691668
(List **) palloc0(sizeof(List *) * partnatts);
1669+
joinrel->nparts = outer_rel->nparts;
1670+
joinrel->part_rels =
1671+
(RelOptInfo **) palloc0(sizeof(RelOptInfo *) * joinrel->nparts);
1672+
16701673

16711674
/*
16721675
* Construct partition keys for the join.

src/include/nodes/relation.h

+9-5
Original file line numberDiff line numberDiff line change
@@ -666,13 +666,17 @@ typedef struct RelOptInfo
666666
/*
667667
* Is given relation partitioned?
668668
*
669-
* A join between two partitioned relations with same partitioning scheme
670-
* without any matching partitions will not have any partition in it but will
671-
* have partition scheme set. So a relation is deemed to be partitioned if it
672-
* has a partitioning scheme, bounds and positive number of partitions.
669+
* It's not enough to test whether rel->part_scheme is set, because it might
670+
* be that the basic partitioning properties of the input relations matched
671+
* but the partition bounds did not.
672+
*
673+
* We treat dummy relations as unpartitioned. We could alternatively
674+
* treat them as partitioned, but it's not clear whether that's a useful thing
675+
* to do.
673676
*/
674677
#define IS_PARTITIONED_REL(rel) \
675-
((rel)->part_scheme && (rel)->boundinfo && (rel)->nparts > 0)
678+
((rel)->part_scheme && (rel)->boundinfo && (rel)->nparts > 0 && \
679+
(rel)->part_rels && !(IS_DUMMY_REL(rel)))
676680

677681
/*
678682
* Convenience macro to make sure that a partitioned relation has all the

src/test/regress/expected/partition_join.out

+25-18
Original file line numberDiff line numberDiff line change
@@ -1217,24 +1217,31 @@ SELECT t1.a, t1.c, t2.b, t2.c FROM (SELECT * FROM prt1 WHERE a = 1 AND a = 2) t1
12171217
(2 rows)
12181218

12191219
EXPLAIN (COSTS OFF)
1220-
SELECT t1.a, t1.c, t2.b, t2.c FROM (SELECT * FROM prt1 WHERE a = 1 AND a = 2) t1 RIGHT JOIN prt2 t2 ON t1.a = t2.b WHERE t2.a = 0 ORDER BY t1.a, t2.b;
1221-
QUERY PLAN
1222-
--------------------------------------------
1223-
Sort
1224-
Sort Key: a, t2.b
1225-
-> Hash Left Join
1226-
Hash Cond: (t2.b = a)
1227-
-> Append
1228-
-> Seq Scan on prt2_p1 t2
1229-
Filter: (a = 0)
1230-
-> Seq Scan on prt2_p2 t2_1
1231-
Filter: (a = 0)
1232-
-> Seq Scan on prt2_p3 t2_2
1233-
Filter: (a = 0)
1234-
-> Hash
1235-
-> Result
1236-
One-Time Filter: false
1237-
(14 rows)
1220+
SELECT t1.a, t1.c, t2.b, t2.c FROM (SELECT * FROM prt1 WHERE a = 1 AND a = 2) t1 RIGHT JOIN prt2 t2 ON t1.a = t2.b, prt1 t3 WHERE t2.b = t3.a;
1221+
QUERY PLAN
1222+
--------------------------------------------------
1223+
Hash Left Join
1224+
Hash Cond: (t2.b = a)
1225+
-> Append
1226+
-> Hash Join
1227+
Hash Cond: (t3.a = t2.b)
1228+
-> Seq Scan on prt1_p1 t3
1229+
-> Hash
1230+
-> Seq Scan on prt2_p1 t2
1231+
-> Hash Join
1232+
Hash Cond: (t3_1.a = t2_1.b)
1233+
-> Seq Scan on prt1_p2 t3_1
1234+
-> Hash
1235+
-> Seq Scan on prt2_p2 t2_1
1236+
-> Hash Join
1237+
Hash Cond: (t3_2.a = t2_2.b)
1238+
-> Seq Scan on prt1_p3 t3_2
1239+
-> Hash
1240+
-> Seq Scan on prt2_p3 t2_2
1241+
-> Hash
1242+
-> Result
1243+
One-Time Filter: false
1244+
(21 rows)
12381245

12391246
EXPLAIN (COSTS OFF)
12401247
SELECT t1.a, t1.c, t2.b, t2.c FROM (SELECT * FROM prt1 WHERE a = 1 AND a = 2) t1 FULL JOIN prt2 t2 ON t1.a = t2.b WHERE t2.a = 0 ORDER BY t1.a, t2.b;

src/test/regress/sql/partition_join.sql

+1-1
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ EXPLAIN (COSTS OFF)
224224
SELECT t1.a, t1.c, t2.b, t2.c FROM (SELECT * FROM prt1 WHERE a = 1 AND a = 2) t1 LEFT JOIN prt2 t2 ON t1.a = t2.b;
225225

226226
EXPLAIN (COSTS OFF)
227-
SELECT t1.a, t1.c, t2.b, t2.c FROM (SELECT * FROM prt1 WHERE a = 1 AND a = 2) t1 RIGHT JOIN prt2 t2 ON t1.a = t2.b WHERE t2.a = 0 ORDER BY t1.a, t2.b;
227+
SELECT t1.a, t1.c, t2.b, t2.c FROM (SELECT * FROM prt1 WHERE a = 1 AND a = 2) t1 RIGHT JOIN prt2 t2 ON t1.a = t2.b, prt1 t3 WHERE t2.b = t3.a;
228228

229229
EXPLAIN (COSTS OFF)
230230
SELECT t1.a, t1.c, t2.b, t2.c FROM (SELECT * FROM prt1 WHERE a = 1 AND a = 2) t1 FULL JOIN prt2 t2 ON t1.a = t2.b WHERE t2.a = 0 ORDER BY t1.a, t2.b;

0 commit comments

Comments
 (0)