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

test: add string function test cases #4

Merged
merged 6 commits into from
Jun 26, 2023

Conversation

richtia
Copy link
Member

@richtia richtia commented May 3, 2023

No description provided.

@richtia richtia force-pushed the add_substring_test branch from 25a1712 to 9c13219 Compare May 9, 2023 17:00
@richtia
Copy link
Member Author

richtia commented May 10, 2023

The like function is having some issues: Reason: like requires second argument to be a constant of type VARCHAR

postgres does not support negative start positions for substring.

@richtia richtia requested a review from westonpace May 10, 2023 00:10
@richtia richtia marked this pull request as ready for review May 10, 2023 15:04
@richtia richtia force-pushed the add_substring_test branch from 2cdb0b5 to 3e03141 Compare May 15, 2023 20:57
@richtia richtia force-pushed the add_substring_test branch 4 times, most recently from 1de58ae to 7acb9db Compare May 31, 2023 18:48
@richtia richtia force-pushed the add_substring_test branch from 7acb9db to f86901f Compare June 12, 2023 20:59
@richtia richtia force-pushed the add_substring_test branch from f86901f to 78db3b9 Compare June 13, 2023 20:17
Copy link
Member

@westonpace westonpace left a comment

Choose a reason for hiding this comment

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

Let's mark pyvelox as unsupported for like for now and we can add a "requires_constant" option later when they come up with a newer release.

Let's open an issue for this before we merge this.

Let's make another PR to add a new option to substrait specifying behavior of negative start for substring. Choices are:

  • Wrap from the end (duckdb, sqlite?)
  • Go left past the beginning (postgres)
  • Throw an error (???)

For this PR maybe update the substring function to have some examples of each and use required_options similar to how we handle overflow in addition.

@richtia richtia force-pushed the add_substring_test branch from 9be5c62 to 0abb80e Compare June 14, 2023 21:13
@richtia
Copy link
Member Author

richtia commented Jun 14, 2023

Let's mark pyvelox as unsupported for like for now and we can add a "requires_constant" option later when they come up with a newer release.

Let's open an issue for this before we merge this.

Let's make another PR to add a new option to substrait specifying behavior of negative start for substring. Choices are:

  • Wrap from the end (duckdb, sqlite?)
  • Go left past the beginning (postgres)
  • Throw an error (???)

For this PR maybe update the substring function to have some examples of each and use required_options similar to how we handle overflow in addition.

  1. Created a clickup task for adding the requires constant option.
  2. Opened substrait PR for negative start in substrings: feat: add options to substring for start parameter being negative substrait#508
  3. Added the required_options in this PR

@richtia richtia requested a review from westonpace June 16, 2023 20:03
@westonpace westonpace merged commit 4f240de into substrait-io:main Jun 26, 2023
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.

2 participants