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.

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

FunctionScore: RandomScoreFunction now accepts long, as well a strings. #8311

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -80,6 +80,14 @@ public static FactorBuilder factorFunction(float boost) {
public static RandomScoreFunctionBuilder randomFunction(int seed) {
return (new RandomScoreFunctionBuilder()).seed(seed);
}

public static RandomScoreFunctionBuilder randomFunction(long seed) {
return (new RandomScoreFunctionBuilder()).seed(seed);
}

public static RandomScoreFunctionBuilder randomFunction(String seed) {
return (new RandomScoreFunctionBuilder()).seed(seed);
}

public static WeightBuilder weightFactorFunction(float weight) {
return (WeightBuilder)(new WeightBuilder().setWeight(weight));
Expand Down
Expand Up @@ -28,7 +28,7 @@
*/
public class RandomScoreFunctionBuilder extends ScoreFunctionBuilder {

private Integer seed = null;
private Object seed = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we can just make this Number and we try to parse the string?

Copy link
Contributor

Choose a reason for hiding this comment

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

nevermind I saw the usage


public RandomScoreFunctionBuilder() {
}
Expand All @@ -49,11 +49,33 @@ public RandomScoreFunctionBuilder seed(int seed) {
return this;
}

/**
* seed variant taking a long value.
* @see {@link #seed(int)}
*/
public RandomScoreFunctionBuilder seed(long seed) {
this.seed = seed;
return this;
}

/**
* seed variant taking a long value.
Copy link
Contributor

Choose a reason for hiding this comment

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

this javadoc is wrong

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

* @see {@link #seed(int)}
*/
public RandomScoreFunctionBuilder seed(String seed) {
this.seed = seed;
return this;
}

@Override
public void doXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(getName());
if (seed != null) {
builder.field("seed", seed.intValue());
if (seed instanceof Integer) {
builder.field("seed", ((Integer)seed).intValue());
} else if (seed instanceof Long) {
builder.field("seed", ((Long)seed).longValue());
} else if (seed instanceof String) {
builder.field("seed", (String)seed);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you should just do instanceof Number and else call .toString()

Copy link
Member Author

Choose a reason for hiding this comment

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

I was trying to satisfy the possibility for a session id (say alphanum + dashes).

Copy link
Contributor

Choose a reason for hiding this comment

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

I modified the comment after you replied ^^

Copy link
Contributor

Choose a reason for hiding this comment

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

@rjernst can you fix this ^^

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, sorry, I misunderstood. Fixed.

}
builder.endObject();
}
Expand Down
Expand Up @@ -20,6 +20,7 @@

package org.elasticsearch.index.query.functionscore.random;

import com.google.common.primitives.Longs;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.lucene.search.function.RandomScoreFunction;
import org.elasticsearch.common.lucene.search.function.ScoreFunction;
Expand Down Expand Up @@ -59,7 +60,19 @@ public ScoreFunction parse(QueryParseContext parseContext, XContentParser parser
currentFieldName = parser.currentName();
} else if (token.isValue()) {
if ("seed".equals(currentFieldName)) {
seed = parser.intValue();
if (token == XContentParser.Token.VALUE_NUMBER) {
if (parser.numberType() == XContentParser.NumberType.INT) {
seed = parser.intValue();
} else if (parser.numberType() == XContentParser.NumberType.LONG) {
seed = Longs.hashCode(parser.longValue());
} else {
throw new QueryParsingException(parseContext.index(), "random_score seed must be an int, long or string, not '" + token.toString() + "'");
}
} else if (token == XContentParser.Token.VALUE_STRING) {
seed = parser.text().hashCode();
} else {
throw new QueryParsingException(parseContext.index(), "random_score seed must be an int/long or string, not '" + token.toString() + "'");
}
} else {
throw new QueryParsingException(parseContext.index(), NAMES[0] + " query does not support [" + currentFieldName + "]");
}
Expand All @@ -73,7 +86,7 @@ public ScoreFunction parse(QueryParseContext parseContext, XContentParser parser
}

if (seed == -1) {
seed = (int)parseContext.nowInMillis();
seed = Longs.hashCode(parseContext.nowInMillis());
}
final ShardId shardId = SearchContext.current().indexShard().shardId();
final int salt = (shardId.index().name().hashCode() << 10) | shardId.id();
Expand Down
Expand Up @@ -25,7 +25,6 @@
import org.elasticsearch.test.ElasticsearchIntegrationTest;
import org.hamcrest.CoreMatchers;
import org.junit.Ignore;
import org.junit.Test;

import java.util.Arrays;
import java.util.Comparator;
Expand All @@ -41,7 +40,6 @@

public class RandomScoreFunctionTests extends ElasticsearchIntegrationTest {

@Test
public void testConsistentHitsWithSameSeed() throws Exception {
createIndex("test");
ensureGreen(); // make sure we are done otherwise preference could change?
Expand Down Expand Up @@ -103,7 +101,6 @@ public int compare(SearchHit o1, SearchHit o2) {
}
}

@Test
public void testScoreAccessWithinScript() throws Exception {
assertAcked(prepareCreate("test")
.addMapping("type", "body", "type=string", "index", "type=" + randomFrom(new String[]{"short", "float", "long", "integer", "double"})));
Expand Down Expand Up @@ -170,7 +167,6 @@ public void testScoreAccessWithinScript() throws Exception {
assertThat(firstHit.getScore(), greaterThan(1f));
}

@Test
public void testSeedReportedInExplain() throws Exception {
createIndex("test");
ensureGreen();
Expand Down Expand Up @@ -201,7 +197,6 @@ public void testNoDocs() throws Exception {
assertEquals(0, resp.getHits().totalHits());
}

@Test
public void testScoreRange() throws Exception {
// all random scores should be in range [0.0, 1.0]
createIndex("test");
Expand All @@ -227,8 +222,32 @@ public void testScoreRange() throws Exception {
}
}
}

public void testSeeds() throws Exception {
createIndex("test");
ensureGreen();
final int docCount = randomIntBetween(100, 200);
for (int i = 0; i < docCount; i++) {
index("test", "type", "" + i, jsonBuilder().startObject().endObject());
}
flushAndRefresh();

assertNoFailures(client().prepareSearch()
.setSize(docCount) // get all docs otherwise we are prone to tie-breaking
.setQuery(functionScoreQuery(matchAllQuery(), randomFunction(randomInt())))
.execute().actionGet());

assertNoFailures(client().prepareSearch()
.setSize(docCount) // get all docs otherwise we are prone to tie-breaking
.setQuery(functionScoreQuery(matchAllQuery(), randomFunction(randomLong())))
.execute().actionGet());

assertNoFailures(client().prepareSearch()
.setSize(docCount) // get all docs otherwise we are prone to tie-breaking
.setQuery(functionScoreQuery(matchAllQuery(), randomFunction(randomRealisticUnicodeOfLengthBetween(10, 20))))
.execute().actionGet());
}

@Test
@Ignore
public void checkDistribution() throws Exception {
int count = 10000;
Expand Down