Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The parent filter of the nested aggregator isn't resolved correctly all the time #9335

Conversation

martijnvg
Copy link
Member

The nested aggregator's parent filter isn't resolved properly in the case the nested agg gets created on the fly for buckets that are constructed during query execution.

The fix is the move the parent filter resolving from the nextReader(...) method to the collect(...) method, because only then any parent nested filter's parent filter is then properly instantiated.

Closes #9280

@martijnvg martijnvg changed the title aggs: The nested aggregator's parent filter is n't resolved properly i... aggs: The parent filter of the nested aggregator isn't resolved correctly all the time Jan 16, 2015
@jpountz
Copy link
Contributor

jpountz commented Jan 19, 2015

Can you elaborate a bit why it does not work correctly today?

@martijnvg
Copy link
Member Author

When aggregations aren't created before collection started (so not the top
level aggregations) then the resolving of the parentFilter in the
NestedAggregator#setNextReader(...) will resolve the incorrect parent.
(resolves the NonNestedDocsFilter.INSTANCE, because the parentFilter of all
the parent aggregations have not been resolved yet)

Moving the resolving of the parentFilter to the collect method fixes the
issue, because by then the parent filters of nested parent aggregators have
been resolved.

On 20 January 2015 at 00:22, Adrien Grand notifications@github.com wrote:

Can you elaborate a bit why it does not work correctly today?


Reply to this email directly or view it on GitHub
#9335 (comment)
.

Met vriendelijke groet,

Martijn van Groningen

@colings86
Copy link
Contributor

LGTM

@colings86 colings86 removed their assignment Jan 23, 2015
@jpountz
Copy link
Contributor

jpountz commented Jan 26, 2015

I see @martijnvg , thanks for the explanation. I think we should make the framework more resilient to this kind of corner cases in the long term, but +1 to your fix as this would be quite a hard task.

…y in the case the nested agg gets created on the fly for buckets that are constructed during query execution.

The fix is the move the parent filter resolving from the nextReader(...) method to the collect(...) method, because only then any parent nested filter's parent filter is then properly instantiated.

Closes elastic#9280
Closes elastic#9335
@martijnvg martijnvg force-pushed the bugs/nested_parent_filter_not_resolved_properly branch from 00bdced to 061337f Compare January 26, 2015 10:23
martijnvg added a commit to martijnvg/elasticsearch that referenced this pull request Jan 26, 2015
…y in the case the nested agg gets created on the fly for buckets that are constructed during query execution.

The fix is the move the parent filter resolving from the nextReader(...) method to the collect(...) method, because only then any parent nested filter's parent filter is then properly instantiated.

Closes elastic#9280
Closes elastic#9335
@martijnvg martijnvg closed this in f9c0e0d Jan 26, 2015
martijnvg added a commit that referenced this pull request Jan 26, 2015
…y in the case the nested agg gets created on the fly for buckets that are constructed during query execution.

The fix is the move the parent filter resolving from the nextReader(...) method to the collect(...) method, because only then any parent nested filter's parent filter is then properly instantiated.

Closes #9280
Closes #9335
martijnvg added a commit that referenced this pull request Jan 26, 2015
…y in the case the nested agg gets created on the fly for buckets that are constructed during query execution.

The fix is the move the parent filter resolving from the nextReader(...) method to the collect(...) method, because only then any parent nested filter's parent filter is then properly instantiated.

Closes #9280
Closes #9335
@martijnvg martijnvg removed the review label Jan 26, 2015
@clintongormley clintongormley changed the title aggs: The parent filter of the nested aggregator isn't resolved correctly all the time Aggs: The parent filter of the nested aggregator isn't resolved correctly all the time Feb 10, 2015
@martijnvg martijnvg deleted the bugs/nested_parent_filter_not_resolved_properly branch May 18, 2015 23:26
@clintongormley clintongormley changed the title Aggs: The parent filter of the nested aggregator isn't resolved correctly all the time The parent filter of the nested aggregator isn't resolved correctly all the time Jun 7, 2015
mute pushed a commit to mute/elasticsearch that referenced this pull request Jul 29, 2015
…y in the case the nested agg gets created on the fly for buckets that are constructed during query execution.

The fix is the move the parent filter resolving from the nextReader(...) method to the collect(...) method, because only then any parent nested filter's parent filter is then properly instantiated.

Closes elastic#9280
Closes elastic#9335
mute pushed a commit to mute/elasticsearch that referenced this pull request Jul 29, 2015
…y in the case the nested agg gets created on the fly for buckets that are constructed during query execution.

The fix is the move the parent filter resolving from the nextReader(...) method to the collect(...) method, because only then any parent nested filter's parent filter is then properly instantiated.

Closes elastic#9280
Closes elastic#9335
@clintongormley clintongormley added :Search/Search Search-related issues that do not fall into other categories and removed :Nested Docs labels Feb 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Search/Search Search-related issues that do not fall into other categories v1.3.8 v1.4.3 v1.5.0 v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants