edponce commented on a change in pull request #11023:
URL: https://github.com/apache/arrow/pull/11023#discussion_r740353081



##########
File path: cpp/src/arrow/compute/kernels/scalar_string.cc
##########
@@ -537,6 +549,358 @@ struct FixedSizeBinaryTransformExecWithState
   }
 };
 
+template <typename Type1, typename Type2>
+struct StringBinaryTransformBase {
+  using ViewType2 = typename GetViewType<Type2>::T;
+  using ArrayType1 = typename TypeTraits<Type1>::ArrayType;
+  using ArrayType2 = typename TypeTraits<Type2>::ArrayType;
+
+  virtual ~StringBinaryTransformBase() = default;
+
+  virtual Status PreExec(KernelContext* ctx, const ExecBatch& batch, Datum* 
out) {
+    return Status::OK();
+  }
+
+  virtual Status InvalidInputSequence() {
+    return Status::Invalid("Invalid UTF8 sequence in input");
+  }
+
+  // Return the maximum total size of the output in codeunits (i.e. bytes)
+  // given input characteristics for different input shapes.
+  // The Status parameter should only be set if an error needs to be signaled.
+
+  // Scalar-Scalar
+  virtual int64_t MaxCodeunits(const int64_t input1_ncodeunits, const 
ViewType2,
+                               Status*) {
+    return input1_ncodeunits;
+  }
+
+  // Scalar-Array
+  virtual int64_t MaxCodeunits(const int64_t input1_ncodeunits, const 
ArrayType2&,
+                               Status*) {
+    return input1_ncodeunits;
+  }
+
+  // Array-Scalar
+  virtual int64_t MaxCodeunits(const ArrayType1& input1, const ViewType2, 
Status*) {
+    return input1.total_values_length();
+  }
+
+  // Array-Array
+  virtual int64_t MaxCodeunits(const ArrayType1& input1, const ArrayType2&, 
Status*) {
+    return input1.total_values_length();
+  }
+
+  // Not all combinations of input shapes are meaningful to string binary
+  // transforms, so these flags serve as control toggles for enabling/disabling
+  // the corresponding ones. These flags should be set in the PreExec() method.
+  //
+  // This is an example of a StringTransform that disables support for 
arguments
+  // with mixed Scalar/Array shapes.
+  //
+  // template <typename Type1, typename Type2>
+  // struct MyStringTransform : public StringBinaryTransformBase<Type1, Type2> 
{
+  //   Status PreExec(KernelContext* ctx, const ExecBatch& batch, Datum* out) 
override {
+  //     EnableScalarArray = false;
+  //     EnableArrayScalar = false;
+  //     return StringBinaryTransformBase::PreExec(ctx, batch, out);
+  //   }
+  //   ...
+  // };
+  bool EnableScalarScalar = true;
+  bool EnableScalarArray = true;
+  bool EnableArrayScalar = true;
+  bool EnableArrayArray = true;
+};
+
+/// Kernel exec generator for binary (two parameters) string transforms.
+/// The first parameter is expected to always be a Binary/StringType while the
+/// second parameter is generic. Types of template parameter StringTransform
+/// need to define a transform method with the following signature:
+///
+/// int64_t Transform(const uint8_t* input, const int64_t 
input_string_ncodeunits,
+///                   const ViewType2 value2, uint8_t* output, Status* st);
+///
+/// where
+///   * `input` - input sequence (binary or string)
+///   * `input_string_ncodeunits` - length of input sequence in codeunits
+///   * `value2` - second argument to the string transform
+///   * `output` - output sequence (binary or string)
+///   * `st` - Status code, only set if transform needs to signal an error
+///
+/// and returns the number of codeunits of the `output` sequence or a negative
+/// value if an invalid input sequence is detected.
+template <typename Type1, typename Type2, typename StringTransform>
+struct StringBinaryTransformExecBase {
+  using offset_type = typename Type1::offset_type;
+  using ViewType2 = typename GetViewType<Type2>::T;
+  using ArrayType1 = typename TypeTraits<Type1>::ArrayType;
+  using ArrayType2 = typename TypeTraits<Type2>::ArrayType;
+
+  static Status Execute(KernelContext* ctx, StringTransform* transform,
+                        const ExecBatch& batch, Datum* out) {
+    if (batch[0].is_scalar()) {
+      if (batch[1].is_scalar()) {
+        if (transform->EnableScalarScalar) {
+          return ExecScalarScalar(ctx, transform, batch[0].scalar(), 
batch[1].scalar(),
+                                  out);
+        }
+      } else if (batch[1].is_array()) {
+        if (transform->EnableScalarArray) {
+          return ExecScalarArray(ctx, transform, batch[0].scalar(), 
batch[1].array(),
+                                 out);
+        }
+      }
+    } else if (batch[0].is_array()) {
+      if (batch[1].is_array()) {
+        if (transform->EnableArrayArray) {
+          return ExecArrayArray(ctx, transform, batch[0].array(), 
batch[1].array(), out);
+        }
+      } else if (batch[1].is_scalar()) {
+        if (transform->EnableArrayScalar) {
+          return ExecArrayScalar(ctx, transform, batch[0].array(), 
batch[1].scalar(),
+                                 out);
+        }
+      }
+    }
+    return Status::Invalid("Invalid combination of operands for binary string 
transform",
+                           " (", batch[0].ToString(), ", ", 
batch[1].ToString(),
+                           "). Only Array/Scalar kinds are supported.");

Review comment:
       Previous statement is false. We actually know the valid combinations, so 
we can print them as a helpful error comment.




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