Skip to content
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

[C++] Bug in substring_index with negative index. #36182

Closed
lriggs opened this issue Jun 20, 2023 · 1 comment · Fixed by #36184
Closed

[C++] Bug in substring_index with negative index. #36182

lriggs opened this issue Jun 20, 2023 · 1 comment · Fixed by #36184

Comments

@lriggs
Copy link
Contributor

lriggs commented Jun 20, 2023

Describe the bug, including details regarding any error messages, version, and platform.

substring_index("Abc.DE.fGh", '.', -2) returns "fGh" but it should return "DE.fGh" (ie starting from the second occurrence of the delimiter from the right). The proposed behavior matches the behavior of other databases.

substring_index was added here: #32374

Component(s)

C++ - Gandiva

@lriggs
Copy link
Contributor Author

lriggs commented Jun 20, 2023

take

pitrou pushed a commit that referenced this issue Jun 28, 2023
…egative. (#36184)

### Rationale for this change

substring_index("Abc.DE.fGh", '.', -2) returns "fGh" but it should return "DE.fGh" (ie starting from the second occurrence of the delimiter from the right). The proposed behavior matches the behavior of other databases.

### What changes are included in this PR?

Fixed reverse index calculation and updated unit tests.

### Are these changes tested?

Yes, unit tests and integration testing.

### Are there any user-facing changes?

Function behavior change with substring_index function.

* Closes: #36182

Authored-by: Projjal Chanda <iam@pchanda.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
@pitrou pitrou added this to the 13.0.0 milestone Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants