From 29982b3b517287425d0de6f6a464d24b1b3a379a Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Mon, 28 Mar 2022 14:21:49 -0700 Subject: [PATCH 1/2] Use Set instead of LongSet in long script field 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. relates #84735 --- .../java/org/elasticsearch/common/Numbers.java | 4 ++-- .../index/mapper/DateScriptFieldType.java | 7 +++---- .../index/mapper/LongScriptFieldType.java | 7 +++---- .../index/mapper/NumberFieldMapper.java | 2 +- .../runtime/LongScriptFieldTermsQuery.java | 9 ++++----- .../org/elasticsearch/common/NumbersTests.java | 17 +++++++++-------- .../runtime/LongScriptFieldTermsQueryTests.java | 14 +++++++------- 7 files changed, 29 insertions(+), 31 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/Numbers.java b/server/src/main/java/org/elasticsearch/common/Numbers.java index 2416b23167c4a..cb58ba277e65a 100644 --- a/server/src/main/java/org/elasticsearch/common/Numbers.java +++ b/server/src/main/java/org/elasticsearch/common/Numbers.java @@ -126,9 +126,9 @@ public static long toLongExact(Number n) { /** Return the long that {@code stringValue} stores or throws an exception if the * stored value cannot be converted to a long that stores the exact same * value and {@code coerce} is false. */ - public static long toLong(String stringValue, boolean coerce) { + public static Long toLong(String stringValue, boolean coerce) { try { - return Long.parseLong(stringValue); + return Long.valueOf(stringValue); } catch (NumberFormatException e) { // we will try again with BigDecimal } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DateScriptFieldType.java b/server/src/main/java/org/elasticsearch/index/mapper/DateScriptFieldType.java index 2f83db6dc55e5..c442d6c498b00 100644 --- 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 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 index dfbe79176ae4d..262fe1bcbe4ae 100644 --- 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 terms = new HashSet<>(values.size()); for (Object value : values) { if (NumberType.hasDecimalPart(value)) { continue; diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java index f73cd9b1ae5d7..816850e77cfe9 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -1098,7 +1098,7 @@ public static double objectToDouble(Object value) { * Converts an Object to a {@code long} by checking it against known * types and checking its range. */ - public static long objectToLong(Object value, boolean coerce) { + public static Long objectToLong(Object value, boolean coerce) { if (value instanceof Long) { return (Long) value; } diff --git a/server/src/main/java/org/elasticsearch/search/runtime/LongScriptFieldTermsQuery.java b/server/src/main/java/org/elasticsearch/search/runtime/LongScriptFieldTermsQuery.java index 59ed6591522a7..3819f4fbd95a3 100644 --- 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 terms; public LongScriptFieldTermsQuery( Script script, Function leafFactory, String fieldName, - LongSet terms + Set 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 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 index 6be3fc23ef2dc..9a4e3caf2a2ff 100644 --- 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 index fd8462f140568..e5d272488da3d 100644 --- 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 { @Override protected LongScriptFieldTermsQuery createTestInstance() { - LongSet terms = new LongHashSet(); + Set 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 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)); From 198d2ae0681870dfc31bd59166d63d6e8149f2b6 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 29 Mar 2022 11:00:10 -0700 Subject: [PATCH 2/2] change back to primitive --- server/src/main/java/org/elasticsearch/common/Numbers.java | 4 ++-- .../org/elasticsearch/index/mapper/NumberFieldMapper.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/Numbers.java b/server/src/main/java/org/elasticsearch/common/Numbers.java index cb58ba277e65a..2416b23167c4a 100644 --- a/server/src/main/java/org/elasticsearch/common/Numbers.java +++ b/server/src/main/java/org/elasticsearch/common/Numbers.java @@ -126,9 +126,9 @@ public static long toLongExact(Number n) { /** Return the long that {@code stringValue} stores or throws an exception if the * stored value cannot be converted to a long that stores the exact same * value and {@code coerce} is false. */ - public static Long toLong(String stringValue, boolean coerce) { + public static long toLong(String stringValue, boolean coerce) { try { - return Long.valueOf(stringValue); + return Long.parseLong(stringValue); } catch (NumberFormatException e) { // we will try again with BigDecimal } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java index 816850e77cfe9..f73cd9b1ae5d7 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -1098,7 +1098,7 @@ public static double objectToDouble(Object value) { * Converts an Object to a {@code long} by checking it against known * types and checking its range. */ - public static Long objectToLong(Object value, boolean coerce) { + public static long objectToLong(Object value, boolean coerce) { if (value instanceof Long) { return (Long) value; }