Fix exception when using complex aggs with result level caching#7614
Merged
jon-wei merged 7 commits intoapache:masterfrom May 9, 2019
Merged
Fix exception when using complex aggs with result level caching#7614jon-wei merged 7 commits intoapache:masterfrom
jon-wei merged 7 commits intoapache:masterfrom
Conversation
gianm
approved these changes
May 8, 2019
Contributor
gianm
left a comment
gianm
left a comment
There was a problem hiding this comment.
LGTM with or without the extraction of a helper function (although I do think it would be good to do that).
|
|
||
| Iterator<AggregatorFactory> aggsIter = aggs.iterator(); | ||
|
|
||
| // When using the result level cache, the agg values seen here are |
Contributor
There was a problem hiding this comment.
There's a long comment and identical block in each toolchest -- perhaps extract to a helper method in CacheUtil?
Contributor
Author
There was a problem hiding this comment.
Extracted to a helper method in CacheStrategy (CacheUtil is in server which is not a dependency of processing)
jon-wei
added a commit
to implydata/druid-public
that referenced
this pull request
May 8, 2019
clintropolis
pushed a commit
that referenced
this pull request
May 10, 2019
* Fix exception when using complex aggs with result level caching * Add test comments * checkstyle * Add helper function for getting aggs from cache * Move method to CacheStrategy * Revert QueryToolChest changes * Update test comments
jihoonson
pushed a commit
to jihoonson/druid
that referenced
this pull request
May 10, 2019
…he#7614) * Fix exception when using complex aggs with result level caching * Add test comments * checkstyle * Add helper function for getting aggs from cache * Move method to CacheStrategy * Revert QueryToolChest changes * Update test comments
clintropolis
pushed a commit
that referenced
this pull request
May 11, 2019
… (#7634) * Fix exception when using complex aggs with result level caching * Add test comments * checkstyle * Add helper function for getting aggs from cache * Move method to CacheStrategy * Revert QueryToolChest changes * Update test comments
jihoonson
added a commit
to implydata/druid-public
that referenced
this pull request
Jun 4, 2019
…he#7614) (apache#7634) * Fix exception when using complex aggs with result level caching * Add test comments * checkstyle * Add helper function for getting aggs from cache * Move method to CacheStrategy * Revert QueryToolChest changes * Update test comments
jihoonson
pushed a commit
to implydata/druid-public
that referenced
this pull request
Jun 26, 2019
…he#7614) * Fix exception when using complex aggs with result level caching * Add test comments * checkstyle * Add helper function for getting aggs from cache * Move method to CacheStrategy * Revert QueryToolChest changes * Update test comments
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.
Fixes #6483
This fixes an issue in the GroupBy/TopN/Timeseries query tool chests where aggregator values from the result level cache (these are finalized values generated by
AggregatorFactory.finalizeComputation()) were being incorrectly passed toAggregatorFactory.deserialize()which operates on intermediate aggregator values.