Skip to content

Add support for earliest aggregatorMergeStrategy#14598

Merged
zachjsh merged 5 commits intoapache:masterfrom
abhishekrb19:earliest_agg_strategy
Jul 18, 2023
Merged

Add support for earliest aggregatorMergeStrategy#14598
zachjsh merged 5 commits intoapache:masterfrom
abhishekrb19:earliest_agg_strategy

Conversation

@abhishekrb19
Copy link
Copy Markdown
Contributor

This change:

  • Adds an earliest aggregator merge strategy similar to the latest merge strategy done in Add aggregatorMergeStrategy property in SegmentMetadata queries #14560.
  • Updates SegmentMetadataQueryQueryToolChestTest to include tests for earliest aggregator strategy, a test case for union datasource, and null analysis types.
  • Enables AGGREGATORS analysis type in SegmentMetadataQueryTest to get more test coverage.
Key changed/added classes in this PR
  • AggregatorMergeStrategy.java
  • SegmentMetadataQueryQueryToolChest.java
  • SegmentMetadataQueryQueryToolChestTest.java

Release note:

aggregatorMergeStrategy also supports an earliest merge strategy. Related to #14560.

This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • a release note entry in the PR description.
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • been tested in a test Druid cluster.

@abhishekrb19 abhishekrb19 force-pushed the earliest_agg_strategy branch from aa4e2e1 to 4ed0926 Compare July 17, 2023 21:07
Copy link
Copy Markdown
Contributor

@ektravel ektravel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes to the documentation look good.

@zachjsh zachjsh self-requested a review July 18, 2023 19:27
Copy link
Copy Markdown
Contributor

@zachjsh zachjsh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zachjsh zachjsh merged commit f4d0ea7 into apache:master Jul 18, 2023
@abhishekrb19 abhishekrb19 deleted the earliest_agg_strategy branch July 18, 2023 19:41
sergioferragut pushed a commit to sergioferragut/druid that referenced this pull request Jul 21, 2023
* Add EARLIEST aggregator merge strategy.

- More unit tests.
- Include the aggregators analysis type by default in tests.

* Docs.

* Some comments and a test

* Collapse into individual code blocks.
@LakshSingla LakshSingla added this to the 28.0 milestone Oct 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants