[GitHub] [arrow] wesm commented on a change in pull request #7264: ARROW-8926: [C++] Improve arrow/compute/*.h comments, correct typos and outdated language

[GitBox <gi...@apache.org>]

wesm commented on a change in pull request #7264:
URL: https://github.com/apache/arrow/pull/7264#discussion_r430008693



##########
File path: cpp/src/arrow/compute/exec.h
##########
@@ -60,29 +60,38 @@ static constexpr int64_t kDefaultExecChunksize = UINT16_MAX;
 /// function evaluation
 class ARROW_EXPORT ExecContext {
  public:
-  // If no function registry passed, the default is used
+  // If no function registry passed, the default is used.
   explicit ExecContext(MemoryPool* pool = default_memory_pool(),
                        FunctionRegistry* func_registry = NULLPTR);
 
+  /// \brief The MemoryPool used for allocations, default is
+  /// default_memory_pool().
   MemoryPool* memory_pool() const { return pool_; }
 
   ::arrow::internal::CpuInfo* cpu_info() const;
 
+  /// \brief The FunctionRegistry for looking up functions by name and
+  /// selecting kernels for execution. Defaults to the library-global function
+  /// registry provided by GetFunctionRegistry.
   FunctionRegistry* func_registry() const { return func_registry_; }
 
-  // \brief Set maximum length unit of work for kernel execution. Larger inputs
-  // will be split into smaller chunks, and, if desired, processed in
-  // parallel. Set to -1 for no limit
+  // \brief Set maximum length unit of work for kernel execution. Larger
+  // contiguous array inputs will be split into smaller chunks, and, if
+  // possible and enable, processed in parallel. The default chunksize is

Review comment:
       enabled

##########
File path: cpp/src/arrow/compute/kernel.h
##########
@@ -345,13 +388,13 @@ class ARROW_EXPORT KernelSignature {
                                                bool is_varargs = false);
 
   /// \brief Return true if the signature if compatible with the list of input
-  /// value descriptors
+  /// value descriptorsl

Review comment:
       typo. fixed

##########
File path: cpp/src/arrow/compute/kernel.h
##########
@@ -386,47 +435,75 @@ struct SimdLevel {
   enum type { NONE, SSE4_2, AVX, AVX2, AVX512, NEON };
 };
 
+/// \brief The strategy to use for propagating or otherwise populating the
+/// validity bitmap of a kernel output.
 struct NullHandling {
   enum type {
     /// Compute the output validity bitmap by intersecting the validity bitmaps
-    /// of the arguments. Kernel does not do anything with the bitmap
+    /// of the arguments. Kernel generally need not touch the bitmap
+    /// thereafter, but a kernel's exec function is permitted to alter the
+    /// bitmap after the null intersection is computed if it needs to.

Review comment:
       Yes. I'll clarify that it means a bitwise-and operation with all of the respective validity bitmaps




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org