rui-mo commented on code in PR #5966:
URL: https://github.com/apache/incubator-gluten/pull/5966#discussion_r1625885474
##########
cpp/velox/substrait/SubstraitToVeloxPlan.cc:
##########
@@ -1577,26 +1582,28 @@ bool SubstraitToVeloxPlanConverter::canPushdownNot(
std::vector<RangeRecorder>& rangeRecorders) {
VELOX_CHECK(scalarFunction.arguments().size() == 1, "Only one arg is
expected for Not.");
const auto& notArg = scalarFunction.arguments()[0];
- if (!notArg.value().has_scalar_function()) {
- // Not for a Boolean Literal or Or List is not supported curretly.
- // It can be pushed down with an AlwaysTrue or AlwaysFalse Range.
- return false;
- }
-
- auto argFunction =
- SubstraitParser::findFunctionSpec(functionMap_,
notArg.value().scalar_function().function_reference());
- auto functionName = SubstraitParser::getNameBeforeDelimiter(argFunction);
+ if (notArg.value().has_singular_or_list()) {
+ auto singularOrList = notArg.value().singular_or_list();
+ uint32_t colIdx = getColumnIndexFromSingularOrList(singularOrList);
+ return rangeRecorders.at(colIdx).setInRange();
+ } else if (notArg.value().has_scalar_function()) {
+ auto argFunction =
+ SubstraitParser::findFunctionSpec(functionMap_,
notArg.value().scalar_function().function_reference());
+ auto functionName = SubstraitParser::getNameBeforeDelimiter(argFunction);
- static const std::unordered_set<std::string> supportedNotFunctions = {sGte,
sGt, sLte, sLt, sEqual};
+ static const std::unordered_set<std::string> supportedNotFunctions =
{sGte, sGt, sLte, sLt, sEqual};
- uint32_t fieldIdx;
- bool isFieldOrWithLiteral =
fieldOrWithLiteral(notArg.value().scalar_function().arguments(), fieldIdx);
+ uint32_t fieldIdx;
+ bool isFieldOrWithLiteral =
fieldOrWithLiteral(notArg.value().scalar_function().arguments(), fieldIdx);
- if (supportedNotFunctions.find(functionName) != supportedNotFunctions.end()
&& isFieldOrWithLiteral &&
- rangeRecorders.at(fieldIdx).setCertainRangeForFunction(functionName,
true /*reverse*/)) {
- return true;
+ if (supportedNotFunctions.find(functionName) !=
supportedNotFunctions.end() && isFieldOrWithLiteral &&
+ rangeRecorders.at(fieldIdx).setCertainRangeForFunction(functionName,
true /*reverse*/)) {
+ return true;
+ }
+ return false;
Review Comment:
This return seems to be not needed as we can return false finally.
--
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]
