From b995abfa805189e7aa45e4d5a2afe088254155b4 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Thu, 13 Jun 2013 17:52:42 +0200 Subject: [PATCH] Call DISI#cost() ahead of time to prevent NPE NotDocIdSet resets the internal DocIdSetIterator to null causing NPE if cost is called. Closes #3177 --- .../common/lucene/docset/NotDocIdSet.java | 8 +++-- .../search/query/SimpleQueryTests.java | 32 +++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/elasticsearch/common/lucene/docset/NotDocIdSet.java b/src/main/java/org/elasticsearch/common/lucene/docset/NotDocIdSet.java index d4b81ca0bdc..ef260725a1b 100644 --- a/src/main/java/org/elasticsearch/common/lucene/docset/NotDocIdSet.java +++ b/src/main/java/org/elasticsearch/common/lucene/docset/NotDocIdSet.java @@ -113,11 +113,15 @@ public class NotDocIdSet extends DocIdSet { private DocIdSetIterator it1; private int lastReturn = -1; private int innerDocid = -1; + private final long cost; IteratorBasedIterator(int max, DocIdSetIterator it) throws IOException { this.max = max; this.it1 = it; - if ((innerDocid = it1.nextDoc()) == DocIdSetIterator.NO_MORE_DOCS) it1 = null; + this.cost = it1.cost(); + if ((innerDocid = it1.nextDoc()) == DocIdSetIterator.NO_MORE_DOCS) { + it1 = null; + } } @Override @@ -165,7 +169,7 @@ public class NotDocIdSet extends DocIdSet { @Override public long cost() { - return it1.cost(); + return cost; } } } diff --git a/src/test/java/org/elasticsearch/test/integration/search/query/SimpleQueryTests.java b/src/test/java/org/elasticsearch/test/integration/search/query/SimpleQueryTests.java index 2b41f3b09d9..13eec4906c7 100644 --- a/src/test/java/org/elasticsearch/test/integration/search/query/SimpleQueryTests.java +++ b/src/test/java/org/elasticsearch/test/integration/search/query/SimpleQueryTests.java @@ -56,6 +56,38 @@ public class SimpleQueryTests extends AbstractSharedClusterTest { public int numberOfNodes() { return 4; } + + @Test // see https://github.com/elasticsearch/elasticsearch/issues/3177 + public void testIssue3177() { + run(prepareCreate("test").setSettings(ImmutableSettings.settingsBuilder().put("index.number_of_shards", 1))); + client().prepareIndex("test", "type1", "1").setSource("field1", "value1").execute().actionGet(); + client().prepareIndex("test", "type1", "2").setSource("field1", "value2").execute().actionGet(); + client().prepareIndex("test", "type1", "3").setSource("field1", "value3").execute().actionGet(); + ensureGreen(); + waitForRelocation(); + optimize(); + refresh(); + assertThat( + client().prepareSearch() + .setQuery(matchAllQuery()) + .setFilter( + FilterBuilders.andFilter( + FilterBuilders.queryFilter(matchAllQuery()), + FilterBuilders.notFilter(FilterBuilders.andFilter(FilterBuilders.queryFilter(QueryBuilders.termQuery("field1", "value1")), + FilterBuilders.queryFilter(QueryBuilders.termQuery("field1", "value2")))))).execute().actionGet().getHits().getTotalHits(), + equalTo(3l)); + assertThat( + client().prepareSearch() + .setQuery( + QueryBuilders.filteredQuery( + QueryBuilders.boolQuery().should(QueryBuilders.termQuery("field1", "value1")).should(QueryBuilders.termQuery("field1", "value2")) + .should(QueryBuilders.termQuery("field1", "value3")), + FilterBuilders.notFilter(FilterBuilders.andFilter(FilterBuilders.queryFilter(QueryBuilders.termQuery("field1", "value1")), + FilterBuilders.queryFilter(QueryBuilders.termQuery("field1", "value2")))))).execute().actionGet().getHits().getTotalHits(), + equalTo(3l)); + assertThat(client().prepareSearch().setQuery(matchAllQuery()).setFilter(FilterBuilders.notFilter(FilterBuilders.termFilter("field1", "value3"))).execute().actionGet() + .getHits().getTotalHits(), equalTo(2l)); + } @Test public void passQueryAsStringTest() throws Exception {