feat: support Utf8View for more args of regexp_replace

[mbutrovich]

Which issue does this PR close?

• Closes #.

Rationale for this change

#11667 added some Uft8View support for regexp_replace, but only for the first argument. The other args only support Uft8, which likely isn't a huge deal since they're probably short literals, but adding support for other string types makes it easier to simplify the signature. Systems like Comet (which may emit string literals as Utf8View) will also benefit from being able to use this function.

What changes are included in this PR?

• Change the signature to support more types, only when all the args are the same types.
• Update end-to-end tests for other string types.

Are these changes tested?

Yes, there are new tests. See above.

Are there any user-facing changes?

The signature for regexp_replace changed, but it was maybe wrong before (did not have LargeUtf8).

[mbutrovich]

I change all of the other arg string types at once since they hit the same code path (fetch_string_arg) so I don't think we need to test them independently.

[mbutrovich]

I'm afraid I might have lost the plot with all of those string args. I looked at other string functions to see if a huge match is how we handle this, but the worst I could find was 2 args, not 4.

[mbutrovich]

I'm not proud of the readability of the change, but at least there doesn't seem to be a meaningful performance regression:

main

regexp_replace_1000     time:   [1.5948 ms 1.5958 ms 1.5967 ms]

regexp_replace_1000 utf8view
                        time:   [1.5956 ms 1.5966 ms 1.5976 ms]

PR

regexp_replace_1000     time:   [1.5982 ms 1.5991 ms 1.6000 ms]

regexp_replace_1000 utf8view
                        time:   [1.6023 ms 1.6038 ms 1.6056 ms]

[alamb]

@mbutrovich -- thank you for this PR.

I still can't help but feel there is a mismatch between what this PR is doing (trying expand the types of string arguments this function supports) and what you are actually seeing in Comet.

Can you explain what is happening in comet? For example, is regexp_replace being called with regexp_replace(UTF8View, Utf8View, Utf8View)

I ask because a mismatch between the actual argument types, and the types a function accepts can be fixed in at least 2 ways:

1. Expand the types a function supports (this PR)
2. Cast the arguments to some combination that the function supports (via coercion).

Typically coercion is done in the LogicalPlan, and I know that Comet does not use LogicalPlans. However, @adriangb and @kosiew have been working on a version of this for PhysicalExprs, in the PhysicalExprAdapter

I also reviewed the documentation on types and type signatures and I think there is some ambiguity about what a function "supporting argument types" means. I will make a PR to improve the documentation in this area

[mbutrovich]

I haven't forgotten about this. I need to generate a new .slt file for the relevant tests.

[mbutrovich]

Can you explain what is happening in comet? For example, is regexp_replace being called with regexp_replace(UTF8View, Utf8View, Utf8View)

In my experimental branch adding StringView support to Comet, we need a way to represent string literals during serialization from the Spark side to DataFusion. Currently all string literals come over as Utf8 and that just works. However, with Utf8View columns coming out of the Parquet reader, Arrow complains about not being able to evaluate filter expressions with mismatched types. I changed all string literals to be Utf8View, which underneath doesn't really change anything underneath for single ScalarValues. Now, however, I have problems with functions like regexp_replace which expect literals to be Utf8. Since Comet does not use DataFusion's front-end, we don't get the cast operations inserted into the plan that the signature logic is designed for.

I am increasingly of the mind that Comet needs to start doing some passes over the physical plan, and type coercion like this might be one reason.

I think this PR is good to go, but also am okay if we think it's needless complexity.

[Jefffrey]

I think this is an option too?

Uniform(3, vec![Utf8View, LargeUtf8, Utf8]),
Uniform(4, vec![Utf8View, LargeUtf8, Utf8]),

I tested it on a checked out copy of this PR and the slt tests passed

[Jefffrey]

👍

[mbutrovich]

Thanks for the reviews and helpful feedback @Jefffrey and @alamb! I'm hopeful that this will get Comet closer to fully supporting Utf8View.