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

feat: add case_sensitivity option to string functions #289

Merged
merged 2 commits into from
Aug 25, 2022

Conversation

richtia
Copy link
Member

@richtia richtia commented Aug 16, 2022

Add an option to control case sensitivity to string functions that may need it.

BREAKING CHANGE: added option arguments to existing functions.

jvanstraten
jvanstraten previously approved these changes Aug 17, 2022
Copy link
Contributor

@jvanstraten jvanstraten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, and the added documentation makes things look a lot better already. Note that this is a breaking change though (the required: false is not what you might think it means), so I've edited the description accordingly so it ends up in the squash commit (at least I think it does).

Looking at this I also realized that for another PR I got pedantic about making an option for ASCII vs full unicode case conversions, and the same logic applies to these options, so maybe there should be a CASE_INSENSITIVE_ASCII option added to the end. But that should probably go in another PR.

@richtia richtia force-pushed the string_case_sensitivity branch from c1422ef to 8bab17f Compare August 24, 2022 19:08
@cpcloud cpcloud merged commit 4c354de into substrait-io:main Aug 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants