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

Fix 2 bugs in children agg #10263

Conversation

martijnvg
Copy link
Member

PR for #10158

  1. Multiple nested children aggs result in a NPE. This only occurs in 1.x and not in master, since post collection works there in the correct order.
  2. Fixed a counting bug where the same readers where post collected twice. Possible fixes multiple 'children' aggregations interfere with each other #9958

Note this PR is against 1.x b/c bug 1 is fixed in master already. This bug fix was part of a refactoring that happened as part of #9544

@@ -66,7 +66,8 @@
private final LongObjectPagedHashMap<long[]> parentOrdToOtherBuckets;
private boolean multipleBucketsPerParentOrd = false;

private List<AtomicReaderContext> replay = new ArrayList<>();
// This needs to be a Set to avoid duplicate reader context entries (#setNextReader(...) can get invoked multiple times with the same reader context)
private Set<AtomicReaderContext> replay = new HashSet<>();
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should be a LinkedHashSet since we rely on doc ID order for tie-breaking.

Copy link
Member Author

Choose a reason for hiding this comment

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

good point, I'll change that

@jpountz
Copy link
Contributor

jpountz commented Mar 26, 2015

LGTM

@martijnvg martijnvg removed the review label Mar 28, 2015
@martijnvg martijnvg force-pushed the children_agg/broken_nested_usage_and_wrong_doc_counts branch from 56fefc8 to eb8eb74 Compare March 28, 2015 07:14
This also fixes a NPE when the nested part has been filtered out of the _source, because of _source filtering.

Closes elastic#9766
@martijnvg martijnvg force-pushed the children_agg/broken_nested_usage_and_wrong_doc_counts branch from eb8eb74 to fd61eda Compare March 28, 2015 07:35
1) multiple nested children aggs result in a NPE
2) fixed a counting bug where the same readers where post collected twice

Closes elastic#10158
@martijnvg martijnvg force-pushed the children_agg/broken_nested_usage_and_wrong_doc_counts branch from fd61eda to 29058c7 Compare March 28, 2015 07:47
@martijnvg
Copy link
Member Author

pushed to master, 1.x and 1.5

@martijnvg martijnvg closed this Mar 28, 2015
@martijnvg martijnvg deleted the children_agg/broken_nested_usage_and_wrong_doc_counts branch May 18, 2015 23:26
@clintongormley clintongormley changed the title children aggregation: Fix 2 bugs in children agg Fix 2 bugs in children agg Jun 8, 2015
@clintongormley clintongormley added :Search/Search Search-related issues that do not fall into other categories and removed :Parent/Child 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.5.1 v1.6.0 v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants