Copilot commented on code in PR #12793:
URL: https://github.com/apache/gluten/pull/12793#discussion_r3796825887
##########
cpp/velox/operators/functions/SparkExprToSubfieldFilterParser.cc:
##########
@@ -16,11 +16,83 @@
*/
#include "operators/functions/SparkExprToSubfieldFilterParser.h"
+#include "utils/Exception.h"
+#include "velox/common/base/BloomFilter.h"
+#include "velox/expression/Expr.h"
+#include "velox/functions/sparksql/XxHash64.h"
+#include "velox/vector/ComplexVector.h"
+
namespace gluten {
using namespace facebook::velox;
namespace {
+
+// Evaluates an expression as a constant. Returns nullptr if the expression is
+// not constant or evaluation fails. Errors are intentionally swallowed because
+// a non-evaluable expression simply means the filter cannot be pushed down.
+VectorPtr toConstant(const core::TypedExprPtr& expr,
core::ExpressionEvaluator* evaluator) {
+ auto exprSet = evaluator->compile(expr);
+ if (!exprSet->exprs()[0]->isConstantExpr()) {
+ return nullptr;
+ }
+ RowVector input(evaluator->pool(), ROW({}, {}), nullptr, 1,
std::vector<VectorPtr>{});
+ SelectivityVector rows(1);
+ VectorPtr result;
+ try {
+ evaluator->evaluate(exprSet.get(), rows, input, result);
+ } catch (const VeloxUserError& error) {
+ VLOG(1) << "Failed to evaluate constant expression for scan filter
pushdown: " << error.what();
+ return nullptr;
+ }
+ return result;
+}
Review Comment:
`toConstant` claims to swallow evaluation failures, but `compile()` (and
potentially `isConstantExpr()`) can also throw; additionally, only
`VeloxUserError` is caught. This can turn a non-pushdownable expression into a
hard failure. Consider wrapping `compile` + evaluate in a broader exception
handler (e.g., `VeloxException` / `std::exception`) and returning `nullptr`
consistently for any failure in this best-effort pushdown path.
--
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]