rui-mo commented on code in PR #7363:
URL: https://github.com/apache/incubator-gluten/pull/7363#discussion_r1792922261
##########
cpp/velox/substrait/SubstraitToVeloxPlan.cc:
##########
@@ -1686,14 +1687,15 @@ bool SubstraitToVeloxPlanConverter::canPushdownOr(
}
} else if (arg.value().has_singular_or_list()) {
const auto& singularOrList = arg.value().singular_or_list();
+ // Disable IN pushdown for int-like types.
if (!canPushdownSingularOrList(singularOrList, true)) {
return false;
}
uint32_t fieldIdx = getColumnIndexFromSingularOrList(singularOrList);
- // Disable IN pushdown for int-like types.
- if (!rangeRecorders.at(fieldIdx).setInRange(true /*forOrRelation*/)) {
+ if (!rangeRecorders.at(fieldIdx).setInRange(sign /*forOrRelation*/)) {
Review Comment:
We don't need this change as this logic is checking the nested expressions
in an `or` condition, while here the issue is `or` cannot be pushed down
totally.
##########
cpp/velox/substrait/SubstraitToVeloxPlan.cc:
##########
@@ -1669,7 +1669,8 @@ bool SubstraitToVeloxPlanConverter::canPushdownOr(
}
Review Comment:
And I suggest we add a `setOrRange` function in RangeRecorder which returns
false when other conditions already exist for a field.
##########
backends-velox/src/test/scala/org/apache/gluten/execution/MiscOperatorSuite.scala:
##########
@@ -239,6 +239,13 @@ class MiscOperatorSuite extends
VeloxWholeStageTransformerSuite with AdaptiveSpa
"select l_orderkey from lineitem " +
"where l_partkey in (1552, 674) or l_partkey in (1552) and l_orderkey
> 1") { _ => }
checkLengthAndPlan(df, 73)
+
+ runQueryAndCompare(
+ "select count(1) from lineitem " +
+ "where (l_shipmode in ('TRUCK', 'MAIL') or l_shipmode in ('AIR',
'FOB')) " +
+ "and l_shipmode in ('RAIL','SHIP')") {
Review Comment:
I think this issue is in the condition `(A or B) and C`, when C is already
pushed down, the `(A or B)` cannot be pushed down.
--
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]
