Add an option to get_eclass_for_sort_expr() to ignore RelabelType

[yaowangm]

Overview

The PR is to fix issue https://github.com/greenplum-db/gpdb/issues/13402 (similar issue: https://github.com/greenplum-db/gpdb/issues/14475)

Basically, for joining a partition table to another normal table, an extra unnecessary Redistribute Motion node is generated by planner:

create table tb_part (id varchar(32), t varchar(32))
distributed by (id)
partition by range(t)
(
  partition p1 start ('0') end ('100'),
  partition p2 start ('101') end ('201')
);

create table tb_spl (id varchar(32), t varchar(32))
distributed by (id);

set optimizer=off;
explain (costs off) select * from tb_spl a join tb_part b on a.id = b.id;

The result:

                         QUERY PLAN
------------------------------------------------------------
 Gather Motion 3:1  (slice1; segments: 3)
   ->  Hash Join
         Hash Cond: ((b.id)::text = (a.id)::text)
         ->  Redistribute Motion 3:3  (slice2; segments: 3)
               Hash Key: b.id
               ->  Append
                     ->  Seq Scan on tb_part_1_prt_p1 b
                     ->  Seq Scan on tb_part_1_prt_p2 b_1
         ->  Hash
               ->  Seq Scan on tb_spl a
 Optimizer: Postgres-based planner
(11 rows)

However, the Redistribute Motion is unnecessary since a.id and b.id have same distribution actually. The correct result is:

                     QUERY PLAN
----------------------------------------------------
 Gather Motion 3:1  (slice1; segments: 3)
   ->  Hash Join
         Hash Cond: ((b.id)::text = (a.id)::text)
         ->  Append
               ->  Seq Scan on tb_part_1_prt_p1 b
               ->  Seq Scan on tb_part_1_prt_p2 b_1
         ->  Hash
               ->  Seq Scan on tb_spl a
 Optimizer: Postgres-based planner
(9 rows)

Root cause

In cdb_pull_up_eclass() to upgrade the equivalence class in the append path, cdbpullup_findEclassInTargetList() is called to find eclass from targetlist with ignoring RelabelType:

276         while (IsA(key, RelabelType))
277             key = (Expr *) ((RelabelType *) key)->arg;

However, in get_eclass_for_sort_expr() called later, RelabelType is not considered. Then an unnecessary equivalence member is created and resulted in a wrong judgment in the subsequent cdbpath_match_preds_to_both_distkeys(), and then the wrong motion node.

Existing Solutions

There are a couple of solutions already.

Solution 1 (https://github.com/greenplum-db/gpdb/pull/13423): It strips out real expr whenever the expr is RelabelType in get_eclass_for_sort_expr(). It actually changed the behavior of the function, and we are not sure any side effect because get_eclass_for_sort_expr() is called in a number of code paths.

Solution 2 (https://github.com/greenplum-db/gpdb/pull/15831): in cdb_pull_up_eclass(), it firstly checks whether the expr from cdbpullup_findEclassInTargetList() is stripped from RelabelType. If yes, return current eclass directly without calling get_eclass_for_sort_expr(). In addition, it limits the behavior to append code path by an extra parameter. It also changed the behavior of cdb_pull_up_eclass(): since there are some other checks inside get_eclass_for_sort_expr(), we are not sure any side effect.

Solution 3(https://github.com/greenplum-db/gpdb/pull/15831#issuecomment-1777104791) from @yanwr1: it directly returns the expr of ec member instead of targetlist item, and get_eclass_for_sort_expr() can match successfully. However, it also changed the behavior: for a sub query scenario, the ec member might not exist in root->eq_classes, so get_eclass_for_sort_expr() will fail again.

My Solution

The trouble here is, the interface of cdbpullup_findEclassInTargetList() has been eroded. According to its definition, it should always return an expr of chosen ec member or null:

 * Searches the given equivalence class for a member that uses no rels
 * outside the 'relids' set, and either is a member of 'targetlist', or
 * uses no Vars that are not in 'targetlist'. Furthermore, if
 * 'hashOpFamily' is valid, the member must be hashable using that hash
 * operator family.
 *
 * If found, returns the chosen member's expression, otherwise returns
 * NULL

In GPDB6 it does. But in GPDB7, sometimes it returns expr of targetlist item, and some callers depend on the not-defined behavior in complex scenarios. The result is, whatever we do to fix the issue, we have to face a couple of unpredictable consequences because of the orthogonal conditions in various scenarios. If we are not able to confirm all the possibilies, a solution is unacceptable.

The most complete solution is to update cdbpullup_findEclassInTargetList() to always return expr of ec member (as solution 3), and refactor relevant code path of sub query. But it seems infeasible because too much effort is needed.

My solution is: back to the original problem, the error is due to that we consider RelabelType in cdbpullup_findEclassInTargetList(), but not in get_eclass_for_sort_expr(). So we can simply add an extra option to indicate get_eclass_for_sort_expr() to consider RelabelType (as solution 1 with an extra parameter). In addition, change get_eclass_for_sort_expr() as a wrapper function of get_eclass_for_sort_expr_real(), so all other callers can follow existing code path without more change.

The solution is not so ideal, but at least it avoids all possible side effects and the code change is not much.

CI pipeline: https://dev.ci.gpdb.pivotal.io/teams/main/pipelines/wayao_ec_relabel_type