dmitry-chirkov-dremio commented on code in PR #48597:
URL: https://github.com/apache/arrow/pull/48597#discussion_r3581632040
##########
cpp/src/gandiva/projector.cc:
##########
@@ -230,8 +230,6 @@ Status Projector::AllocArrayData(const DataTypePtr& type,
int64_t num_records,
}
Status Projector::ValidateEvaluateArgsCommon(const arrow::RecordBatch& batch)
const {
- ARROW_RETURN_IF(!batch.schema()->Equals(*schema_),
Review Comment:
Removing the full schema equality check seems risky unless we replace it
with an equally strong validation of the referenced fields. Right now the new
runtime path only checks matching names that happen to be present, so this
change broadens the accepted input batches without guaranteeing that the JIT's
expected buffer layout is still satisfied.
Can we either keep the old schema check for now, or add explicit validation
that every referenced field descriptor was matched before executing the
compiled expression?
##########
cpp/src/gandiva/filter.cc:
##########
@@ -91,8 +91,6 @@ Status Filter::Make(SchemaPtr schema, ConditionPtr condition,
Status Filter::Evaluate(const arrow::RecordBatch& batch,
std::shared_ptr<SelectionVector> out_selection) {
const auto num_rows = batch.num_rows();
- ARROW_RETURN_IF(!batch.schema()->Equals(*schema_),
Review Comment:
Same concern here as in `Projector::Evaluate`: once the schema equality
guard is removed, the safety of cache reuse depends on the replacement
validation being complete. As written, the new check only validates types for
matched columns and does not fail when a referenced field is missing entirely.
That means we can accept batches that the original code would have rejected,
without proving that all input buffer slots expected by the compiled filter
were populated.
--
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]
