SNOW-876999: Add support for timezone-aware type annotation on UDF #979
Merged
sfc-gh-jdu merged 2 commits intomainfrom Jul 28, 2023
Merged
SNOW-876999: Add support for timezone-aware type annotation on UDF #979sfc-gh-jdu merged 2 commits intomainfrom
sfc-gh-jdu merged 2 commits intomainfrom
Conversation
sfc-gh-azhan
approved these changes
Jul 27, 2023
Codecov Report
@@ Coverage Diff @@
## main #979 +/- ##
==========================================
- Coverage 98.35% 98.34% -0.01%
==========================================
Files 51 51
Lines 9227 9298 +71
Branches 1671 1691 +20
==========================================
+ Hits 9075 9144 +69
- Misses 61 62 +1
- Partials 91 92 +1
... and 7 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
sfc-gh-sfan
reviewed
Jul 28, 2023
Collaborator
sfc-gh-sfan
left a comment
sfc-gh-sfan
left a comment
There was a problem hiding this comment.
Please also address the comment about test coverage for default case :)
| timezone = TimestampTimeZone.LTZ | ||
| elif tp_args[0] == TZ: | ||
| timezone = TimestampTimeZone.TZ | ||
| else: |
Collaborator
There was a problem hiding this comment.
Just curious: what can this else be? Or should we actually raise?
Collaborator
Author
There was a problem hiding this comment.
Actually it can be Timestamp[int], which is invalid and we should raise an exception here, good catch! I added tests to cover these code path.
sfc-gh-sfan
approved these changes
Jul 28, 2023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 SNOW-876999
Fill out the following pre-review checklist:
Please describe how your code solves the related issue.
Previously we add support for timestamp variations in data types, but we also need such support for UDFs with type annotations.