Support Nested Aggregations as part of Star-Tree

[Shailesh-Kumar-Singh]

Description

Support Nested Aggregations as part of Star-Tree

Related Issues

Resolves #17274

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

❌ Gradle check result for 16f18d9: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[bharath-techie]

lets add some comments please on why its done this way

[bharath-techie]

This block of code can be extracted out and get reused ?

[github-actions]

❌ Gradle check result for b3bee6c: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[sandeshkr419]

StarTreePreComputeCollector is only involved for aggregations not including Metric aggregators. You can very well define the method here, instead of having the same implementation everywhere. (overlooking the overrides - the implementations look same.

[sandeshkr419]

I had fixed redundant initialization of globalOrds as part of this change #18059 (pending merge)

Make sure that the changes don't break.

[sandeshkr419]

Return null early in the method itself when parent != null

Basically we don't want to run the follwoing steps in case parent is not null - since the parent will be executing all this:

StarTreeValues starTreeValues = StarTreeQueryHelper.getStarTreeValues(ctx, starTree);
SortedNumericStarTreeValuesIterator valuesIterator = (SortedNumericStarTreeValuesIterator) starTreeValues
            .getDimensionValuesIterator(fieldName);
SortedNumericStarTreeValuesIterator docCountsIterator = StarTreeQueryHelper.getDocCountsIterator(starTreeValues, starTree);

[Shailesh-Kumar-Singh]

Return null early in the method itself when parent != null

This would not work I believe, if parent != null, we would still need to return an object of StarTreeBucketCollector as we have to call collectStarTreeEntry() for the subcollectors.

for (StarTreeBucketCollector subCollector : collector.getSubCollectors()) {
           subCollector.collectStarTreeEntry(entryBit, bucketOrd);
}

[sandeshkr419]

Ohh I missed that. No worries.

[sandeshkr419]

nit: Just to maintain boolean check consistency across project: isValid == false

[Shailesh-Kumar-Singh]

sure.

[sandeshkr419]

Thanks @Shailesh-Kumar-Singh for working on this.

Code wise, I only have a few refactoring comments on the implementation.

I have some concerns regarding the supported cases:

What is the break-point of nested aggregators to use or not use star-tree, like upto how many nested levels?
So for too many nested levels, I'm concerned that the depth_first mode might be utilized (I'm not sure, you need to verify it) - so let's add depth first test cases if required.
In my PRs earlier, I had skipped using star-tree for depth-first cases since they were out of scope of the query shapes which were supported then.
For high cardinality keyword terms or timestamp fields (date-histograms) with nested setup, it might be beneficial to run some benchmarks if using star-tree is beneficial or not - else let's think of some gate-keeping based on the number of nested levels. Basically we don't want the star-tree to run for non-performant cases.

[github-actions]

❌ Gradle check result for a58027f: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?