fix: apply regexp_instr subexpr to the N-th match, not the first

[MaxFreedomPollard]

Which issue does this PR close?

• No issue filed. I hit this reading get_index in regexpinstr.rs to work out how N and subexpr are meant to combine. Happy to file one if the changelog needs the link.

Rationale for this change

regexp_instr ignores N whenever subexpr is greater than zero. It always reports a position inside the first match, whichever occurrence you asked for.

> SELECT regexp_instr('12 abc def ghi 34 abc def ghi 56', '(abc) (def) (ghi)', 1, 2, 'i', 2);
8   -- the 'def' in the first match. Should be 23, the 'def' in the second.

> SELECT regexp_instr('12 abc def ghi 34 abc def ghi 56', '(abc) (def) (ghi)', 1, 3, 'i', 1);
4   -- there is no third match. Should be 0.

The second one is the worse shape: a query that asks for an occurrence which does not exist gets a real position back, so nothing in the result says the match was missing.

PostgreSQL, which this function follows, defines regexp_instr as returning "the starting or ending position of the N'th match", and subexpr as "an integer indicating which subexpression is of interest: the result identifies the position of the substring matching that subexpression" of that match. DataFusion's own doc string promises the two things independently: "N: Optional The N-th occurrence of pattern to find" and "subexpr: Optional Specifies which capture group (subexpression) to return the position for".

What changes are included in this PR?

get_index in datafusion/functions/src/regex/regexpinstr.rs picked the match with Regex::captures, which only ever returns the first match, and dropped n on that branch. It now uses Regex::captures_iter(...).nth(n - 1), the capture-group counterpart of the find_iter(...).nth(n - 1) the subexpr == 0 branch already used, so both branches count occurrences the same way. The comment claiming subexpr took precedence over N goes with it.

N = 1 is unchanged, because captures_iter(...).nth(0) and captures(...) are the same match. That is why no existing test or .slt expectation moves.

What is the testing strategy for this PR?

Every existing test of subexpr, in regexpinstr.rs and in regexp/regexp_instr.slt alike, used N = 1, which is how this survived.

test_case_sensitive_regexp_instr_scalar_subexp gains N = 2 for each of the three capture groups, N = 3 where no such match exists, and a start = 18 row so start and subexpr are exercised together. The harness already runs every row through Utf8, LargeUtf8 and Utf8View. regexp/regexp_instr.slt gains the two queries above.

Reverting only regexpinstr.rs to main and keeping the new tests, both fail:

$ cargo test --profile ci -p datafusion-functions --lib regexp_instr
panicked at datafusion/functions/src/regex/regexpinstr.rs:740:21:
assertion `left == right` failed: regexp_instr scalar test failed
  left: Some(4)
 right: Some(19)

$ cargo test --profile ci -p datafusion-sqllogictest --test sqllogictests -- regexp/regexp_instr
1. query result mismatch:
[SQL] SELECT regexp_instr('12 abc def ghi 34 abc def ghi 56', '(abc) (def) (ghi)', 1, 2, 'i', 2);
[Diff] (-expected|+actual)
-   23
+   8
2. query result mismatch:
[SQL] SELECT regexp_instr('12 abc def ghi 34 abc def ghi 56', '(abc) (def) (ghi)', 1, 3, 'i', 1);
[Diff] (-expected|+actual)
-   0
+   4

With the fix, on aarch64-apple-darwin, rustc 1.97.0:

$ cargo test --profile ci -p datafusion-functions --lib
test result: ok. 341 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

$ cargo test --profile ci -p datafusion-sqllogictest --test sqllogictests -- regexp
Progress: 5/5 files completed (100%)

$ cargo fmt --all -- --check
$ cargo clippy --profile ci -p datafusion-functions --all-targets -- -D warnings

Are there any user-facing changes?

Yes. regexp_instr(str, regexp, start, N, flags, subexpr) with N greater than 1 and subexpr greater than 0 now returns a position in the N'th match, and 0 when there is no N'th match. No API change.

One thing I did not touch: expr_fn::regexp_instr in datafusion/functions/src/regex/mod.rs takes an endoption argument and pushes it as the fifth positional argument. PostgreSQL has that parameter, but this UDF does not, so its fifth argument is flags. Any caller passing Some(endoption) builds a seven-argument call that no signature accepts. That looked like a separate change to me, and possibly an API break, so I left it alone.