-
Notifications
You must be signed in to change notification settings - Fork 13.9k
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
fix(presto): expand data with null item #17470
Conversation
Codecov Report
@@ Coverage Diff @@
## master #17470 +/- ##
=======================================
Coverage 76.94% 76.95%
=======================================
Files 1042 1043 +1
Lines 56254 56324 +70
Branches 7785 7785
=======================================
+ Hits 43286 43345 +59
- Misses 12710 12721 +11
Partials 258 258
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
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 for the fix @ganczarek + the awesome unit test! One comment before submitting this for broader review.
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, but asking for a second pair of eyes
@john-bodley could you take a look and see if you agree with this change? |
Since this is plugging a very specific bug, has very low risk of introducing regressions and adds good tests, I'll go ahead and merge this (we can do incremental improvements on this functionality later if needed). |
* fix(presto): expand data with null item * Fixed pre-commit check
SUMMARY
A simple fix for #17469
TESTING INSTRUCTIONS
Run Presto engine tests:
ADDITIONAL INFORMATION
PRESTO_EXPAND_DATA