From f069c91a5793ff6b7884120de748b2005ee7756f Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Mon, 5 Feb 2018 17:31:57 -0500 Subject: [PATCH] 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 --- src/backend/optimizer/path/allpaths.c | 18 ++------ src/backend/optimizer/path/joinrels.c | 16 -------- src/backend/optimizer/util/relnode.c | 5 ++- src/include/nodes/relation.h | 14 ++++--- src/test/regress/expected/partition_join.out | 43 ++++++++++++-------- src/test/regress/sql/partition_join.sql | 2 +- 6 files changed, 43 insertions(+), 55 deletions(-) diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index 5bff90e1bca..6e842f93d0f 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -3425,20 +3425,8 @@ generate_partition_wise_join_paths(PlannerInfo *root, RelOptInfo *rel) if (!IS_JOIN_REL(rel)) return; - /* - * If we've already proven this join is empty, we needn't consider any - * more paths for it. - */ - if (IS_DUMMY_REL(rel)) - return; - - /* - * We've nothing to do if the relation is not partitioned. An outer join - * relation which had an empty inner relation in every pair will have the - * rest of the partitioning properties set except the child-join - * RelOptInfos. See try_partition_wise_join() for more details. - */ - if (rel->nparts <= 0 || rel->part_rels == NULL) + /* We've nothing to do if the relation is not partitioned. */ + if (!IS_PARTITIONED_REL(rel)) return; /* Guard against stack overflow due to overly deep partition hierarchy. */ @@ -3452,6 +3440,8 @@ generate_partition_wise_join_paths(PlannerInfo *root, RelOptInfo *rel) { RelOptInfo *child_rel = part_rels[cnt_parts]; + Assert(child_rel != NULL); + /* Add partition-wise join paths for partitioned child-joins. */ generate_partition_wise_join_paths(root, child_rel); diff --git a/src/backend/optimizer/path/joinrels.c b/src/backend/optimizer/path/joinrels.c index a35d0689119..f74afdb4dda 100644 --- a/src/backend/optimizer/path/joinrels.c +++ b/src/backend/optimizer/path/joinrels.c @@ -1318,17 +1318,6 @@ try_partition_wise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2, if (!IS_PARTITIONED_REL(joinrel)) return; - /* - * set_rel_pathlist() may not create paths in children of an empty - * partitioned table and so we can not add paths to child-joins. So, deem - * such a join as unpartitioned. When a partitioned relation is deemed - * empty because all its children are empty, dummy path will be set in - * each of the children. In such a case we could still consider the join - * as partitioned, but it might not help much. - */ - if (IS_DUMMY_REL(rel1) || IS_DUMMY_REL(rel2)) - return; - /* * Since this join relation is partitioned, all the base relations * 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, nparts = joinrel->nparts; - /* Allocate space to hold child-joins RelOptInfos, if not already done. */ - if (!joinrel->part_rels) - joinrel->part_rels = - (RelOptInfo **) palloc0(sizeof(RelOptInfo *) * nparts); - /* * Create child-join relations for this partitioned join, if those don't * exist. Add paths to child-joins for a pair of child relations diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c index ac5a7c9553d..5c368321e6e 100644 --- a/src/backend/optimizer/util/relnode.c +++ b/src/backend/optimizer/util/relnode.c @@ -1662,11 +1662,14 @@ build_joinrel_partition_info(RelOptInfo *joinrel, RelOptInfo *outer_rel, */ joinrel->part_scheme = part_scheme; joinrel->boundinfo = outer_rel->boundinfo; - joinrel->nparts = outer_rel->nparts; partnatts = joinrel->part_scheme->partnatts; joinrel->partexprs = (List **) palloc0(sizeof(List *) * partnatts); joinrel->nullable_partexprs = (List **) palloc0(sizeof(List *) * partnatts); + joinrel->nparts = outer_rel->nparts; + joinrel->part_rels = + (RelOptInfo **) palloc0(sizeof(RelOptInfo *) * joinrel->nparts); + /* * Construct partition keys for the join. diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h index 6bf68f31da1..b1c63173c22 100644 --- a/src/include/nodes/relation.h +++ b/src/include/nodes/relation.h @@ -666,13 +666,17 @@ typedef struct RelOptInfo /* * Is given relation partitioned? * - * A join between two partitioned relations with same partitioning scheme - * without any matching partitions will not have any partition in it but will - * have partition scheme set. So a relation is deemed to be partitioned if it - * has a partitioning scheme, bounds and positive number of partitions. + * It's not enough to test whether rel->part_scheme is set, because it might + * be that the basic partitioning properties of the input relations matched + * but the partition bounds did not. + * + * We treat dummy relations as unpartitioned. We could alternatively + * treat them as partitioned, but it's not clear whether that's a useful thing + * to do. */ #define IS_PARTITIONED_REL(rel) \ - ((rel)->part_scheme && (rel)->boundinfo && (rel)->nparts > 0) + ((rel)->part_scheme && (rel)->boundinfo && (rel)->nparts > 0 && \ + (rel)->part_rels && !(IS_DUMMY_REL(rel))) /* * Convenience macro to make sure that a partitioned relation has all the diff --git a/src/test/regress/expected/partition_join.out b/src/test/regress/expected/partition_join.out index 27ab8521f84..333f93889cf 100644 --- a/src/test/regress/expected/partition_join.out +++ b/src/test/regress/expected/partition_join.out @@ -1217,24 +1217,31 @@ SELECT t1.a, t1.c, t2.b, t2.c FROM (SELECT * FROM prt1 WHERE a = 1 AND a = 2) t1 (2 rows) EXPLAIN (COSTS OFF) -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; - QUERY PLAN --------------------------------------------- - Sort - Sort Key: a, t2.b - -> Hash Left Join - Hash Cond: (t2.b = a) - -> Append - -> Seq Scan on prt2_p1 t2 - Filter: (a = 0) - -> Seq Scan on prt2_p2 t2_1 - Filter: (a = 0) - -> Seq Scan on prt2_p3 t2_2 - Filter: (a = 0) - -> Hash - -> Result - One-Time Filter: false -(14 rows) +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; + QUERY PLAN +-------------------------------------------------- + Hash Left Join + Hash Cond: (t2.b = a) + -> Append + -> Hash Join + Hash Cond: (t3.a = t2.b) + -> Seq Scan on prt1_p1 t3 + -> Hash + -> Seq Scan on prt2_p1 t2 + -> Hash Join + Hash Cond: (t3_1.a = t2_1.b) + -> Seq Scan on prt1_p2 t3_1 + -> Hash + -> Seq Scan on prt2_p2 t2_1 + -> Hash Join + Hash Cond: (t3_2.a = t2_2.b) + -> Seq Scan on prt1_p3 t3_2 + -> Hash + -> Seq Scan on prt2_p3 t2_2 + -> Hash + -> Result + One-Time Filter: false +(21 rows) EXPLAIN (COSTS OFF) 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; diff --git a/src/test/regress/sql/partition_join.sql b/src/test/regress/sql/partition_join.sql index 6efdf3c5175..55c5615d06c 100644 --- a/src/test/regress/sql/partition_join.sql +++ b/src/test/regress/sql/partition_join.sql @@ -224,7 +224,7 @@ EXPLAIN (COSTS OFF) 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; EXPLAIN (COSTS OFF) -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; +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; EXPLAIN (COSTS OFF) 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;