This is an automated email from the ASF dual-hosted git repository. yjhjstz pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/cloudberry.git
commit f35acc7eb6c40b66e17db9df3cab2a1e6a7e7932 Author: Wenlin Zhang <[email protected]> AuthorDate: Thu Apr 11 18:13:28 2024 +0800 Fix issue https://github.com/greenplum-db/gpdb/issues/17333. In the adjust_setop_arguments(), if the setop is partitioned and subpath locus is general, it will call make_motion_hash_all_targets(), and this function will change the locus to singleQE when none of the columns are hashable. But mark_append_locus() will change it back to the original partitioned locus, which will result a wrong Motion node added in the top. This PR moves check for non-hashable column(no targetlist) to choose_setop_type(). --- src/backend/cdb/cdbsetop.c | 24 +++++++++++-- src/backend/optimizer/prep/prepunion.c | 2 +- src/include/cdb/cdbsetop.h | 4 +-- src/test/regress/expected/qp_union_intersect.out | 39 ++++++++++++++++++++++ .../expected/qp_union_intersect_optimizer.out | 38 +++++++++++++++++++++ src/test/regress/sql/qp_union_intersect.sql | 10 ++++++ 6 files changed, 112 insertions(+), 5 deletions(-) diff --git a/src/backend/cdb/cdbsetop.c b/src/backend/cdb/cdbsetop.c index cde90949e2..2d685e8ca6 100644 --- a/src/backend/cdb/cdbsetop.c +++ b/src/backend/cdb/cdbsetop.c @@ -40,13 +40,15 @@ * See the comments in cdbsetop.h for discussion of types of setop plan. */ GpSetOpType -choose_setop_type(List *pathlist) +choose_setop_type(List *pathlist, List *tlist_list) { ListCell *cell; + ListCell *tlistcell; bool ok_general = true; bool ok_partitioned = true; bool ok_single_qe = true; bool has_partitioned = false; + bool all_resjunk = true; Assert(Gp_role == GP_ROLE_DISPATCH || Gp_role == GP_ROLE_UTILITY); @@ -101,9 +103,27 @@ choose_setop_type(List *pathlist) } } + /* + * This is for handling the case when there is no targetList in the ouput. + * For example: select from generate_series(1,10) except select from a; + */ + foreach(tlistcell, tlist_list) + { + List *subtlist = (List *) lfirst(tlistcell); + foreach(cell, subtlist) + { + TargetEntry *tle = (TargetEntry *) lfirst(cell); + if (!tle->resjunk) + { + all_resjunk = false; + break; + } + } + } + if (ok_general) return PSETOP_GENERAL; - else if (ok_partitioned && has_partitioned) + else if (ok_partitioned && has_partitioned && !all_resjunk) return PSETOP_PARALLEL_PARTITIONED; else if (ok_single_qe) return PSETOP_SEQUENTIAL_QE; diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c index 74725edc0b..af3e489d59 100644 --- a/src/backend/optimizer/prep/prepunion.c +++ b/src/backend/optimizer/prep/prepunion.c @@ -832,7 +832,7 @@ generate_nonunion_paths(SetOperationStmt *op, PlannerInfo *root, /* CDB: Decide on approach, condition argument plans to suit. */ if ( Gp_role == GP_ROLE_DISPATCH ) { - optype = choose_setop_type(pathlist); + optype = choose_setop_type(pathlist,tlist_list); adjust_setop_arguments(root, pathlist, tlist_list, optype); } else if ( Gp_role == GP_ROLE_UTILITY diff --git a/src/include/cdb/cdbsetop.h b/src/include/cdb/cdbsetop.h index a8361e543b..de6ba3a5ec 100644 --- a/src/include/cdb/cdbsetop.h +++ b/src/include/cdb/cdbsetop.h @@ -39,7 +39,7 @@ * unordered. The set operation is performed on the QE as if it were * sequential. * - * PSETOP_SEQUENTIAL_QE + * PSETOP_SEQUENTIAL_OUTERQUERY * Similar to SEQUENTIAL_QD/QE, but the output must be made available * to the outer query's locus. We don't know the outer query's locus yet, * but we treat it sequential. @@ -59,7 +59,7 @@ typedef enum GpSetOpType } GpSetOpType; extern -GpSetOpType choose_setop_type(List *pathlist); +GpSetOpType choose_setop_type(List *pathlist, List *tlist_list); extern void adjust_setop_arguments(PlannerInfo *root, List *pathlist, List *tlist_list, GpSetOpType setop_type); diff --git a/src/test/regress/expected/qp_union_intersect.out b/src/test/regress/expected/qp_union_intersect.out index ccc10eeca0..5b90963bb6 100644 --- a/src/test/regress/expected/qp_union_intersect.out +++ b/src/test/regress/expected/qp_union_intersect.out @@ -2012,6 +2012,45 @@ select * from generate_series(100, 105); 105 (12 rows) +-- test INTERSECT/EXCEPT with General and partitioned locus, but none of the columns are hashable +CREATE TABLE p1(a int) distributed by (a); +INSERT INTO p1 select generate_series(1,10); +explain (costs off) +select from generate_series(1,5) intersect select from p1; + QUERY PLAN +------------------------------------------------------ + HashSetOp Intersect + -> Append + -> Subquery Scan on "*SELECT* 1" + -> Function Scan on generate_series + -> Gather Motion 3:1 (slice1; segments: 3) + -> Subquery Scan on "*SELECT* 2" + -> Seq Scan on p1 + Optimizer: Postgres query optimizer +(8 rows) + +select from generate_series(1,5) intersect select from p1; +-- +(1 row) + +explain (costs off) +select from generate_series(1,5) except select from p1; + QUERY PLAN +------------------------------------------------------ + HashSetOp Except + -> Append + -> Subquery Scan on "*SELECT* 1" + -> Function Scan on generate_series + -> Gather Motion 3:1 (slice1; segments: 3) + -> Subquery Scan on "*SELECT* 2" + -> Seq Scan on p1 + Optimizer: Postgres query optimizer +(8 rows) + +select from generate_series(1,5) except select from p1; +-- +(0 rows) + -- -- Test for creation of MergeAppend paths. -- diff --git a/src/test/regress/expected/qp_union_intersect_optimizer.out b/src/test/regress/expected/qp_union_intersect_optimizer.out index c312eedc99..26a8fb71c6 100644 --- a/src/test/regress/expected/qp_union_intersect_optimizer.out +++ b/src/test/regress/expected/qp_union_intersect_optimizer.out @@ -2014,6 +2014,44 @@ select * from generate_series(100, 105); 105 (12 rows) +-- test INTERSECT/EXCEPT with General and partitioned locus, but none of the columns are hashable +CREATE TABLE p1(a int) distributed by (a); +INSERT INTO p1 select generate_series(1,10); +explain (costs off) +select from generate_series(1,5) intersect select from p1; + QUERY PLAN +------------------------------------------------------ + Nested Loop + Join Filter: true + -> Aggregate + -> Gather Motion 3:1 (slice1; segments: 3) + -> Seq Scan on p1 + -> Aggregate + -> Function Scan on generate_series + Optimizer: Pivotal Optimizer (GPORCA) +(8 rows) + +select from generate_series(1,5) intersect select from p1; +-- +(1 row) + +explain (costs off) +select from generate_series(1,5) except select from p1; + QUERY PLAN +------------------------------------------------------ + Nested Loop Anti Join + Join Filter: true + -> Function Scan on generate_series + -> Materialize + -> Gather Motion 3:1 (slice1; segments: 3) + -> Seq Scan on p1 + Optimizer: Pivotal Optimizer (GPORCA) +(7 rows) + +select from generate_series(1,5) except select from p1; +-- +(0 rows) + -- -- Test for creation of MergeAppend paths. -- diff --git a/src/test/regress/sql/qp_union_intersect.sql b/src/test/regress/sql/qp_union_intersect.sql index a51759d1ba..1872734b6b 100644 --- a/src/test/regress/sql/qp_union_intersect.sql +++ b/src/test/regress/sql/qp_union_intersect.sql @@ -767,6 +767,16 @@ select a from t_test_append_rep union all select * from generate_series(100, 105); +-- test INTERSECT/EXCEPT with General and partitioned locus, but none of the columns are hashable +CREATE TABLE p1(a int) distributed by (a); +INSERT INTO p1 select generate_series(1,10); +explain (costs off) +select from generate_series(1,5) intersect select from p1; +select from generate_series(1,5) intersect select from p1; +explain (costs off) +select from generate_series(1,5) except select from p1; +select from generate_series(1,5) except select from p1; + -- -- Test for creation of MergeAppend paths. -- --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
