Remove null and empty fields from native queries#12634
Merged
gianm merged 6 commits intoapache:masterfrom Jun 16, 2022
Merged
Conversation
|
This pull request introduces 3 alerts when merging 1c70cf1 into 1ace733 - view on LGTM.com new alerts:
|
|
This pull request introduces 3 alerts when merging e9a623c into 1ace733 - view on LGTM.com new alerts:
|
|
This pull request introduces 3 alerts when merging 922087b into 1ace733 - view on LGTM.com new alerts:
|
gianm
reviewed
Jun 13, 2022
|
This pull request introduces 3 alerts when merging a93bf0d into 27e8b43 - view on LGTM.com new alerts:
|
|
This pull request introduces 3 alerts when merging ec67530 into 1f6e888 - view on LGTM.com new alerts:
|
|
This pull request introduces 3 alerts when merging 2969d46 into 45e3111 - view on LGTM.com new alerts:
|
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This is a subset of the changes for the planner test framework, split out to make reviews easier.
Druid native queries are often serialized to JSON. In the planner framework, we save the JSON as an "expected plan." To make the resulting files a bit smaller, this PR removes from the JSON those fields with default values: values that Jackson would infer even if the fields are not present. For example, rather than spelling out
"foo": null, we can simply omit"foo"and get the same results. The same is true for"foo": {}and"foo": []since most constructors will create an empty collection if passed a null.The clean-up is done using the
@JsonIncludeannotation, sometimes with a custom filter class. Scan Query has a batch size which defaults to4096 * 5if the provided value is 0. Given this, added a filter which omits the value if it is the default, which avoids cluttering the deserialized JSON with magic numbers.Some unit tests check for an exact serialization of the JSON, including the now-removed
nullfields. All such tests were updated with the revised format.This change is purely cosmetic: the deserialized query objects do not change meaning. Tested via unit tests, including the new planner test framework.
This PR has: