leborchuk commented on code in PR #1756:
URL: https://github.com/apache/cloudberry/pull/1756#discussion_r3273868752
##########
src/backend/gporca/libgpopt/src/translate/CTranslatorExprToDXL.cpp:
##########
@@ -4964,41 +4964,46 @@ CTranslatorExprToDXL::PdxlnMergeJoin(CExpression
*pexprMJ,
// At this point, they all better be merge joinable
GPOS_ASSERT(CPhysicalJoin::FMergeJoinCompatible(
pexprPred, pexprOuterChild, pexprInnerChild));
- CExpression *pexprPredOuter = (*pexprPred)[0];
- CExpression *pexprPredInner = (*pexprPred)[1];
- // align extracted columns with outer and inner children of the
join
- CColRefSet *pcrsOuterChild =
pexprOuterChild->DeriveOutputColumns();
- CColRefSet *pcrsPredInner = pexprPredInner->DeriveUsedColumns();
+ // Extract the two key columns out of pexprPred. Plain equality
+ // is `a = b` (binary ScalarCmp) but NULL-safe equality (INDF)
is
+ // `NOT (a IS DISTINCT FROM b)`, a *unary* NOT around the binary
+ // IsDistinctFrom. Indexing [0]/[1] directly works for equality
+ // but accesses out of bounds for INDF.
+ CExpression *pexprPredOuter = nullptr;
+ CExpression *pexprPredInner = nullptr;
+ IMDId *mdid_scop = nullptr;
+ CPhysicalJoin::AlignJoinKeyOuterInner(pexprPred,
pexprOuterChild,
+
pexprInnerChild, &pexprPredOuter,
+
&pexprPredInner, &mdid_scop);
+
#ifdef GPOS_DEBUG
+ CColRefSet *pcrsOuterChild =
pexprOuterChild->DeriveOutputColumns();
CColRefSet *pcrsInnerChild =
pexprInnerChild->DeriveOutputColumns();
CColRefSet *pcrsPredOuter = pexprPredOuter->DeriveUsedColumns();
+ CColRefSet *pcrsPredInner = pexprPredInner->DeriveUsedColumns();
+ GPOS_ASSERT(pcrsOuterChild->ContainsAll(pcrsPredOuter) &&
+
pcrsInnerChild->ContainsAll(pcrsPredInner) &&
+ "merge join keys are not aligned with
children");
#endif
- if (pcrsOuterChild->ContainsAll(pcrsPredInner))
+ pexprPredOuter->AddRef();
+ pexprPredInner->AddRef();
+ CExpression *pexprPredNew;
+ if (CPredicateUtils::IsEqualityOp(pexprPred))
{
- GPOS_ASSERT(pcrsInnerChild->ContainsAll(pcrsPredOuter));
- std::swap(pexprPredOuter, pexprPredInner);
-#ifdef GPOS_DEBUG
- std::swap(pcrsPredOuter, pcrsPredInner);
-#endif
-
- pexprPredOuter->AddRef();
- pexprPredInner->AddRef();
- pexprPred =
- CUtils::PexprScalarEqCmp(m_mp, pexprPredOuter,
pexprPredInner);
+ pexprPredNew = CUtils::PexprScalarCmp(m_mp,
pexprPredOuter,
+
pexprPredInner, mdid_scop);
}
else
{
- pexprPred->AddRef();
+ GPOS_ASSERT(CPredicateUtils::FINDF(pexprPred));
Review Comment:
Is it possible to have INDF for merge joins? Because in
`CPhysicalJoin::FMergeJoinCompatible ` we check:
```
// Only merge join between ScalarIdents of the same types is currently
supported
if (CPredicateUtils::FEqIdentsOfSameType(pexprPred))
{
pexprPredOuter = (*pexprPred)[0];
pexprPredInner = (*pexprPred)[1];
mdid_scop = CScalarCmp::PopConvert(pexprPred->Pop())->MdIdOp();
GPOS_ASSERT(CUtils::FScalarIdent(pexprPredOuter));
GPOS_ASSERT(CUtils::FScalarIdent(pexprPredInner));
}
else
{
return false;
}
```
So the old code should work since we always check only for valid merge join
operands.
Maybe we want to change it in future PR's?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
