Copilot commented on code in PR #47377:
URL: https://github.com/apache/arrow/pull/47377#discussion_r3756066719
##########
cpp/src/arrow/compute/exec.cc:
##########
@@ -442,6 +450,13 @@ bool ExecSpanIterator::Next(ExecSpan* span) {
PromoteExecSpanScalars(span);
}
+ if (!have_all_scalars_ || promote_if_all_scalars_) {
+ if (selection_vector_) {
+ DCHECK_NE(selection_span, nullptr);
+ *selection_span = SelectionVectorSpan(selection_vector_->indices());
+ }
+ }
Review Comment:
ExecSpanIterator::Next() can be called with selection_span == nullptr
(default arg), but the initialization path DCHECKs when a selection vector is
present. This can crash callers (and in release can segfault) whenever
ExecBatch carries a selection vector but the caller doesn't request a
SelectionVectorSpan.
This issue also appears on line 484 of the same file.
##########
cpp/src/arrow/compute/exec.cc:
##########
@@ -367,6 +369,12 @@ Status ExecSpanIterator::Init(const ExecBatch& batch,
int64_t max_chunksize,
value_offsets_.clear();
value_offsets_.resize(args_->size(), 0);
max_chunksize_ = std::min(length_, max_chunksize);
+ selection_vector_ = batch.selection_vector.get();
+ if (selection_vector_) {
+ selection_length_ = selection_vector_->length();
+ } else {
+ selection_length_ = 0;
+ }
Review Comment:
ExecSpanIterator assumes the selection vector is sorted and in-bounds
(lower_bound slicing + later kernels indexing). Without validation, an invalid
selection vector can lead to incorrect slicing and out-of-bounds reads/writes
(especially with TakeOptions{boundscheck=false} in dense fallback).
##########
cpp/src/arrow/compute/exec.cc:
##########
@@ -888,7 +985,7 @@ class ScalarExecutor : public
KernelExecutorImpl<ScalarKernel> {
} else if (kernel_->null_handling == NullHandling::OUTPUT_NOT_NULL) {
Review Comment:
When executing via selective_exec, null propagation / output validity is
still computed for the entire span, but selective kernels are described as only
computing selected indices. If a selective kernel doesn't explicitly write
non-selected slots, those slots may remain uninitialized while marked valid,
potentially exposing stale heap contents in the output array.
--
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]
