-
Notifications
You must be signed in to change notification settings - Fork 120
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
SNOW-1016709: Add local testing support for STRIP_NULL_VALUE #1222
Conversation
CHANGELOG.md
Outdated
@@ -16,6 +16,8 @@ | |||
- `array_except` | |||
- `create_map` | |||
- `sign`/`signum` | |||
- Added support for the following local testing functions: |
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.
This should move to a new release.
def mock_strip_null_value(expr: ColumnEmulator): | ||
return ColumnEmulator( | ||
[None if x == "null" else x for x in expr], sf_type=expr.sf_type | ||
) |
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.
I feel like nullability could be different than the input column?
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.
same question about the nullability of the ColumnType, is the current backend behavior always returning true?
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.
Experimentally it appears to always return a column that is nullable. I wonder if it would be useful to create a test that runs both the normal version and local testing version of a test and then ensures they return the same result.
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.
Tests outside tests/mock
that are marked with pytest.mark.localtest
are run against both a live connection and a local testing connection.
892295f
to
32a0861
Compare
def mock_strip_null_value(expr: ColumnEmulator): | ||
return ColumnEmulator( | ||
[None if x == "null" else x for x in expr], sf_type=expr.sf_type | ||
) |
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.
same question about the nullability of the ColumnType, is the current backend behavior always returning true?
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.
The code changes LGTM, please fix the changelog before merging.
470950c
to
a3e34fc
Compare
Please answer these questions before submitting your pull requests. Thanks!
What GitHub issue is this PR addressing? Make sure that there is an accompanying issue to your PR.
Fixes #1016709
Fill out the following pre-review checklist:
Please describe how your code solves the related issue.
This PR adds local testing support for STRIP_NULL_VALUE.
Question for reviewers:
Is STRIP_NULL_VALUE always called with a value column or should it be able to walk a json object and replace all instances of 'null'?