Allows for a composite aggregation to be a child of a filter aggregation

[juliovalcarcel]

A composite aggregation should be allowed to be a grandchild of a nested aggregation only if it's parent is a filter aggregation.

A nested aggregation has no way to filter a list of nested documents like you do when the composite is the top level aggregation.

Relates to #37178

[juliovalcarcel]

CC: @jimczi

[elasticmachine]

Pinging @elastic/es-analytics-geo

[juliovalcarcel]

Hey @polyfractal / @jimczi the cla is signed now, but it doesn't appear to be updated. Is there something else I have to do?

[polyfractal]

Heya @juliovalcarcel, just chatted with our infra. Looks like you signed right when the clachecker was down for a change, so the signature slipped into the aether.

Would you mind signing again? Sorry for the hassle!

[juliovalcarcel]

@polyfractal resigned

[polyfractal]

Awesome, thanks @juliocamarero. Change looks fine to me, but I only have some experience with composite agg so we should wait for @jimczi too.

Could you add a test to verify this works as expected?

[juliocamarero]

wrong julio? @juliovalcarcel :D

[polyfractal]

Argh, sorry about that. 🤦‍♂️

[juliovalcarcel]

@polyfractal I am not all that familiar with the test framework and didn't see an existing test for the composite as a child of a nested. If you or @jimczi could help provide me some guidance on how to set up the test within the elastic test framework I would be more than happy too!

[javanna]

heya @juliovalcarcel sorry for keeping you on hold. I think it would be great if you could add a unit test to CompositeAggregationBuilderTests which verifies the updated behaviour. If I understand correctly the updated is called when the builder.build method is called, that should be a good start. Let me know if you have any further question.

[akanthos]

Are there any news on this ticket? I was wondering if it's going to be merged soon, as filtering before using composite aggregations seems to be a very nice thing to have.
Are there any plans to merge it? Maybe some expected version that would support that?
@juliovalcarcel, @javanna

Thanks!

[jimczi]

Are there any news on this ticket? I was wondering if it's going to be merged soon, as filtering before using composite aggregations seems to be a very nice thing to have.

The logic in the pr looks good but we need a test. Apologize for missing the ping but a rest test similar to 230_composite should be added to validate the logic.

[noah79]

Is this ever going to be merged?

[not-napoleon]

I was working on merging this, which requires writing tests for it, and I think I found an issue with the PR as it stands. I think the current implementation allows multiple levels of nested aggs before the composite, but the proposed change would only allow a single level. Should be fixable, but I don't expect to get to it soon.