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 support for named filters/queries inside nested filters. #6293

Merged

Conversation

Projects
None yet
3 participants
@martijnvg
Copy link
Member

martijnvg commented May 23, 2014

Named filters/queries inside nested queries/filters were being silently ignored.

@martijnvg martijnvg added bug labels May 23, 2014

@s1monw

View changes

src/main/java/org/elasticsearch/search/fetch/matchedqueries/MatchedQueriesFetchSubPhase.java Outdated
addMatchedQueries(hitContext, context.parsedPostFilter().namedFilters(), matchedQueries, docAndNestedDocsIdSet);
}
} catch (IOException e) {
// ignore

This comment has been minimized.

Copy link
@s1monw

s1monw Jun 2, 2014

Contributor

why don't we just bubble this exception up as an ElasticsearchException

This comment has been minimized.

Copy link
@martijnvg

martijnvg Jun 5, 2014

Author Member

We should, I'll change it.

DocIdSetIterator docAndNestedDocsIterator = docAndNestedDocsIdSet.iterator();
if (filterIterator != null && docAndNestedDocsIterator != null) {
int matchedDocId = -1;
for (int docId = docAndNestedDocsIterator.nextDoc(); docId < DocIdSetIterator.NO_MORE_DOCS; docId = docAndNestedDocsIterator.nextDoc()) {

This comment has been minimized.

Copy link
@s1monw

s1monw Jun 2, 2014

Contributor

can we also step out if matchedDocId < DocIdSetIterator.NO_MORE_DOC

This comment has been minimized.

Copy link
@martijnvg

martijnvg Jun 5, 2014

Author Member

Yes we can, but docId will reach DocIdSetIterator.NO_MORE_DOC sooner than matchedDocId, so it better to check for docId (docId comes from docAndNestedDocsIterator which only match docids of the current main docId and its nested docs in the current hit context of the fetch phase).

@@ -174,6 +173,75 @@ public void simpleNested() throws Exception {
}

@Test
public void simpleNestedMatchQueries() throws Exception {

This comment has been minimized.

Copy link
@s1monw

s1monw Jun 2, 2014

Contributor

can we use indexRandom for this and maybe add a test with more documents too?

This comment has been minimized.

Copy link
@martijnvg

martijnvg Jun 5, 2014

Author Member

yes, make sense

@s1monw

This comment has been minimized.

Copy link
Contributor

s1monw commented Jun 2, 2014

I left a bunch of comments

@s1monw s1monw added v2.0.0 and removed review labels Jun 2, 2014

@martijnvg

This comment has been minimized.

Copy link
Member Author

martijnvg commented Jun 5, 2014

@s1monw I updated the PR

@martijnvg martijnvg added the review label Jul 9, 2014

@s1monw

This comment has been minimized.

Copy link
Contributor

s1monw commented Jul 9, 2014

LGTM

@s1monw s1monw removed the review label Jul 9, 2014

@clintongormley

This comment has been minimized.

Copy link
Member

clintongormley commented Jul 11, 2014

@martijnvg can we get this merged in?

@s1monw s1monw added v1.4.0 and removed v1.3.0 labels Jul 14, 2014

@martijnvg

This comment has been minimized.

Copy link
Member Author

martijnvg commented Jul 16, 2014

@clintongormley yes, we can get this merged in. I will do it today.

@martijnvg martijnvg changed the title Fix support for named filters/queries inside nested filters. Search: Fix support for named filters/queries inside nested filters. Jul 16, 2014

martijnvg added a commit that referenced this pull request Jul 16, 2014

@martijnvg martijnvg merged commit 473d171 into elastic:master Jul 16, 2014

@martijnvg martijnvg added enhancement and removed bug labels Jul 16, 2014

@clintongormley clintongormley changed the title Search: Fix support for named filters/queries inside nested filters. Nested: Fix support for named filters/queries inside nested filters. Sep 8, 2014

@martijnvg martijnvg deleted the martijnvg:bugs/named-nested-filters-queries branch May 18, 2015

@clintongormley clintongormley changed the title Nested: Fix support for named filters/queries inside nested filters. Fix support for named filters/queries inside nested filters. Jun 7, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.