Re: [PR] WIP: ORCA: Fix eliminate self comparison [cloudberry]
fanfuxiaoran commented on code in PR #722:
URL: https://github.com/apache/cloudberry/pull/722#discussion_r1868887771
##
src/backend/gporca/libgpopt/src/operators/CExpressionPreprocessor.cpp:
##
@@ -70,16 +70,23 @@ 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())
Review Comment:
Fixed! And add regress test case for it
--
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: commits-unsubscr...@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cloudberry.apache.org
For additional commands, e-mail: commits-h...@cloudberry.apache.org
Re: [PR] WIP: ORCA: Fix eliminate self comparison [cloudberry]
fanfuxiaoran commented on PR #722: URL: https://github.com/apache/cloudberry/pull/722#issuecomment-2516459006 I'm supposed to add uttest for the new commit https://github.com/apache/cloudberry/pull/722/commits/ffb8b2e1e8141c43d38a0eac37b879af7b7c415b, but the uttests in the orca seem broken (before this pr) , some are failed, I need to to fix that first. As that will take some time, so plan to create a issue for it and do it in a separate pr. -- 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: commits-unsubscr...@cloudberry.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cloudberry.apache.org For additional commands, e-mail: commits-h...@cloudberry.apache.org
Re: [PR] WIP: ORCA: Fix eliminate self comparison [cloudberry]
fanfuxiaoran commented on code in PR #722:
URL: https://github.com/apache/cloudberry/pull/722#discussion_r1862842852
##
src/backend/gporca/libgpopt/src/operators/CExpressionPreprocessor.cpp:
##
@@ -70,16 +70,23 @@ 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())
Review Comment:
Agree. And I will add the comments for it.
--
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: commits-unsubscr...@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cloudberry.apache.org
For additional commands, e-mail: commits-h...@cloudberry.apache.org
Re: [PR] WIP: ORCA: Fix eliminate self comparison [cloudberry]
jiaqizho commented on code in PR #722:
URL: https://github.com/apache/cloudberry/pull/722#discussion_r1861958563
##
src/backend/gporca/libgpopt/src/operators/CExpressionPreprocessor.cpp:
##
@@ -70,16 +70,23 @@ 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())
Review Comment:
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
```
--
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: commits-unsubscr...@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cloudberry.apache.org
For additional commands, e-mail: commits-h...@cloudberry.apache.org
Re: [PR] WIP: ORCA: Fix eliminate self comparison [cloudberry]
jiaqizho commented on code in PR #722:
URL: https://github.com/apache/cloudberry/pull/722#discussion_r1861958563
##
src/backend/gporca/libgpopt/src/operators/CExpressionPreprocessor.cpp:
##
@@ -70,16 +70,23 @@ 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())
Review Comment:
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 phy operator occur here
```
--
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: commits-unsubscr...@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cloudberry.apache.org
For additional commands, e-mail: commits-h...@cloudberry.apache.org
Re: [PR] WIP: ORCA: Fix eliminate self comparison [cloudberry]
fanfuxiaoran commented on code in PR #722:
URL: https://github.com/apache/cloudberry/pull/722#discussion_r1861936638
##
src/backend/gporca/libgpopt/src/operators/CExpressionPreprocessor.cpp:
##
@@ -70,16 +70,23 @@ 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())
Review Comment:
Sure.
But in block `if (CUtils::FScalarCmp(pexpr))` , the function will return.
Does `} else if (pop->FLogical())` help to understand the code?
--
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: commits-unsubscr...@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cloudberry.apache.org
For additional commands, e-mail: commits-h...@cloudberry.apache.org
Re: [PR] WIP: ORCA: Fix eliminate self comparison [cloudberry]
jiaqizho commented on code in PR #722:
URL: https://github.com/apache/cloudberry/pull/722#discussion_r1861844726
##
src/backend/gporca/libgpopt/src/operators/CExpressionPreprocessor.cpp:
##
@@ -70,16 +70,23 @@ 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())
Review Comment:
i see. so can we just `} else if (pop->FLogical())` here?
`FScalarCmp` already check the CExpression is the `scalar` operator.
--
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: commits-unsubscr...@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cloudberry.apache.org
For additional commands, e-mail: commits-h...@cloudberry.apache.org
Re: [PR] WIP: ORCA: Fix eliminate self comparison [cloudberry]
jiaqizho commented on code in PR #722:
URL: https://github.com/apache/cloudberry/pull/722#discussion_r1861823581
##
src/backend/gporca/libgpopt/src/operators/CExpressionPreprocessor.cpp:
##
@@ -70,16 +70,23 @@ 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())
Review Comment:
i see..
how about change the L98 ？
```
CExpression *pexprChild =
PexprEliminateSelfComparison(mp, (*pexpr)[ul],
(*pexpr)[ul]->DeriveNotNullColumns());
```
--
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: commits-unsubscr...@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cloudberry.apache.org
For additional commands, e-mail: commits-h...@cloudberry.apache.org
Re: [PR] WIP: ORCA: Fix eliminate self comparison [cloudberry]
jiaqizho commented on code in PR #722:
URL: https://github.com/apache/cloudberry/pull/722#discussion_r1861823581
##
src/backend/gporca/libgpopt/src/operators/CExpressionPreprocessor.cpp:
##
@@ -70,16 +70,23 @@ 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())
Review Comment:
i see..
how about change the L98 ？
```
CExpression *pexprChild =
PexprEliminateSelfComparison(mp, (*pexpr)[ul],
(*pexpr)[ul]->DeriveNotNullColumns());
```
--
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: commits-unsubscr...@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cloudberry.apache.org
For additional commands, e-mail: commits-h...@cloudberry.apache.org
Re: [PR] WIP: ORCA: Fix eliminate self comparison [cloudberry]
fanfuxiaoran commented on code in PR #722:
URL: https://github.com/apache/cloudberry/pull/722#discussion_r1861786883
##
src/backend/gporca/libgpopt/src/operators/CExpressionPreprocessor.cpp:
##
@@ -70,16 +70,23 @@ 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())
Review Comment:
> ```
> 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 proble
##
src/backend/gporca/libgpopt/src/operators/CExpressionPreprocessor.cpp:
##
@@ -70,16 +70,23 @@ 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())
Review Comment:
> ```
> 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
```
Re: [PR] WIP: ORCA: Fix eliminate self comparison [cloudberry]
jiaqizho commented on code in PR #722:
URL: https://github.com/apache/cloudberry/pull/722#discussion_r1861768388
##
src/backend/gporca/libgpopt/src/operators/CExpressionPreprocessor.cpp:
##
@@ -70,16 +70,23 @@ 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())
Review Comment:
```
CExpression *pexprSelfCompEliminated = PexprEliminateSelfComparison(
mp, pexprInferredPreds,
pexprInferredPreds->DeriveNotNullColumns());
```
caller already call the `DeriveNotNullColumns`. why we need Derive it twice?
--
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: commits-unsubscr...@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cloudberry.apache.org
For additional commands, e-mail: commits-h...@cloudberry.apache.org
Re: [PR] WIP: ORCA: Fix eliminate self comparison [cloudberry]
jiaqizho commented on code in PR #722:
URL: https://github.com/apache/cloudberry/pull/722#discussion_r1861773478
##
src/backend/gporca/libgpopt/src/operators/CExpressionPreprocessor.cpp:
##
@@ -70,16 +70,23 @@ 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())
Review Comment:
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`.
--
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: commits-unsubscr...@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cloudberry.apache.org
For additional commands, e-mail: commits-h...@cloudberry.apache.org
Re: [PR] WIP: ORCA: Fix eliminate self comparison [cloudberry]
jiaqizho commented on code in PR #722:
URL: https://github.com/apache/cloudberry/pull/722#discussion_r1861773478
##
src/backend/gporca/libgpopt/src/operators/CExpressionPreprocessor.cpp:
##
@@ -70,16 +70,23 @@ 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())
Review Comment:
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`.
--
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: commits-unsubscr...@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cloudberry.apache.org
For additional commands, e-mail: commits-h...@cloudberry.apache.org
Re: [PR] WIP: ORCA: Fix eliminate self comparison [cloudberry]
jiaqizho commented on code in PR #722:
URL: https://github.com/apache/cloudberry/pull/722#discussion_r1861773478
##
src/backend/gporca/libgpopt/src/operators/CExpressionPreprocessor.cpp:
##
@@ -70,16 +70,23 @@ 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())
Review Comment:
The `Derive*` is not related to the CExpression type(logic/scalar/phy, also
no phy CExpression will occar in the CExpressionPreprocessor) itself.
So i guess the `CDrvdProp` always return true. when it check the
`m_is_prop_derived`.
--
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: commits-unsubscr...@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cloudberry.apache.org
For additional commands, e-mail: commits-h...@cloudberry.apache.org
Re: [PR] WIP: ORCA: Fix eliminate self comparison [cloudberry]
fanfuxiaoran commented on PR #722: URL: https://github.com/apache/cloudberry/pull/722#issuecomment-2490353062 Found that gpdb has a similar commit : https://github.com/greenplum-db/gpdb-archive/commit/d3dd98c1a8daf04fbf6cb91fc4afa6f91b317e93 Just cherry-pick it . But it has some problems, added a commit to fix it. -- 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: commits-unsubscr...@cloudberry.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cloudberry.apache.org For additional commands, e-mail: commits-h...@cloudberry.apache.org
Re: [PR] WIP: ORCA: Fix eliminate self comparison [cloudberry]
fanfuxiaoran commented on PR #722: URL: https://github.com/apache/cloudberry/pull/722#issuecomment-2489993028 Working on adding tests for it! -- 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: commits-unsubscr...@cloudberry.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cloudberry.apache.org For additional commands, e-mail: commits-h...@cloudberry.apache.org
