-
Notifications
You must be signed in to change notification settings - Fork 567
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
Snowflake: support IGNORE/RESPECT NULLS inside function argument list #1263
Conversation
Pull Request Test Coverage Report for Build 9090118483Details
💛 - Coveralls |
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.
LGTM! cc @alamb
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.
Shall we add a test for this too so it doesn't get broken accidentally in a future release?
Thanks for the review @iffyio
@alamb |
What I was thinking was "if someone accidentally broke this feature in the future (by reverting this code, for example), there would be no test failure to let us know of the regression" If you are fine with that risk I think we can merge this PR once its conflicts are resolved, given how small it is. |
@alamb Gotcha, added a Snowflake specific test and resolved the conflicts |
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.
Thanks @jmhain
Just a tiny fix for a parsing failure I hit.