WIP: ORCA: Fix eliminate self comparison

[fanfuxiaoran]

For the below query

create table t1(a int, b int not null);
create table t2(like t1);
select t1.*, t2.* from t1 full join t2 on false where (t1.b < t1.b) is null;

orca generates a wrong plan:

Result  (cost=0.00..0.00 rows=0 width=16)
    One-Time Filter: false
Optimizer: Pivotal Optimizer (GPORCA)

The root cause is '(t1.b < t1.b)' is been transformed into 'CScalarConst (0)' by 'PexprEliminateSelfComparison'. The reason is that when checking if the selfcomparison can be simplified by function FSelfComparison, it checks the CColRef IsNullable only from the column definition, not checking if the column is from outer join.

To fix it, before simplifing the scalar expression, we fisrt get the 'pcrsNotNull' from its parent expression. 'pcrsNotNull' recoreds the output columns' nullable property. If the column is not in 'pcrsNotNull', then the self comparison cannot be transformed into const true or false.

Fixes #594

What does this PR do?

Type of Change

• Bug fix (non-breaking change)
• New feature (non-breaking change)
• Breaking change (fix or feature with breaking changes)
• Documentation update

Breaking Changes

Test Plan

• Unit tests added/updated
• Integration tests added/updated
• Passed make installcheck
• Passed make -C src/test installcheck-cbdb-parallel

Impact

Performance:

User-facing changes:

Dependencies:

Checklist

• Followed contribution guide
• Added/updated documentation
• Reviewed code for security implications
• Requested review from cloudberry committers

Additional Context

⚠️ To skip CI: Add [skip ci] to your PR title. Only use when necessary! ⚠️

---

[fanfuxiaoran]

Working on adding tests for it!

[fanfuxiaoran]

Found that gpdb has a similar commit : greenplum-db/gpdb-archive@d3dd98c
Just cherry-pick it .
But it has some problems, added a commit to fix it.

[jiaqizho]

CExpression *pexprSelfCompEliminated = PexprEliminateSelfComparison(
		mp, pexprInferredPreds, pexprInferredPreds->DeriveNotNullColumns());

caller already call the DeriveNotNullColumns. why we need Derive it twice?

[jiaqizho]

The Derive* is not related to the CExpression type(logic/scalar/phy, also no phy CExpression will occar in the CExpressionPreprocessor) .

So i guess the CDrvdProp always return true when it check the m_is_prop_derived.

[fanfuxiaoran]

CExpression *pexprSelfCompEliminated = PexprEliminateSelfComparison(
		mp, pexprInferredPreds, pexprInferredPreds->DeriveNotNullColumns());

caller already call the DeriveNotNullColumns. why we need Derive it twice?

'PexprEliminateSelfComparison' only uses the 'pcrsNotNull'
from the topmost expression to filter the nullable columns.
This can lead the PexprEliminateSelfComparison cannot apply
to the subquery properly.

create table t1(a int not null, b int not null);
create table t2(like t1);
create table t3(like t1);
select * from t2  left join (select  t2.a , t2.b  from t1, t2 where t1.a
	< t1.a) as t on t2. a = t.a;

the plan for it from orca is

 Gather Motion 3:1  (slice1; segments: 3)
   ->  Hash Left Join
         Hash Cond: (t2.a = t2_1.a)
         ->  Seq Scan on t2
         ->  Hash
               ->  Nested Loop
                     Join Filter: true
                     ->  Seq Scan on t2 t2_1
                     ->  Materialize
                           ->  Broadcast Motion 3:3  (slice2; segments: 3)
                                 ->  Seq Scan on t1
                                       Filter: (a < a)

the self comparison in subquery is not eliminated.

This commit is to optimize it by fetching 'pcrsNotNull' from
the current logical expression and apply them to its child
scalar expression.
The second commit is mainly to solve the above problem

[jiaqizho]

i see. so can we just } else if (pop->FLogical()) here?

FScalarCmp already check the CExpression is the scalar operator.

[fanfuxiaoran]

Sure.
But in block if (CUtils::FScalarCmp(pexpr)) , the function will return.
Does } else if (pop->FLogical()) help to understand the code?

[jiaqizho]

yes, This is how I understand this part of logic.

   // The scalar operator with cmp type(COperator::EopScalarCmp), just replace it with True or False expression if possible
   if (CUtils::FScalarCmp(pexpr))
   {
      ... 
   // The logic operator should use the current operator properly rather then the root.
   } else if (pop->FLogical()) {
      ...
   } // should no occur phy operator here

[fanfuxiaoran]

Agree. And I will add the comments for it.