kosiew commented on code in PR #22311:
URL: https://github.com/apache/datafusion/pull/22311#discussion_r3264797149
##########
datafusion/sqllogictest/test_files/regexp/regexp_count.slt:
##########
@@ -51,6 +51,11 @@ SELECT regexp_count('ABCABCABCABC', 'Abc', 1, 'i');
----
4
+query I
Review Comment:
Nice addition to cover `regexp_count('abc', '')`. Since this change also
affects the start-position and flag handling paths, it would be great to add a
couple more SQL-visible cases here as well, like `regexp_count('abc', '', 2)`
and `regexp_count('abc', '', 1, 'i')`. It may also be worth covering the
one-past-end or beyond-end boundary behavior if that is intentional. The Rust
unit tests already exercise some of the internals, but adding these to the SLT
suite would help lock in the user-facing behavior that motivated this PR.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
