ZhangHuiGui commented on code in PR #41975:
URL: https://github.com/apache/arrow/pull/41975#discussion_r1637454842


##########
cpp/src/arrow/compute/exec.cc:
##########
@@ -1123,6 +1142,34 @@ class VectorExecutor : public 
KernelExecutorImpl<VectorKernel> {
     }
   }
 
+  Status SetupPreallocation(const std::vector<Datum>& args) {

Review Comment:
   > Was this code churn necessary to achieve the goals of this PR?
   
   Yes, that's one of the goals of this pr, we could use a more precise 
pre-allocation way for vector-executor.
   
   The main change is to support the detection of null values ​​when 
`kernel_->null_handling == NullHandling::INTERSECTION`, so as to skip some 
unnecessary preallocations, including the null value detection of chunk-array, 
which was not done before. And also, the logic here is just same as 
`ScalarExecutor::SetupPreallocation`.
   
   Because this logic is decoupled from the scheduling of VectorExecutor 
Execute, it is better to consider extracting it.
   
   > Whats the difference between it and `SetupPreallocation` in Scalar?
   
   The difference between this function and `SetupPreallocation` of 
`ScalarExecutor` is that ScalarExecutor does not have an independent 
`chunk_exec`, so it needs to use `preallocate_contiguous_` to confirm whether 
it needs to execute `PrepareOutput` according to chunks through 
`span_iterator_`, which is not required in `VectorExecutor`.
   
   
   
   
   



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

Reply via email to