Skip to content

Commit

Permalink
Use Set instead of LongSet in long script field (elastic#85417)
Browse files Browse the repository at this point in the history
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 elastic#84735
  • Loading branch information
rjernst authored Mar 30, 2022
1 parent 53fb1d8 commit 105cdeb
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -62,7 +61,7 @@ public boolean equals(Object obj) {
return terms.equals(other.terms);
}

LongSet terms() {
Set<Long> terms() {
return terms;
}
}
17 changes: 9 additions & 8 deletions server/src/test/java/org/elasticsearch/common/NumbersTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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
}
Expand All @@ -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));
Expand Down

0 comments on commit 105cdeb

Please sign in to comment.