Browse Source

Fix potential AssertionError with include/exclude on terms aggregations. #19252

We call `LongBitSet.set(start, end)`, which fails when `start >= length`
(0 in that case).

Closes #18575
Adrien Grand 9 years ago
parent
commit
dd95dc7a0f

+ 1 - 1
core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/GlobalOrdinalsStringTermsAggregator.java

@@ -92,7 +92,7 @@ public class GlobalOrdinalsStringTermsAggregator extends AbstractStringTermsAggr
         globalOrds = valuesSource.globalOrdinalsValues(ctx);
 
         if (acceptedGlobalOrdinals == null && includeExclude != null) {
-            acceptedGlobalOrdinals = includeExclude.acceptedGlobalOrdinals(globalOrds, valuesSource);
+            acceptedGlobalOrdinals = includeExclude.acceptedGlobalOrdinals(globalOrds);
         }
 
         if (acceptedGlobalOrdinals != null) {

+ 4 - 7
core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/support/IncludeExclude.java

@@ -44,8 +44,6 @@ import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.search.DocValueFormat;
-import org.elasticsearch.search.aggregations.support.ValuesSource;
-import org.elasticsearch.search.aggregations.support.ValuesSource.Bytes.WithOrdinals;
 
 import java.io.IOException;
 import java.util.HashSet;
@@ -136,8 +134,7 @@ public class IncludeExclude implements Writeable, ToXContent {
     }
 
     public abstract static class OrdinalsFilter {
-        public abstract LongBitSet acceptedGlobalOrdinals(RandomAccessOrds globalOrdinals, ValuesSource.Bytes.WithOrdinals valueSource)
-                throws IOException;
+        public abstract LongBitSet acceptedGlobalOrdinals(RandomAccessOrds globalOrdinals) throws IOException;
 
     }
 
@@ -154,7 +151,7 @@ public class IncludeExclude implements Writeable, ToXContent {
          *
          */
         @Override
-        public LongBitSet acceptedGlobalOrdinals(RandomAccessOrds globalOrdinals, ValuesSource.Bytes.WithOrdinals valueSource)
+        public LongBitSet acceptedGlobalOrdinals(RandomAccessOrds globalOrdinals)
                 throws IOException {
             LongBitSet acceptedGlobalOrdinals = new LongBitSet(globalOrdinals.getValueCount());
             TermsEnum globalTermsEnum;
@@ -180,7 +177,7 @@ public class IncludeExclude implements Writeable, ToXContent {
         }
 
         @Override
-        public LongBitSet acceptedGlobalOrdinals(RandomAccessOrds globalOrdinals, WithOrdinals valueSource) throws IOException {
+        public LongBitSet acceptedGlobalOrdinals(RandomAccessOrds globalOrdinals) throws IOException {
             LongBitSet acceptedGlobalOrdinals = new LongBitSet(globalOrdinals.getValueCount());
             if (includeValues != null) {
                 for (BytesRef term : includeValues) {
@@ -189,7 +186,7 @@ public class IncludeExclude implements Writeable, ToXContent {
                         acceptedGlobalOrdinals.set(ord);
                     }
                 }
-            } else {
+            } else if (acceptedGlobalOrdinals.length() > 0) {
                 // default to all terms being acceptable
                 acceptedGlobalOrdinals.set(0, acceptedGlobalOrdinals.length());
             }

+ 128 - 0
core/src/test/java/org/elasticsearch/search/aggregations/support/IncludeExcludeTests.java

@@ -0,0 +1,128 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.search.aggregations.support;
+
+import org.apache.lucene.index.DocValues;
+import org.apache.lucene.index.RandomAccessOrds;
+import org.apache.lucene.index.SortedSetDocValues;
+import org.apache.lucene.util.BytesRef;
+import org.apache.lucene.util.LongBitSet;
+import org.elasticsearch.search.DocValueFormat;
+import org.elasticsearch.search.aggregations.bucket.terms.support.IncludeExclude;
+import org.elasticsearch.search.aggregations.bucket.terms.support.IncludeExclude.OrdinalsFilter;
+import org.elasticsearch.test.ESTestCase;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.TreeSet;
+
+public class IncludeExcludeTests extends ESTestCase {
+
+    public void testEmptyTermsWithOrds() throws IOException {
+        IncludeExclude inexcl = new IncludeExclude(
+                new TreeSet<>(Collections.singleton(new BytesRef("foo"))),
+                null);
+        OrdinalsFilter filter = inexcl.convertToOrdinalsFilter(DocValueFormat.RAW);
+        LongBitSet acceptedOrds = filter.acceptedGlobalOrdinals(DocValues.emptySortedSet());
+        assertEquals(0, acceptedOrds.length());
+
+        inexcl = new IncludeExclude(
+                null,
+                new TreeSet<>(Collections.singleton(new BytesRef("foo"))));
+        filter = inexcl.convertToOrdinalsFilter(DocValueFormat.RAW);
+        acceptedOrds = filter.acceptedGlobalOrdinals(DocValues.emptySortedSet());
+        assertEquals(0, acceptedOrds.length());
+    }
+
+    public void testSingleTermWithOrds() throws IOException {
+        RandomAccessOrds ords = new RandomAccessOrds() {
+
+            boolean consumed = true;
+
+            @Override
+            public void setDocument(int docID) {
+                consumed = false;
+            }
+
+            @Override
+            public long nextOrd() {
+                if (consumed) {
+                    return SortedSetDocValues.NO_MORE_ORDS;
+                } else {
+                    consumed = true;
+                    return 0;
+                }
+            }
+
+            @Override
+            public BytesRef lookupOrd(long ord) {
+                assertEquals(0, ord);
+                return new BytesRef("foo");
+            }
+
+            @Override
+            public long getValueCount() {
+                return 1;
+            }
+
+            @Override
+            public long ordAt(int index) {
+                return 0;
+            }
+
+            @Override
+            public int cardinality() {
+                return 1;
+            }
+        };
+        IncludeExclude inexcl = new IncludeExclude(
+                new TreeSet<>(Collections.singleton(new BytesRef("foo"))),
+                null);
+        OrdinalsFilter filter = inexcl.convertToOrdinalsFilter(DocValueFormat.RAW);
+        LongBitSet acceptedOrds = filter.acceptedGlobalOrdinals(ords);
+        assertEquals(1, acceptedOrds.length());
+        assertTrue(acceptedOrds.get(0));
+
+        inexcl = new IncludeExclude(
+                new TreeSet<>(Collections.singleton(new BytesRef("bar"))),
+                null);
+        filter = inexcl.convertToOrdinalsFilter(DocValueFormat.RAW);
+        acceptedOrds = filter.acceptedGlobalOrdinals(ords);
+        assertEquals(1, acceptedOrds.length());
+        assertFalse(acceptedOrds.get(0));
+
+        inexcl = new IncludeExclude(
+                new TreeSet<>(Collections.singleton(new BytesRef("foo"))),
+                new TreeSet<>(Collections.singleton(new BytesRef("foo"))));
+        filter = inexcl.convertToOrdinalsFilter(DocValueFormat.RAW);
+        acceptedOrds = filter.acceptedGlobalOrdinals(ords);
+        assertEquals(1, acceptedOrds.length());
+        assertFalse(acceptedOrds.get(0));
+
+        inexcl = new IncludeExclude(
+                null, // means everything included
+                new TreeSet<>(Collections.singleton(new BytesRef("foo"))));
+        filter = inexcl.convertToOrdinalsFilter(DocValueFormat.RAW);
+        acceptedOrds = filter.acceptedGlobalOrdinals(ords);
+        assertEquals(1, acceptedOrds.length());
+        assertFalse(acceptedOrds.get(0));
+    }
+
+}