WangGuangxin commented on code in PR #5954:
URL: https://github.com/apache/incubator-gluten/pull/5954#discussion_r1716420160


##########
cpp/velox/substrait/SubstraitToVeloxPlan.cc:
##########
@@ -1712,19 +1733,6 @@ void SubstraitToVeloxPlanConverter::separateFilters(
   for (const auto& scalarFunction : scalarFunctions) {
     auto filterNameSpec = SubstraitParser::findFunctionSpec(functionMap_, 
scalarFunction.function_reference());
     auto filterName = SubstraitParser::getNameBeforeDelimiter(filterNameSpec);
-    // Add all decimal filters to remaining functions because their pushdown 
are not supported.
-    if (format == dwio::common::FileFormat::ORC && 
scalarFunction.arguments().size() > 0) {

Review Comment:
   The main idea of these three types (`Timestamp/VarBinary/HugeInt`) in this 
PR is that if we don't put them in `remainingFilter`, then when we call 
`mapToFilters`, a exception `Subfield filters creation not supported for input 
type '{}' in mapToFilters`  will throw, which will cause scan fallback. 
   
   The reason why `mapToFilters` don't support these three types is because 
there is no `MultiRangeType` or related type traits defined, which has nothing 
to do with the fileformat. cc @rui-mo @jiangjiangtian @kecookier 



-- 
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]

Reply via email to