Skip to content

Commit

Permalink
ORCA: Fix eliminate self comparison
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
fanfuxiaoran committed Nov 21, 2024
1 parent f92faf0 commit 8902941
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ class CExpressionPreprocessor

// eliminate self comparisons
static CExpression *PexprEliminateSelfComparison(CMemoryPool *mp,
CExpression *pexpr);
CExpression *pexpr,
CColRefSet *pcrsNotNull);

// remove CTE Anchor nodes
static CExpression *PexprRemoveCTEAnchors(CMemoryPool *mp,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,11 +130,14 @@ class CPredicateUtils
static BOOL FPlainEquality(CExpression *pexpr);

// is the given expression a self comparison on some column
static BOOL FSelfComparison(CExpression *pexpr, IMDType::ECmpType *pecmpt);
static BOOL FSelfComparison(CExpression *pexpr,
IMDType::ECmpType *pecmpt,
CColRefSet *pcrsNotNull);

// eliminate self comparison if possible
static CExpression *PexprEliminateSelfComparison(CMemoryPool *mp,
CExpression *pexpr);
CExpression *pexpr,
CColRefSet *pcrsNotNull);

// is the given expression in the form (col1 Is NOT DISTINCT FROM col2)
static BOOL FINDFScalarIdents(CExpression *pexpr);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,29 +70,31 @@ using namespace gpopt;
// eliminate self comparisons in the given expression
CExpression *
CExpressionPreprocessor::PexprEliminateSelfComparison(CMemoryPool *mp,
CExpression *pexpr)
CExpression *pexpr,
CColRefSet *pcrsNotNull)
{
// protect against stack overflow during recursion
GPOS_CHECK_STACK_SIZE;
GPOS_ASSERT(nullptr != mp);
GPOS_ASSERT(nullptr != pexpr);

COperator *pop = pexpr->Pop();
if (CUtils::FScalarCmp(pexpr))
{
return CPredicateUtils::PexprEliminateSelfComparison(mp, pexpr);
return CPredicateUtils::PexprEliminateSelfComparison(mp, pexpr, pcrsNotNull);
}
if (pop->FLogical())
{
pcrsNotNull = pexpr->DeriveNotNullColumns();
}

// recursively process children
const ULONG arity = pexpr->Arity();
CExpressionArray *pdrgpexprChildren = GPOS_NEW(mp) CExpressionArray(mp);
for (ULONG ul = 0; ul < arity; ul++)
{
CExpression *pexprChild =
PexprEliminateSelfComparison(mp, (*pexpr)[ul]);
pdrgpexprChildren->Append(pexprChild);
pdrgpexprChildren->Append(
PexprEliminateSelfComparison(mp, (*pexpr)[ul], pcrsNotNull));
}

COperator *pop = pexpr->Pop();
pop->AddRef();

return GPOS_NEW(mp) CExpression(mp, pop, pdrgpexprChildren);
Expand Down Expand Up @@ -3078,7 +3080,7 @@ CExpressionPreprocessor::PexprPreprocess(

// (11) eliminate self comparisons
CExpression *pexprSelfCompEliminated =
PexprEliminateSelfComparison(mp, pexprInferredPreds);
PexprEliminateSelfComparison(mp, pexprInferredPreds, nullptr);
GPOS_CHECK_ABORT;
pexprInferredPreds->Release();

Expand Down
11 changes: 8 additions & 3 deletions src/backend/gporca/libgpopt/src/operators/CPredicateUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -856,7 +856,7 @@ CPredicateUtils::FPlainEquality(CExpression *pexpr)

// is an expression a self comparison on some column
BOOL
CPredicateUtils::FSelfComparison(CExpression *pexpr, IMDType::ECmpType *pecmpt)
CPredicateUtils::FSelfComparison(CExpression *pexpr, IMDType::ECmpType *pecmpt, CColRefSet *pcrsNotNull)
{
GPOS_ASSERT(nullptr != pexpr);
GPOS_ASSERT(nullptr != pecmpt);
Expand All @@ -881,6 +881,10 @@ CPredicateUtils::FSelfComparison(CExpression *pexpr, IMDType::ECmpType *pecmpt)

CColRef *colref =
const_cast<CColRef *>(CScalarIdent::PopConvert(popLeft)->Pcr());
if (nullptr != pcrsNotNull && !pcrsNotNull->FMember(colref))
{
return false;
}

return CColRef::EcrtTable == colref->Ecrt() &&
!CColRefTable::PcrConvert(colref)->IsNullable();
Expand All @@ -892,14 +896,15 @@ CPredicateUtils::FSelfComparison(CExpression *pexpr, IMDType::ECmpType *pecmpt)
// eliminate self comparison and replace it with True or False if possible
CExpression *
CPredicateUtils::PexprEliminateSelfComparison(CMemoryPool *mp,
CExpression *pexpr)
CExpression *pexpr,
CColRefSet *pcrsNotNull)
{
GPOS_ASSERT(pexpr->Pop()->FScalar());

pexpr->AddRef();
CExpression *pexprNew = pexpr;
IMDType::ECmpType cmp_type = IMDType::EcmptOther;
if (FSelfComparison(pexpr, &cmp_type))
if (FSelfComparison(pexpr, &cmp_type, pcrsNotNull))
{
switch (cmp_type)
{
Expand Down

0 comments on commit 8902941

Please sign in to comment.