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

Add test cases for comparison functions with decimal arguments #87

Merged
merged 1 commit into from
Aug 13, 2024

Conversation

anshuldata
Copy link
Contributor

@anshuldata anshuldata commented Aug 1, 2024

  • Added test cases for comparison function with decimal arguments
  • Fixed a bug (regression in this PR) where unsupported dialect type should return error. Dialect checks for "None" if type is unsupported. In previous PR I changed it to throw exception (e.g. here and here) hence snowflake tests were failing for unsupported type (i.e. i8)
  • Kept precision/scale same as substriat. All input type is "any" so type really doesn't matter. For Result type is either "any" or "boolean" so no specific care needed in type for decimal

@anshuldata anshuldata changed the title Add test cases for comparison functions with decimal arguments Draft: Add test cases for comparison functions with decimal arguments Aug 1, 2024
@anshuldata anshuldata marked this pull request as draft August 1, 2024 13:27
@anshuldata anshuldata changed the title Draft: Add test cases for comparison functions with decimal arguments Add test cases for comparison functions with decimal arguments Aug 1, 2024
@anshuldata anshuldata marked this pull request as ready for review August 1, 2024 13:49
@anshuldata
Copy link
Contributor Author

@richtia kindly review

@anshuldata
Copy link
Contributor Author

@richtia I think this PR needs to be merged first, since it fixes a regression

@richtia richtia merged commit dd9d355 into substrait-io:main Aug 13, 2024
7 checks passed
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