Use Set instead of LongSet in long script field (#85417)

The long script field uses hppc's LongSet to store the unique values to be queried. However, this set should be relatively small. This commit changes the internals of the runtime long script field to use Set<Long>. relates #84735
elastic · Mar 30, 2022 · 105cdeb · 105cdeb
1 parent 53fb1d8
commit 105cdeb
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 28 deletions.
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DateScriptFieldType.java b/server/src/main/java/org/elasticsearch/index/mapper/DateScriptFieldType.java
@@ -8,9 +8,6 @@
 
 package org.elasticsearch.index.mapper;
 
-import com.carrotsearch.hppc.LongHashSet;
-import com.carrotsearch.hppc.LongSet;
-
 import org.apache.lucene.search.Query;
 import org.elasticsearch.common.lucene.search.Queries;
 import org.elasticsearch.common.time.DateFormatter;
@@ -38,9 +35,11 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Set;
 import java.util.function.Function;
 import java.util.function.Supplier;
 
@@ -233,7 +232,7 @@ public Query termsQuery(Collection<?> values, SearchExecutionContext context) {
             return Queries.newMatchAllQuery();
         }
         return DateFieldType.handleNow(context, now -> {
-            LongSet terms = new LongHashSet(values.size());
+            Set<Long> terms = new HashSet<>(values.size());
             for (Object value : values) {
                 terms.add(DateFieldType.parseToLong(value, false, null, this.dateMathParser, now, DateFieldMapper.Resolution.MILLISECONDS));
             }

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/LongScriptFieldType.java b/server/src/main/java/org/elasticsearch/index/mapper/LongScriptFieldType.java
@@ -8,9 +8,6 @@
 
 package org.elasticsearch.index.mapper;
 
-import com.carrotsearch.hppc.LongHashSet;
-import com.carrotsearch.hppc.LongSet;
-
 import org.apache.lucene.search.Query;
 import org.elasticsearch.common.lucene.search.Queries;
 import org.elasticsearch.common.time.DateMathParser;
@@ -30,7 +27,9 @@
 
 import java.time.ZoneId;
 import java.util.Collection;
+import java.util.HashSet;
 import java.util.Map;
+import java.util.Set;
 import java.util.function.Function;
 import java.util.function.Supplier;
 
@@ -137,7 +136,7 @@ public Query termsQuery(Collection<?> values, SearchExecutionContext context) {
         if (values.isEmpty()) {
             return Queries.newMatchAllQuery();
         }
-        LongSet terms = new LongHashSet(values.size());
+        Set<Long> terms = new HashSet<>(values.size());
         for (Object value : values) {
             if (NumberType.hasDecimalPart(value)) {
                 continue;

diff --git a/server/src/main/java/org/elasticsearch/search/runtime/LongScriptFieldTermsQuery.java b/server/src/main/java/org/elasticsearch/search/runtime/LongScriptFieldTermsQuery.java
@@ -8,23 +8,22 @@
 
 package org.elasticsearch.search.runtime;
 
-import com.carrotsearch.hppc.LongSet;
-
 import org.apache.lucene.index.LeafReaderContext;
 import org.elasticsearch.script.AbstractLongFieldScript;
 import org.elasticsearch.script.Script;
 
 import java.util.Objects;
+import java.util.Set;
 import java.util.function.Function;
 
 public class LongScriptFieldTermsQuery extends AbstractLongScriptFieldQuery {
-    private final LongSet terms;
+    private final Set<Long> terms;
 
     public LongScriptFieldTermsQuery(
         Script script,
         Function<LeafReaderContext, AbstractLongFieldScript> leafFactory,
         String fieldName,
-        LongSet terms
+        Set<Long> terms
     ) {
         super(script, leafFactory, fieldName);
         this.terms = terms;
@@ -62,7 +61,7 @@ public boolean equals(Object obj) {
         return terms.equals(other.terms);
     }
 
-    LongSet terms() {
+    Set<Long> terms() {
         return terms;
     }
 }
diff --git a/server/src/test/java/org/elasticsearch/common/NumbersTests.java b/server/src/test/java/org/elasticsearch/common/NumbersTests.java
@@ -16,20 +16,21 @@
 import java.math.BigInteger;
 import java.util.concurrent.atomic.AtomicInteger;
 
+import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
 
 public class NumbersTests extends ESTestCase {
 
     @Timeout(millis = 10000)
     public void testToLong() {
-        assertEquals(3L, Numbers.toLong("3", false));
-        assertEquals(3L, Numbers.toLong("3.1", true));
-        assertEquals(9223372036854775807L, Numbers.toLong("9223372036854775807.00", false));
-        assertEquals(-9223372036854775808L, Numbers.toLong("-9223372036854775808.00", false));
-        assertEquals(9223372036854775807L, Numbers.toLong("9223372036854775807.00", true));
-        assertEquals(-9223372036854775808L, Numbers.toLong("-9223372036854775808.00", true));
-        assertEquals(9223372036854775807L, Numbers.toLong("9223372036854775807.99", true));
-        assertEquals(-9223372036854775808L, Numbers.toLong("-9223372036854775808.99", true));
+        assertThat(Numbers.toLong("3", false), equalTo(3L));
+        assertThat(Numbers.toLong("3.1", true), equalTo(3L));
+        assertThat(Numbers.toLong("9223372036854775807.00", false), equalTo(9223372036854775807L));
+        assertThat(Numbers.toLong("-9223372036854775808.00", false), equalTo(-9223372036854775808L));
+        assertThat(Numbers.toLong("9223372036854775807.00", true), equalTo(9223372036854775807L));
+        assertThat(Numbers.toLong("-9223372036854775808.00", true), equalTo(-9223372036854775808L));
+        assertThat(Numbers.toLong("9223372036854775807.99", true), equalTo(9223372036854775807L));
+        assertThat(Numbers.toLong("-9223372036854775808.99", true), equalTo(-9223372036854775808L));
 
         assertEquals(
             "Value [9223372036854775808] is out of range for a long",

diff --git a/server/src/test/java/org/elasticsearch/search/runtime/LongScriptFieldTermsQueryTests.java b/server/src/test/java/org/elasticsearch/search/runtime/LongScriptFieldTermsQueryTests.java
@@ -8,17 +8,17 @@
 
 package org.elasticsearch.search.runtime;
 
-import com.carrotsearch.hppc.LongHashSet;
-import com.carrotsearch.hppc.LongSet;
-
 import org.elasticsearch.script.Script;
 
+import java.util.HashSet;
+import java.util.Set;
+
 import static org.hamcrest.Matchers.equalTo;
 
 public class LongScriptFieldTermsQueryTests extends AbstractLongScriptFieldQueryTestCase<LongScriptFieldTermsQuery> {
     @Override
     protected LongScriptFieldTermsQuery createTestInstance() {
-        LongSet terms = new LongHashSet();
+        Set<Long> terms = new HashSet<>();
         int count = between(1, 100);
         while (terms.size() < count) {
             terms.add(randomLong());
@@ -35,12 +35,12 @@ protected LongScriptFieldTermsQuery copy(LongScriptFieldTermsQuery orig) {
     protected LongScriptFieldTermsQuery mutate(LongScriptFieldTermsQuery orig) {
         Script script = orig.script();
         String fieldName = orig.fieldName();
-        LongSet terms = orig.terms();
+        Set<Long> terms = orig.terms();
         switch (randomInt(2)) {
             case 0 -> script = randomValueOtherThan(script, this::randomScript);
             case 1 -> fieldName += "modified";
             case 2 -> {
-                terms = new LongHashSet(terms);
+                terms = new HashSet<>(terms);
                 while (false == terms.add(randomLong())) {
                     // Random long was already in the set
                 }
@@ -52,7 +52,7 @@ protected LongScriptFieldTermsQuery mutate(LongScriptFieldTermsQuery orig) {
 
     @Override
     public void testMatches() {
-        LongScriptFieldTermsQuery query = new LongScriptFieldTermsQuery(randomScript(), leafFactory, "test", LongHashSet.from(1, 2, 3));
+        LongScriptFieldTermsQuery query = new LongScriptFieldTermsQuery(randomScript(), leafFactory, "test", Set.of(1L, 2L, 3L));
         assertTrue(query.matches(new long[] { 1 }, 1));
         assertTrue(query.matches(new long[] { 2 }, 1));
         assertTrue(query.matches(new long[] { 3 }, 1));