-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: substr_index
not handling negative occurrence correctly
#9475
Conversation
}); | ||
} | ||
} | ||
// In MySQL, these cases will return an empty string. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
# 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
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_index
and fix bug.Are these changes tested?
Yes
Are there any user-facing changes?
No