zanmato1984 commented on code in PR #50989:
URL: https://github.com/apache/arrow/pull/50989#discussion_r4118576884
##########
cpp/src/arrow/engine/substrait/extension_set.cc:
##########
@@ -961,6 +975,60 @@ ExtensionIdRegistry::SubstraitCallToArrow
DecodeConcatMapping() {
};
}
+// Substrait's starts_with / ends_with / contains take the pattern as a second
+// value argument. The matching Arrow kernels (starts_with / ends_with /
+// match_substring) are unary and carry the pattern in MatchSubstringOptions,
so
+// the second argument must be a string literal.
+ExtensionIdRegistry::SubstraitCallToArrow DecodeMatchSubstringMapping(
+ const std::string& function_name) {
+ return [function_name](const SubstraitCall& call) ->
Result<compute::Expression> {
+ if (call.size() != 2) {
+ return Status::NotImplemented("Acero does not have a kernel for ",
function_name,
+ " that receives ", call.size(), "
arguments");
+ }
+ ARROW_ASSIGN_OR_RAISE(
+ CaseSensitivity case_sensitivity,
+ ParseOptionOrElse(
+ call, "case_sensitivity", kCaseSensitivityParser,
+ {CaseSensitivity::kCaseSensitive,
CaseSensitivity::kCaseInsensitive},
+ CaseSensitivity::kCaseSensitive));
+ ARROW_ASSIGN_OR_RAISE(compute::Expression input, call.GetValueArg(0));
+ ARROW_ASSIGN_OR_RAISE(compute::Expression substring, call.GetValueArg(1));
+ const Datum* pattern = substring.literal();
+ if (pattern == nullptr || !pattern->is_scalar() ||
+ pattern->type()->id() != Type::STRING || !pattern->scalar()->is_valid)
{
+ return Status::NotImplemented(
+ "The Arrow ", function_name,
+ " kernel requires the substring argument to be a non-null string
literal");
+ }
+ auto options = std::make_shared<compute::MatchSubstringOptions>(
+ std::string(pattern->scalar_as<StringScalar>().view()),
+ /*ignore_case=*/case_sensitivity == CaseSensitivity::kCaseInsensitive);
+ return compute::call(function_name, {std::move(input)},
std::move(options));
+ };
+}
+
+ExtensionIdRegistry::ArrowToSubstraitCall EncodeMatchSubstring(Id
substrait_fn_id) {
+ return
+ [substrait_fn_id](const compute::Expression::Call& call) ->
Result<SubstraitCall> {
+ if (call.options == nullptr) {
+ return Status::Invalid("The call to ", call.function_name,
+ " is missing its MatchSubstringOptions");
+ }
+ auto match_options =
+ checked_pointer_cast<compute::MatchSubstringOptions>(call.options);
+ // nullable=true errs on the side of caution
+ SubstraitCall substrait_call(substrait_fn_id, call.type.GetSharedPtr(),
+ /*nullable=*/true);
+ substrait_call.SetValueArg(0, call.arguments[0]);
+ substrait_call.SetValueArg(1,
compute::literal(match_options->pattern));
Review Comment:
Good catch. I think we should check the bound input type before using the
standard Substrait string mapping. These Arrow kernels also accept binary and
large binary/string inputs, but `functions_string.yaml` has no matching
signatures for those types, so the current encoder can produce an invalid
standard Substrait call. With the current type mappings, I think we should only
accept plain string input here, return `NotImplemented` for unsupported types,
and add a binary regression test. We should also ensure the pattern is valid
for a Substrait string literal.
--
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]
