apache / apache/datafusion

Unify implementations of `regexp_like` and `*` / `*~` operators

Open
#17,941 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
9.3k
Forks
2.4k
Avg merge
3d 7h
Merged PRs (30d)
344

Description

@pepijnve found that `*` goes faster than `regexp_like` for some cases, see:
- https://github.com/apache/datafusion/pull/17839

> If `~` is faster than `regexp_like` can we simply change the implementation to use the same underlying implementation of `~` (why only rewrite in some cases?)

And the answer in https://github.com/apache/datafusion/pull/17839#discussion_r2402499183 is succinctly summarized as

> That's probably the way to go to long term to deduplicate the code entirely, but that would be a much bigger change.

The full answer:
> See https://github.com/apache/datafusion/issues/17838#issuecomment-3355083929
>
> The operator logic is in `physical_expr`, while `regexp_like` lives in `functions`. We would probably have to move the common logic to a separate crate. This PR was intended as a stopgap solution for common cases.
>
> We can only rewrite in some cases because of the optional `flags` argument. With the operators all you have is the case sensitivity (i.e. the `i`flag).
>
> The reason for the operator being more efficient is that it will make use of the `regexp_is_match_scalar` kernel if it can, while `regexp_like` always uses `regexp_is_match`. `regexp_is_match` does maintain a cache of compiled regexes so at least the pattern isn't compiled over and over again, but it's still quite a bit more code compared to `regexp_is_match_scalar`.
>
> Additionally there's a regular expression simplification rule that only operates on `BinaryExpr` with one of the regex matching operators. The transformation here enables that optimisation for `regexp_like` calls as well.

This ticket tracks creating a single implementation

I'd like to see the different calls use the same implementation, having 2 implementations for this seems problematic. I'll file a followup issue is no one else does that references this ticket to create a common implementation.

_Originally posted by @Omega1 in https://github.com/apache/datafusion/issues/17839#issuecomment-3371761835_

Contributor guide

Open the contributing guide

Research direction

Compare the operator logic in `physical_expr` with the `regexp_like` implementation in `functions`, starting from their uses of the regexp kernels. Determine how a shared implementation can preserve flags, caching, scalar-kernel behavior, and the regex simplification rule. Done means the different calls use one common implementation without losing existing behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
data-engineering
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.