dmitry-chirkov-dremio commented on code in PR #49900:
URL: https://github.com/apache/arrow/pull/49900#discussion_r3583848879
##########
cpp/src/gandiva/llvm_generator.cc:
##########
@@ -93,9 +102,19 @@ Status LLVMGenerator::Add(const ExpressionPtr expr, const
FieldDescriptorPtr out
Status LLVMGenerator::Build(const ExpressionVector& exprs,
SelectionVector::Mode mode) {
selection_vector_mode_ = mode;
+ std::vector<ValueValidityPairPtr> expr_value_validities;
for (auto& expr : exprs) {
+ ARROW_ASSIGN_OR_RAISE(auto value_validity, Decompose(expr));
+ expr_value_validities.push_back(value_validity);
+ }
+
+ ARROW_RETURN_NOT_OK(engine_->Init(std::move(functions_in_exprs_)));
Review Comment:
I think this selective path is still a no-op in the current control flow.
`LLVMGenerator::Make()` constructs the engine via `Engine::Make()`, and
`Engine::Make()` still calls the eager `Init()` that registers the full
C-function set before we ever reach this line. By the time we call
`Init(std::move(functions_in_exprs_))` here, the module already contains all of
those functions, so the filtering in `ExternalCFunctions::AddMappings()` never
changes what gets registered.
Can we either defer the original `engine->Init()` until the used-function
set is known, or split engine initialization so the default path is preserved
for tests/other callers but the projector path doesn't pre-register everything
first?
--
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]
