Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Set instead of LongSet in double script field #85475

Merged
merged 6 commits into from
Apr 1, 2022
Merged

Use Set instead of LongSet in double script field #85475

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7d65c47
Use Set instead of LongSet in double script field
rjernst Mar 30, 2022
bb3d484
address feedback
rjernst Mar 31, 2022
156a6c8
Merge branch 'master' into hppc/double_scriptfield
rjernst Mar 31, 2022
cd59eb1
Merge branch 'master' into hppc/double_scriptfield
rjernst Mar 31, 2022
cb5cc0c
Merge branch 'master' into hppc/double_scriptfield
rjernst Mar 31, 2022
0ccb9fc
Merge branch 'master' into hppc/double_scriptfield
rjernst Mar 31, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions server/src/main/java/org/elasticsearch/index/mapper/DoubleScriptFieldType.java
View file
Open in desktop
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 @@ -139,7 +138,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) {
terms.add(Double.doubleToLongBits(NumberType.objectToDouble(value)));
}
Expand Down
12 changes: 5 additions & 7 deletions server/src/main/java/org/elasticsearch/search/runtime/DoubleScriptFieldTermsQuery.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,21 @@

package org.elasticsearch.search.runtime;

import com.carrotsearch.hppc.LongSet;
import com.carrotsearch.hppc.cursors.LongCursor;

import org.elasticsearch.script.DoubleFieldScript;
import org.elasticsearch.script.Script;

import java.util.Arrays;
import java.util.Objects;
import java.util.Set;

public class DoubleScriptFieldTermsQuery extends AbstractDoubleScriptFieldQuery {
private final LongSet terms;
private final Set<Long> terms;

/**
* Build the query.
* @param terms The terms converted to a long with {@link Double#doubleToLongBits(double)}.
*/
public DoubleScriptFieldTermsQuery(Script script, DoubleFieldScript.LeafFactory leafFactory, String fieldName, LongSet terms) {
public DoubleScriptFieldTermsQuery(Script script, DoubleFieldScript.LeafFactory leafFactory, String fieldName, Set<Long> terms) {
super(script, leafFactory, fieldName);
this.terms = terms;
}
Expand Down Expand Up @@ -66,8 +64,8 @@ public boolean equals(Object obj) {
double[] terms() {
double[] result = new double[terms.size()];
int i = 0;
for (LongCursor lc : terms) {
result[i++] = Double.longBitsToDouble(lc.value);
for (long l : terms) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe replace this with stream/mapToDouble.

result[i++] = Double.longBitsToDouble(l);
}
return result;
}
Expand Down
15 changes: 7 additions & 8 deletions server/src/test/java/org/elasticsearch/search/runtime/DoubleScriptFieldTermsQueryTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,12 @@

package org.elasticsearch.search.runtime;

import com.carrotsearch.hppc.LongHashSet;
import com.carrotsearch.hppc.LongSet;

import org.elasticsearch.script.Script;

import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

import static java.util.stream.Collectors.toList;
import static org.hamcrest.Matchers.endsWith;
Expand All @@ -25,7 +24,7 @@
public class DoubleScriptFieldTermsQueryTests extends AbstractDoubleScriptFieldQueryTestCase<DoubleScriptFieldTermsQuery> {
@Override
protected DoubleScriptFieldTermsQuery createTestInstance() {
LongSet terms = new LongHashSet();
Set<Long> terms = new HashSet<>();
int count = between(1, 100);
while (terms.size() < count) {
terms.add(Double.doubleToLongBits(randomDouble()));
Expand All @@ -35,7 +34,7 @@ protected DoubleScriptFieldTermsQuery createTestInstance() {

@Override
protected DoubleScriptFieldTermsQuery copy(DoubleScriptFieldTermsQuery orig) {
LongSet terms = new LongHashSet();
Set<Long> terms = new HashSet<>();
for (double term : orig.terms()) {
terms.add(Double.doubleToLongBits(term));
}
Expand All @@ -46,15 +45,15 @@ protected DoubleScriptFieldTermsQuery copy(DoubleScriptFieldTermsQuery orig) {
protected DoubleScriptFieldTermsQuery mutate(DoubleScriptFieldTermsQuery orig) {
Script script = orig.script();
String fieldName = orig.fieldName();
LongSet terms = new LongHashSet();
Set<Long> terms = new HashSet<>();
for (double term : orig.terms()) {
terms.add(Double.doubleToLongBits(term));
}
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(Double.doubleToLongBits(randomDouble()))) {
// Random double was already in the set
}
Expand All @@ -70,7 +69,7 @@ public void testMatches() {
randomScript(),
leafFactory,
"test",
LongHashSet.from(Double.doubleToLongBits(0.1), Double.doubleToLongBits(0.2), Double.doubleToLongBits(7.5))
Set.of(Double.doubleToLongBits(0.1), Double.doubleToLongBits(0.2), Double.doubleToLongBits(7.5))
);
assertTrue(query.matches(new double[] { 0.1 }, 1));
assertTrue(query.matches(new double[] { 0.2 }, 1));
Expand Down