fix: substr_index not handling negative occurrence correctly
#9475
Conversation
github-actions
bot
added
physical-expr
| }); | ||
| } | ||
| } | ||
| // In MySQL, these cases will return an empty string. |
There was a problem hiding this comment.
I like new implementation which looks more concise to me and easier to read.
About the MySQL comments, does that mean some cases are not the same between MySQL and Postgres?
There was a problem hiding this comment.
About the MySQL comments, does that mean some cases are not the same between MySQL and Postgres?
No, actually there is no such function in PostgreSQL. The closest one is split_part, but their functionalities are different.
There was a problem hiding this comment.
I tried it out here too https://www.w3schools.com/mysql/trymysql.asp?filename=trysql_func_mysql_substring_index2
And this behavior is consistent with what I observed
|
|
||
| # Test substring_index using 'ac' as delimiter | ||
| query TIT | ||
| SELECT str, n, substring_index(str, 'ac', n) AS c FROM |
There was a problem hiding this comment.
the CTE style of these tests is very nice 👌
| }); | ||
| } | ||
| } | ||
| // In MySQL, these cases will return an empty string. |
There was a problem hiding this comment.
I tried it out here too https://www.w3schools.com/mysql/trymysql.asp?filename=trysql_func_mysql_substring_index2
And this behavior is consistent with what I observed
3 participants
Which issue does this PR close?
Closes #9472.
Rationale for this change
Example query:
select substr_index('arrow.apache.org', '.', -1).Expected result is
org, but the current result of main branch ise.org.The previous test input was "www.apache.org", where "www" and "org" have the same length, which coincidentally led to the tests on negative occurrences being passed.
Background: Since this function originates from MySQL, its behavior should remain consistent with MySQL.
What changes are included in this PR?
Refactor the implementation of
substr_indexand fix bug.Are these changes tested?
Yes
Are there any user-facing changes?
No