From 443e84be38f9b53ff1261f9263fcc2f87fa268d9 Mon Sep 17 00:00:00 2001 From: Kevin Risden Date: Fri, 9 Dec 2022 16:10:43 -0500 Subject: [PATCH 01/14] SOLR-5707: Lucene Expressions in Solr --- .../transform/ValueSourceAugmenter.java | 36 ++ .../search/ExpressionValueSourceParser.java | 242 +++++++++ .../conf/solrconfig-expressions-vs.xml | 52 ++ .../ExpressionValueSourceParserTest.java | 462 ++++++++++++++++++ 4 files changed, 792 insertions(+) create mode 100644 solr/core/src/java/org/apache/solr/search/ExpressionValueSourceParser.java create mode 100644 solr/core/src/test-files/solr/collection1/conf/solrconfig-expressions-vs.xml create mode 100644 solr/core/src/test/org/apache/solr/search/ExpressionValueSourceParserTest.java diff --git a/solr/core/src/java/org/apache/solr/response/transform/ValueSourceAugmenter.java b/solr/core/src/java/org/apache/solr/response/transform/ValueSourceAugmenter.java index 993af34770a..7044d341ef8 100644 --- a/solr/core/src/java/org/apache/solr/response/transform/ValueSourceAugmenter.java +++ b/solr/core/src/java/org/apache/solr/response/transform/ValueSourceAugmenter.java @@ -23,6 +23,7 @@ import org.apache.lucene.index.ReaderUtil; import org.apache.lucene.queries.function.FunctionValues; import org.apache.lucene.queries.function.ValueSource; +import org.apache.lucene.search.Scorable; import org.apache.solr.common.SolrDocument; import org.apache.solr.common.SolrException; import org.apache.solr.response.ResultContext; @@ -69,6 +70,15 @@ public void setContext(ResultContext context) { SolrIndexSearcher searcher; List readerContexts; + @Override + public void transform(SolrDocument doc, int docid, float score) throws IOException { + if (context != null && context.wantsScores()) { + fcontext.put("scorer", new ScoreAndDoc(docid, score)); + } + + transform(doc, docid); + } + @Override public void transform(SolrDocument doc, int docid) { // This is only good for random-access functions @@ -100,4 +110,30 @@ protected void setValue(SolrDocument doc, Object val) { doc.setField(name, val); } } + + /** + * Fake scorer for a single document + * + *

TODO: when SOLR-5595 is fixed, this wont be needed, as we dont need to recompute sort values + * here from the comparator + */ + protected static class ScoreAndDoc extends Scorable { + final int docid; + final float score; + + ScoreAndDoc(int docid, float score) { + this.docid = docid; + this.score = score; + } + + @Override + public int docID() { + return docid; + } + + @Override + public float score() throws IOException { + return score; + } + } } diff --git a/solr/core/src/java/org/apache/solr/search/ExpressionValueSourceParser.java b/solr/core/src/java/org/apache/solr/search/ExpressionValueSourceParser.java new file mode 100644 index 00000000000..a72e9f6380e --- /dev/null +++ b/solr/core/src/java/org/apache/solr/search/ExpressionValueSourceParser.java @@ -0,0 +1,242 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.search; + +import static org.apache.solr.common.SolrException.ErrorCode.SERVER_ERROR; + +import java.text.ParseException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import org.apache.lucene.expressions.Bindings; +import org.apache.lucene.expressions.Expression; +import org.apache.lucene.expressions.js.JavascriptCompiler; +import org.apache.lucene.queries.function.ValueSource; +import org.apache.lucene.search.DoubleValuesSource; +import org.apache.solr.common.SolrException; +import org.apache.solr.common.util.NamedList; +import org.apache.solr.schema.IndexSchema; +import org.apache.solr.schema.SchemaField; + +/** + * A ValueSource parser that can be configured with (named) pre-complied Expressions that can then + * be evaluated at request time. :nocommit: more docs, example configs, example usage :nocommit: + * need "bad-solrconfig..." level test of cycle expressions, using score w/null score binding, + * etc..." + */ +public class ExpressionValueSourceParser extends ValueSourceParser { + public static final String SCORE_KEY = "score-name"; + public static final String EXPRESSIONS_KEY = "expressions"; + + private Map expressions; + private String scoreKey = SolrReturnFields.SCORE; + + @Override + public void init(NamedList args) { + initConfiguredExpression(args); + initScoreKey(args); + super.init(args); + } + + /** Checks for optional scoreKey override */ + private void initScoreKey(NamedList args) { + final int scoreIdx = args.indexOf(SCORE_KEY, 0); + if (scoreIdx < 0) { + // if it's not in the list at all, use the existing default... + return; + } + + Object arg = args.remove(scoreIdx); + // null is valid if they want to prevent default binding + scoreKey = (null == arg) ? null : arg.toString(); + } + + /** Parses the pre-configured expressions */ + private void initConfiguredExpression(NamedList args) { + Object arg = args.remove(EXPRESSIONS_KEY); + if (!(arg instanceof NamedList)) { + // :TODO: null arg may be ok if we want to later support dynamic expressions + throw new SolrException( + SERVER_ERROR, EXPRESSIONS_KEY + " must be configured as a list of named expressions"); + } + @SuppressWarnings("unchecked") + NamedList input = (NamedList) arg; + Map expr = new HashMap<>(); + for (Map.Entry item : input) { + String key = item.getKey(); + try { + Expression val = JavascriptCompiler.compile(item.getValue()); + expr.put(key, val); + } catch (ParseException e) { + throw new SolrException( + SERVER_ERROR, "Unable to parse javascript expression: " + item.getValue(), e); + } + } + + // TODO: should we support mapping positional func args to names in bindings? + // + // ie: ... + // + // + // foo * bar / baz + // + // baz + // bar + // + // + // 32 + // ... + // + // and then: "expr(my_expr,42,56)" == "32 * 56 / 42" + + exceptionIfCycles(expr); + this.expressions = Collections.unmodifiableMap(expr); + } + + @Override + public ValueSource parse(FunctionQParser fp) throws SyntaxError { + assert null != fp; + + String key = fp.parseArg(); + // TODO: allow function params for overriding bindings? + // TODO: support dynamic expressions: expr("foo * bar / 32") ?? + + IndexSchema schema = fp.getReq().getSchema(); + + SolrBindings b = new SolrBindings(scoreKey, expressions, schema); + return ValueSource.fromDoubleValuesSource(b.getDoubleValuesSource(key)); + } + + /** Validates that a Map of named expressions does not contain any cycles. */ + public static void exceptionIfCycles(Map expressions) { + + // TODO: there's probably a more efficient way to do this + // TODO: be nice to just return the shortest cycles (ie: b->a->b instead of x->a->b->a) + + List cycles = new ArrayList<>(expressions.size()); + Set checkedKeys = new LinkedHashSet<>(); + + for (String key : expressions.keySet()) { + Set visitedKeys = new LinkedHashSet<>(); + String cycle = makeCycleErrString(key, expressions, visitedKeys, checkedKeys); + if (null != cycle) { + cycles.add(cycle); + } + } + if (0 < cycles.size()) { + throw new SolrException( + SERVER_ERROR, + "At least " + + cycles.size() + + " cycles detected in configured expressions: [" + + String.join("], [", cycles) + + "]"); + } + } + + /** + * Recursively checks for cycles, returning null if none found - otherwise the string is a + * representation of the cycle found (may not be the shortest cycle) This method recursively + * adds to visitedKeys and checkedKeys + */ + private static String makeCycleErrString( + String key, + Map expressions, + Set visitedKeys, + Set checkedKeys) { + if (checkedKeys.contains(key)) { + return null; + } + if (visitedKeys.contains(key)) { + checkedKeys.add(key); + return String.join("=>", visitedKeys) + "=>" + key; + } + visitedKeys.add(key); + + Expression expr = expressions.get(key); + if (null != expr) { + for (String var : expr.variables) { + assert null != var; + + String err = makeCycleErrString(var, expressions, visitedKeys, checkedKeys); + if (null != err) { + return err; + } + } + } + + checkedKeys.add(key); + return null; + } + + /** + * A bindings class that recognizes pre-configured named expression and uses schema fields to + * resolve variables that have not been configured as expressions. + * + * @lucene.internal + */ + public static class SolrBindings extends Bindings { + private final String scoreKey; + private final Map expressions; + private final IndexSchema schema; + /** + * @param scoreKey The binding name that should be used to represent the score, may be null + * @param expressions Mapping of expression names that will be consulted as the primary + * bindings, get() should return null if key is not bound to an expression (will be used + * read only, will *not* be defensively copied, must not contain cycles) + * @param schema IndexSchema for field bindings + */ + public SolrBindings(String scoreKey, Map expressions, IndexSchema schema) { + this.scoreKey = scoreKey; + this.expressions = expressions; + this.schema = schema; + // :TODO: add support for request time bindings (function args) + } + + @Override + public DoubleValuesSource getDoubleValuesSource(String key) { + assert null != key; + + if (Objects.equals(scoreKey, key)) { + return DoubleValuesSource.SCORES; + } + + Expression expr = expressions.get(key); + if (null != expr) { + try { + return expr.getDoubleValuesSource(this); + } catch (IllegalArgumentException e) { + throw new IllegalArgumentException( + "Invalid binding for key '" + key + "' transative binding problem: " + e.getMessage(), + e); + } + } + + SchemaField field = schema.getFieldOrNull(key); + if (null != field) { + return field.getType().getValueSource(field, null).asDoubleValuesSource(); + } + + throw new IllegalArgumentException("No binding or schema field for key: " + key); + } + } +} diff --git a/solr/core/src/test-files/solr/collection1/conf/solrconfig-expressions-vs.xml b/solr/core/src/test-files/solr/collection1/conf/solrconfig-expressions-vs.xml new file mode 100644 index 00000000000..64658c9ae7c --- /dev/null +++ b/solr/core/src/test-files/solr/collection1/conf/solrconfig-expressions-vs.xml @@ -0,0 +1,52 @@ + + + + + + ${tests.luceneMatchVersion:LUCENE_CURRENT} + + + + + + + + + + + + + sin(1) + cos(sin1) + sqrt(int1_i) + sqrt(double1_d) + date1_dt - 631036800000 + 1 + score + 1 + one_plus_score + sqrt_int1_i + one_plus_score + one_plus_score*ln(cos_sin1) + + + + + ssccoorree + + 1 + ssccoorree + + + diff --git a/solr/core/src/test/org/apache/solr/search/ExpressionValueSourceParserTest.java b/solr/core/src/test/org/apache/solr/search/ExpressionValueSourceParserTest.java new file mode 100644 index 00000000000..9c837c55851 --- /dev/null +++ b/solr/core/src/test/org/apache/solr/search/ExpressionValueSourceParserTest.java @@ -0,0 +1,462 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.search; + +import java.text.ParseException; +import java.util.HashMap; +import java.util.Map; +import org.apache.lucene.expressions.Expression; +import org.apache.lucene.expressions.js.JavascriptCompiler; +import org.apache.lucene.queries.function.ValueSource; +import org.apache.solr.SolrTestCaseJ4; +import org.apache.solr.common.SolrException; +import org.apache.solr.schema.IndexSchema; +import org.apache.solr.search.ExpressionValueSourceParser.SolrBindings; +import org.apache.solr.util.DateMathParser; +import org.junit.BeforeClass; + +public class ExpressionValueSourceParserTest extends SolrTestCaseJ4 { + + @BeforeClass + public static void beforeTests() throws Exception { + initCore("solrconfig-expressions-vs.xml", "schema15.xml"); + + assertU( + adoc("id", "1", "int1_i", "50", "double1_d", "-2.5", "date1_dt", "1996-12-19T16:39:57Z")); + assertU( + adoc("id", "2", "int1_i", "-30", "double1_d", "10.3", "date1_dt", "1999-12-19T16:39:57Z")); + assertU( + adoc("id", "3", "int1_i", "10", "double1_d", "500.3", "date1_dt", "1995-12-19T16:39:57Z")); + assertU(adoc("id", "4", "int1_i", "40", "double1_d", "-1", "date1_dt", "1994-12-19T16:39:57Z")); + assertU( + adoc("id", "5", "int1_i", "20", "double1_d", "2.1", "date1_dt", "1997-12-19T16:39:57Z")); + + assertU(commit()); + } + + public void testValidBindings() throws ParseException { + IndexSchema schema = h.getCore().getLatestSchema(); + Map exprs = new HashMap<>(); + exprs.put("foo", JavascriptCompiler.compile("((0.3*popularity)/10.0)+(0.7*ScOrE)")); + exprs.put("popularity", JavascriptCompiler.compile("foo_i")); + SolrBindings bindings = new SolrBindings("ScOrE", exprs, schema); + + assertEquals( + "foo_i from bindings is wrong", + schema + .getFieldType("foo_i") + .getValueSource(schema.getField("foo_i"), null) + .asDoubleValuesSource(), + bindings.getDoubleValuesSource("foo_i")); + assertNotNull("pop bindings failed", bindings.getDoubleValuesSource("popularity")); + assertNotNull("foo bindings failed", bindings.getDoubleValuesSource("foo")); + ValueSource scoreBind = + ValueSource.fromDoubleValuesSource(bindings.getDoubleValuesSource("ScOrE")); + assertNotNull("ScOrE bindings failed", scoreBind); + + try { + bindings.getDoubleValuesSource("not_a_expr_and_not_in_schema"); + fail("no exception from bogus binding"); + } catch (IllegalArgumentException e) { + assertTrue( + "wrong exception message: " + e.getMessage(), + e.getMessage().contains("not_a_expr_and_not_in_schema")); + } + + // change things up a bit + + exprs.put("ScOrE", JavascriptCompiler.compile("42")); + + bindings = new SolrBindings(null, exprs, schema); + assertNotNull("foo bindings failed", bindings.getDoubleValuesSource("foo")); + } + + public void testBogusBindings() throws ParseException { + IndexSchema schema = h.getCore().getLatestSchema(); + Map exprs = new HashMap<>(); + exprs.put("foo", JavascriptCompiler.compile("((0.3*popularity)/10.0)+(0.7*ScOrE)")); + exprs.put("popularity", JavascriptCompiler.compile("yak")); + SolrBindings bindings = new SolrBindings("ScOrE", exprs, schema); + + try { + bindings.getDoubleValuesSource("yak"); + fail("sanity check failed: yak has a binding?"); + } catch (IllegalArgumentException e) { + // NOOP + } + + try { + bindings.getDoubleValuesSource("foo"); + fail("foo should be an invalid transative binding"); + } catch (IllegalArgumentException e) { + String err = e.getMessage(); + assertTrue( + "wrong exception message: " + err, + (err.contains("foo") && err.contains("popularity") && err.contains("yak"))); + } + + // change things up a bit + + bindings = new SolrBindings(null, exprs, schema); + try { + bindings.getDoubleValuesSource("ScOrE"); + fail("ScOrE should not have bindings"); + } catch (IllegalArgumentException e) { + // NOOP + } + try { + bindings.getDoubleValuesSource("score"); + fail("score should not have bindings"); + } catch (IllegalArgumentException e) { + // NOOP + } + } + + /** + * @see ExpressionValueSourceParser#exceptionIfCycles(Map) + */ + public void testCycleChecker() throws Exception { + Map exprs = new HashMap<>(); + + exprs.put("foo", JavascriptCompiler.compile("bar * ack")); + ExpressionValueSourceParser.exceptionIfCycles(exprs); + + exprs.put("bar", JavascriptCompiler.compile("ack * ack")); + ExpressionValueSourceParser.exceptionIfCycles(exprs); + + exprs.put("baz", JavascriptCompiler.compile("bar * foo")); + ExpressionValueSourceParser.exceptionIfCycles(exprs); + + // now we start to get some errors + exprs.put("ack", JavascriptCompiler.compile("ack * ack")); + + try { + ExpressionValueSourceParser.exceptionIfCycles(exprs); + fail("no error about ack depending on ack"); + } catch (SolrException e) { + assertTrue(e.getMessage(), e.getMessage().contains("ack=>ack")); + } + + // different, deeper error + exprs.put("ack", JavascriptCompiler.compile("foo * 2")); + + try { + ExpressionValueSourceParser.exceptionIfCycles(exprs); + fail("no error about ack/foo/bar"); + } catch (SolrException e) { + String msg = e.getMessage(); + // the thing about cycles: they might be found in either order + assertTrue(msg, msg.contains("foo=>ack") || msg.contains("ack=>foo")); + assertTrue(msg, msg.contains("bar=>ack")); + } + + exprs.remove("bar"); + exprs.put("wack", JavascriptCompiler.compile("sqrt(wack)")); + + try { + ExpressionValueSourceParser.exceptionIfCycles(exprs); + fail("no error about ack depending on ack"); + } catch (SolrException e) { + String msg = e.getMessage(); + // the thing about cycles: they might be found in either order + assertTrue(msg, msg.contains("foo=>ack") || msg.contains("ack=>foo")); + assertTrue(msg, msg.contains("wack=>wack")); + assertTrue(msg, msg.contains("At least 2 cycles")); + } + } + + /** tests clean error when no such binding */ + public void testNoSuchBinding() { + assertQEx( + "should have gotten user error for invalid binding", + req( + "fl", "id", + "q", "{!func}field(int1_i)", + "sort", "expr(this_expression_is_not_bound) desc"), + 400); + } + + /** tests an expression referring to a score field using an overridden score binding */ + public void testSortSsccoorree() { + assertQ( + "sort", + req( + "fl", "id", + "q", "{!func}field(int1_i)", + "sort", "expr_ssccoorree(one_plus_score) desc,id asc"), + "//*[@numFound='5']", + "//result/doc[1]/str[@name='id'][.='1']", + "//result/doc[2]/str[@name='id'][.='4']", + "//result/doc[3]/str[@name='id'][.='5']", + "//result/doc[4]/str[@name='id'][.='3']", + "//result/doc[5]/str[@name='id'][.='2']"); + } + + /** tests a constant expression */ + public void testSortConstant() { + assertQ( + "sort", + req("fl", "id", "q", "*:*", "sort", "expr(sin1) desc,id asc"), + "//*[@numFound='5']", + "//result/doc[1]/str[@name='id'][.='1']", + "//result/doc[2]/str[@name='id'][.='2']", + "//result/doc[3]/str[@name='id'][.='3']", + "//result/doc[4]/str[@name='id'][.='4']", + "//result/doc[5]/str[@name='id'][.='5']"); + } + + /** tests an expression referring to another expression */ + public void testSortExpression() { + assertQ( + "sort", + req("fl", "id", "q", "*:*", "sort", "expr(cos_sin1) desc,id asc"), + "//*[@numFound='5']", + "//result/doc[1]/str[@name='id'][.='1']", + "//result/doc[2]/str[@name='id'][.='2']", + "//result/doc[3]/str[@name='id'][.='3']", + "//result/doc[4]/str[@name='id'][.='4']", + "//result/doc[5]/str[@name='id'][.='5']"); + } + + /** tests an expression referring to an int field */ + public void testSortInt() { + assertQ( + "sort", + req("fl", "id", "q", "*:*", "sort", "expr(sqrt_int1_i) desc,id asc"), + "//*[@numFound='5']", + "//result/doc[1]/str[@name='id'][.='2']", // NaN + "//result/doc[2]/str[@name='id'][.='1']", + "//result/doc[3]/str[@name='id'][.='4']", + "//result/doc[4]/str[@name='id'][.='5']", + "//result/doc[5]/str[@name='id'][.='3']"); + } + + /** tests an expression referring to a double field */ + public void testSortDouble() { + assertQ( + "sort", + req("fl", "id", "q", "*:*", "sort", "expr(sqrt_double1_d) desc,id asc"), + "//*[@numFound='5']", + "//result/doc[1]/str[@name='id'][.='1']", // NaN + "//result/doc[2]/str[@name='id'][.='4']", // NaN + "//result/doc[3]/str[@name='id'][.='3']", + "//result/doc[4]/str[@name='id'][.='2']", + "//result/doc[5]/str[@name='id'][.='5']"); + } + + /** tests an expression referring to a date field */ + public void testSortDate() { + assertQ( + "sort", + req("fl", "id", "q", "*:*", "sort", "expr(date1_dt_minus_1990) desc,id asc"), + "//*[@numFound='5']", + "//result/doc[1]/str[@name='id'][.='2']", + "//result/doc[2]/str[@name='id'][.='5']", + "//result/doc[3]/str[@name='id'][.='1']", + "//result/doc[4]/str[@name='id'][.='3']", + "//result/doc[5]/str[@name='id'][.='4']"); + } + + /** tests an expression referring to a score field */ + public void testSortScore() { + assertQ( + "sort", + req("fl", "id", "q", "{!func}field(int1_i)", "sort", "expr(one_plus_score) desc,id asc"), + "//*[@numFound='5']", + "//result/doc[1]/str[@name='id'][.='1']", + "//result/doc[2]/str[@name='id'][.='4']", + "//result/doc[3]/str[@name='id'][.='5']", + "//result/doc[4]/str[@name='id'][.='3']", + "//result/doc[5]/str[@name='id'][.='2']"); + } + + /** tests an expression referring to another expression with externals */ + public void testSortScore2() { + assertQ( + "sort", + req("fl", "id", "q", "{!func}field(int1_i)", "sort", "expr(two_plus_score) desc,id asc"), + "//*[@numFound='5']", + "//result/doc[1]/str[@name='id'][.='1']", + "//result/doc[2]/str[@name='id'][.='4']", + "//result/doc[3]/str[@name='id'][.='5']", + "//result/doc[4]/str[@name='id'][.='3']", + "//result/doc[5]/str[@name='id'][.='2']"); + } + + /** tests an expression referring to two expressions with externals */ + public void testSortScore3() { + assertQ( + "sort", + req( + "fl", + "id", + "q", + "{!func}field(int1_i)", + "sort", + "expr(sqrt_int1_i_plus_one_plus_score) desc,id asc"), + "//*[@numFound='5']", + "//result/doc[1]/str[@name='id'][.='2']", + "//result/doc[2]/str[@name='id'][.='1']", + "//result/doc[3]/str[@name='id'][.='4']", + "//result/doc[4]/str[@name='id'][.='5']", + "//result/doc[5]/str[@name='id'][.='3']"); + } + + /** tests an expression referring to another expression and a function */ + public void testSortMixed() { + assertQ( + "sort", + req("fl", "id", "q", "{!func}field(int1_i)", "sort", "expr(mixed_expr) desc,id asc"), + "//*[@numFound='5']", + "//result/doc[1]/str[@name='id'][.='2']", + "//result/doc[2]/str[@name='id'][.='3']", + "//result/doc[3]/str[@name='id'][.='5']", + "//result/doc[4]/str[@name='id'][.='4']", + "//result/doc[5]/str[@name='id'][.='1']"); + } + + /** tests a constant expression */ + public void testReturnConstant() { + final float expected = (float) Math.sin(1); + assertQ( + "return", + req("fl", "sin1:expr(sin1)", "q", "*:*", "sort", "id asc"), + "//*[@numFound='5']", + "//result/doc[1]/float[@name='sin1'][.='" + expected + "']", + "//result/doc[2]/float[@name='sin1'][.='" + expected + "']", + "//result/doc[3]/float[@name='sin1'][.='" + expected + "']", + "//result/doc[4]/float[@name='sin1'][.='" + expected + "']", + "//result/doc[5]/float[@name='sin1'][.='" + expected + "']"); + } + + /** tests an expression referring to another expression */ + public void testReturnExpression() { + final float expected = (float) Math.cos(Math.sin(1)); + assertQ( + "sort", + req("fl", "expr(cos_sin1)", "q", "*:*", "sort", "id asc"), + "//*[@numFound='5']", + "//result/doc[1]/float[@name='expr(cos_sin1)'][.=" + expected + "]", + "//result/doc[2]/float[@name='expr(cos_sin1)'][.=" + expected + "]", + "//result/doc[3]/float[@name='expr(cos_sin1)'][.=" + expected + "]", + "//result/doc[4]/float[@name='expr(cos_sin1)'][.=" + expected + "]", + "//result/doc[5]/float[@name='expr(cos_sin1)'][.=" + expected + "]"); + } + + /** tests an expression referring to an int field */ + public void testReturnInt() { + assertQ( + "return", + req("fl", "foo:expr(sqrt_int1_i)", "q", "*:*", "sort", "id asc"), + "//*[@numFound='5']", + "//result/doc[1]/float[@name='foo'][.=" + (float) Math.sqrt(50) + "]", + "//result/doc[2]/float[@name='foo'][.='NaN']", + "//result/doc[3]/float[@name='foo'][.=" + (float) Math.sqrt(10) + "]", + "//result/doc[4]/float[@name='foo'][.=" + (float) Math.sqrt(40) + "]", + "//result/doc[5]/float[@name='foo'][.=" + (float) Math.sqrt(20) + "]"); + } + + /** tests an expression referring to a double field */ + public void testReturnDouble() { + assertQ( + "return", + req("fl", "bar:expr(sqrt_double1_d)", "q", "*:*", "sort", "id asc"), + "//*[@numFound='5']", + "//result/doc[1]/float[@name='bar'][.='NaN']", + "//result/doc[2]/float[@name='bar'][.=" + (float) Math.sqrt(10.3d) + "]", + "//result/doc[3]/float[@name='bar'][.=" + (float) Math.sqrt(500.3d) + "]", + "//result/doc[4]/float[@name='bar'][.='NaN']", + "//result/doc[5]/float[@name='bar'][.=" + (float) Math.sqrt(2.1d) + "]"); + } + + /** tests an expression referring to a date field */ + public void testReturnDate() { + assertQ( + "return", + req("fl", "date1_dt_minus_1990:expr(date1_dt_minus_1990)", "q", "*:*", "sort", "id asc"), + "//*[@numFound='5']", + "//result/doc[1]/float[@name='date1_dt_minus_1990'][.='" + + (float) + (DateMathParser.parseMath(null, "1996-12-19T16:39:57Z").getTime() - 631036800000D) + + "']", + "//result/doc[2]/float[@name='date1_dt_minus_1990'][.='" + + (float) + (DateMathParser.parseMath(null, "1999-12-19T16:39:57Z").getTime() - 631036800000D) + + "']", + "//result/doc[3]/float[@name='date1_dt_minus_1990'][.='" + + (float) + (DateMathParser.parseMath(null, "1995-12-19T16:39:57Z").getTime() - 631036800000D) + + "']", + "//result/doc[4]/float[@name='date1_dt_minus_1990'][.='" + + (float) + (DateMathParser.parseMath(null, "1994-12-19T16:39:57Z").getTime() - 631036800000D) + + "']", + "//result/doc[5]/float[@name='date1_dt_minus_1990'][.='" + + (float) + (DateMathParser.parseMath(null, "1997-12-19T16:39:57Z").getTime() - 631036800000D) + + "']"); + } + + /** tests an expression referring to score */ + public void testReturnScores() { + assertQ( + "return", + // :nocommit: see ValueSourceAugmenter's nocommit for why fl needs "score" + req( + "fl", "expr(one_plus_score),score", + "q", "{!func}field(int1_i)", + "sort", "id asc"), + "//*[@numFound='5']", + "//result/doc[1]/float[@name='expr(one_plus_score)'][.='51.0']", + "//result/doc[2]/float[@name='expr(one_plus_score)'][.='1.0']", + "//result/doc[3]/float[@name='expr(one_plus_score)'][.='11.0']", + "//result/doc[4]/float[@name='expr(one_plus_score)'][.='41.0']", + "//result/doc[5]/float[@name='expr(one_plus_score)'][.='21.0']"); + } + + public void testReturnScores2() { + assertQ( + "return", + // :nocommit: see ValueSourceAugmenter's nocommit for why fl needs "score" + req( + "fl", "two_plus_score:expr(two_plus_score),score", + "q", "{!func}field(int1_i)", + "sort", "id asc"), + "//*[@numFound='5']", + "//result/doc[1]/float[@name='two_plus_score'][.='52.0']", + "//result/doc[2]/float[@name='two_plus_score'][.='2.0']", + "//result/doc[3]/float[@name='two_plus_score'][.='12.0']", + "//result/doc[4]/float[@name='two_plus_score'][.='42.0']", + "//result/doc[5]/float[@name='two_plus_score'][.='22.0']"); + } + + public void testReturnScores3() { + assertQ( + "return", + // :nocommit: see ValueSourceAugmenter's nocommit for why fl needs "score" + req( + "fl", "foo:expr(sqrt_int1_i_plus_one_plus_score),score", + "q", "{!func}field(int1_i)", + "sort", "id asc"), + "//*[@numFound='5']", + "//result/doc[1]/float[@name='foo'][.='" + (float) (Math.sqrt(50) + 1 + 50) + "']", + "//result/doc[2]/float[@name='foo'][.='NaN']", + "//result/doc[3]/float[@name='foo'][.='" + (float) (Math.sqrt(10) + 1 + 10) + "']", + "//result/doc[4]/float[@name='foo'][.='" + (float) (Math.sqrt(40) + 1 + 40) + "']", + "//result/doc[5]/float[@name='foo'][.='" + (float) (Math.sqrt(20) + 1 + 20) + "']"); + } +} From 2dbfbde39b2ca9e9b72e3e265ddf97d1540fc8c8 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Tue, 6 May 2025 20:05:49 -0400 Subject: [PATCH 02/14] ValueSourceAugmenter from SOLR-15030 --- .../response/transform/ValueSourceAugmenter.java | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/response/transform/ValueSourceAugmenter.java b/solr/core/src/java/org/apache/solr/response/transform/ValueSourceAugmenter.java index 85a8d57ffbd..ef9ddc21ed9 100644 --- a/solr/core/src/java/org/apache/solr/response/transform/ValueSourceAugmenter.java +++ b/solr/core/src/java/org/apache/solr/response/transform/ValueSourceAugmenter.java @@ -71,15 +71,6 @@ public void setContext(ResultContext context) { SolrIndexSearcher searcher; List readerContexts; - @Override - public void transform(SolrDocument doc, int docid, float score) throws IOException { - if (context != null && context.wantsScores()) { - fcontext.put("scorer", new ScoreAndDoc(docid, score)); - } - - transform(doc, docid); - } - @Override public void transform(SolrDocument doc, int docid, DocIterationInfo docInfo) { // This is only good for random-access functions @@ -89,8 +80,13 @@ public void transform(SolrDocument doc, int docid, DocIterationInfo docInfo) { // TODO: calculate this stuff just once across diff functions int idx = ReaderUtil.subIndex(docid, readerContexts); LeafReaderContext rcontext = readerContexts.get(idx); - FunctionValues values = valueSource.getValues(fcontext, rcontext); int localId = docid - rcontext.docBase; + + if (context.wantsScores()) { + fcontext.put("scorer", new ScoreAndDoc(localId, docInfo.score())); + } + + FunctionValues values = valueSource.getValues(fcontext, rcontext); setValue(doc, values.objectVal(localId)); } catch (IOException e) { throw new SolrException( From fecb3cd77e05d55cef4390061114d4d12a2afbce Mon Sep 17 00:00:00 2001 From: David Smiley Date: Tue, 6 May 2025 20:06:07 -0400 Subject: [PATCH 03/14] Convert nocommit to TODO --- .../solr/search/ExpressionValueSourceParser.java | 12 +++++++----- .../solr/search/ExpressionValueSourceParserTest.java | 8 ++++++++ 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/search/ExpressionValueSourceParser.java b/solr/core/src/java/org/apache/solr/search/ExpressionValueSourceParser.java index a72e9f6380e..80de3eca49e 100644 --- a/solr/core/src/java/org/apache/solr/search/ExpressionValueSourceParser.java +++ b/solr/core/src/java/org/apache/solr/search/ExpressionValueSourceParser.java @@ -39,11 +39,12 @@ /** * A ValueSource parser that can be configured with (named) pre-complied Expressions that can then - * be evaluated at request time. :nocommit: more docs, example configs, example usage :nocommit: - * need "bad-solrconfig..." level test of cycle expressions, using score w/null score binding, - * etc..." + * be evaluated at request time. */ public class ExpressionValueSourceParser extends ValueSourceParser { + + // TODO: more docs, example configs, example usage + public static final String SCORE_KEY = "score-name"; public static final String EXPRESSIONS_KEY = "expressions"; @@ -155,8 +156,8 @@ public static void exceptionIfCycles(Map expressions) { /** * Recursively checks for cycles, returning null if none found - otherwise the string is a - * representation of the cycle found (may not be the shortest cycle) This method recursively - * adds to visitedKeys and checkedKeys + * representation of the cycle found (may not be the shortest cycle) This method recursively adds + * to visitedKeys and checkedKeys */ private static String makeCycleErrString( String key, @@ -198,6 +199,7 @@ public static class SolrBindings extends Bindings { private final String scoreKey; private final Map expressions; private final IndexSchema schema; + /** * @param scoreKey The binding name that should be used to represent the score, may be null * @param expressions Mapping of expression names that will be consulted as the primary diff --git a/solr/core/src/test/org/apache/solr/search/ExpressionValueSourceParserTest.java b/solr/core/src/test/org/apache/solr/search/ExpressionValueSourceParserTest.java index 9c837c55851..82de265c6f5 100644 --- a/solr/core/src/test/org/apache/solr/search/ExpressionValueSourceParserTest.java +++ b/solr/core/src/test/org/apache/solr/search/ExpressionValueSourceParserTest.java @@ -28,9 +28,13 @@ import org.apache.solr.search.ExpressionValueSourceParser.SolrBindings; import org.apache.solr.util.DateMathParser; import org.junit.BeforeClass; +import org.junit.Ignore; public class ExpressionValueSourceParserTest extends SolrTestCaseJ4 { + // TODO need "bad-solrconfig..." level test of cycle expressions, using score w/null score + // binding, etc... + @BeforeClass public static void beforeTests() throws Exception { initCore("solrconfig-expressions-vs.xml", "schema15.xml"); @@ -191,6 +195,7 @@ public void testNoSuchBinding() { } /** tests an expression referring to a score field using an overridden score binding */ + @Ignore("SOLR-XXXX can't sort by expression referencing the score") public void testSortSsccoorree() { assertQ( "sort", @@ -272,6 +277,7 @@ public void testSortDate() { } /** tests an expression referring to a score field */ + @Ignore("SOLR-XXXX can't sort by expression referencing the score") public void testSortScore() { assertQ( "sort", @@ -298,6 +304,7 @@ public void testSortScore2() { } /** tests an expression referring to two expressions with externals */ + @Ignore("SOLR-XXXX can't sort by expression referencing the score") public void testSortScore3() { assertQ( "sort", @@ -317,6 +324,7 @@ public void testSortScore3() { } /** tests an expression referring to another expression and a function */ + @Ignore("SOLR-XXXX can't sort by expression referencing the score") public void testSortMixed() { assertQ( "sort", From e05ebe7530f8d0083058c86b602ec384e68e41be Mon Sep 17 00:00:00 2001 From: David Smiley Date: Thu, 12 Jun 2025 21:47:43 -0400 Subject: [PATCH 04/14] VSA: fix prefetch to consider need for Scorable --- .../transform/ValueSourceAugmenter.java | 60 +++++++++++++++---- 1 file changed, 49 insertions(+), 11 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/response/transform/ValueSourceAugmenter.java b/solr/core/src/java/org/apache/solr/response/transform/ValueSourceAugmenter.java index 7ef2aac1443..52a41bd60be 100644 --- a/solr/core/src/java/org/apache/solr/response/transform/ValueSourceAugmenter.java +++ b/solr/core/src/java/org/apache/solr/response/transform/ValueSourceAugmenter.java @@ -22,6 +22,7 @@ import java.util.Map; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.index.ReaderUtil; +import org.apache.lucene.internal.hppc.IntFloatHashMap; import org.apache.lucene.internal.hppc.IntObjectHashMap; import org.apache.lucene.queries.function.FunctionValues; import org.apache.lucene.queries.function.ValueSource; @@ -69,20 +70,44 @@ public void setContext(ResultContext context) { fcontext = ValueSource.newContext(searcher); this.valueSource.createWeight(fcontext, searcher); final var docList = context.getDocList(); - if (docList == null) { + final int prefetchSize = docList == null ? 0 : Math.min(docList.size(), maxPrefetchSize); + if (prefetchSize == 0) { return; } - final int prefetchSize = Math.min(docList.size(), maxPrefetchSize); + // Check if scores are wanted and initialize the Scorable if so + final MutableScorable scorable; // stored in fcontext (when not null) + final IntFloatHashMap docToScoreMap; + if (context.wantsScores()) { // TODO switch to ValueSource.needsScores once it exists + docToScoreMap = new IntFloatHashMap(prefetchSize); + scorable = + new MutableScorable() { + @Override + public float score() throws IOException { + return docToScoreMap.get(docBase + localDocId); + } + }; + fcontext.put("scorer", scorable); + } else { + scorable = null; + docToScoreMap = null; + } + + // Get the IDs and scores final int[] ids = new int[prefetchSize]; int i = 0; var iter = docList.iterator(); while (iter.hasNext() && i < prefetchSize) { - ids[i++] = iter.nextDoc(); + ids[i] = iter.nextDoc(); + if (docToScoreMap != null) { + docToScoreMap.put(ids[i], iter.score()); + } + i++; } Arrays.sort(ids); - cachedValuesById = new IntObjectHashMap<>(ids.length); + // Get the values in docId order. Store in cachedValuesById + cachedValuesById = new IntObjectHashMap<>(ids.length); FunctionValues values = null; int docBase = -1; int currentIdx = -1; @@ -95,9 +120,16 @@ public void setContext(ResultContext context) { values = valueSource.getValues(fcontext, rcontext); } int localId = docid - docBase; - var value = values.objectVal(localId); + + if (scorable != null) { + scorable.docBase = docBase; + scorable.localDocId = localId; + } + var value = values.objectVal(localId); // note: might use the Scorable + cachedValuesById.put(docid, value != null ? value : NULL_SENTINEL); } + fcontext.remove("scorer"); // remove ours; it was there only for prefetching } catch (IOException e) { throw new SolrException( SolrException.ErrorCode.SERVER_ERROR, "exception for valuesource " + valueSource, e); @@ -136,6 +168,17 @@ public void transform(SolrDocument doc, int docid, DocIterationInfo docIteration } } + private abstract static class MutableScorable extends Scorable { + + int docBase; + int localDocId; + + @Override + public int docID() { + return localDocId; + } + } + /** Always returns true */ @Override public boolean needsSolrIndexSearcher() { @@ -148,12 +191,7 @@ protected void setValue(SolrDocument doc, Object val) { } } - /** - * Fake scorer for a single document - * - *

TODO: when SOLR-5595 is fixed, this wont be needed, as we dont need to recompute sort values - * here from the comparator - */ + /** Fake scorer for a single document */ protected static class ScoreAndDoc extends Scorable { final int docid; final float score; From 00f8435c0374fb1f5c2ee515f8b0ad0fb35f7c2f Mon Sep 17 00:00:00 2001 From: David Smiley Date: Thu, 12 Jun 2025 23:08:31 -0400 Subject: [PATCH 05/14] ignore another test that can't be supported yet --- .../org/apache/solr/search/ExpressionValueSourceParserTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/solr/core/src/test/org/apache/solr/search/ExpressionValueSourceParserTest.java b/solr/core/src/test/org/apache/solr/search/ExpressionValueSourceParserTest.java index 82de265c6f5..3370db0f6fd 100644 --- a/solr/core/src/test/org/apache/solr/search/ExpressionValueSourceParserTest.java +++ b/solr/core/src/test/org/apache/solr/search/ExpressionValueSourceParserTest.java @@ -291,6 +291,7 @@ public void testSortScore() { } /** tests an expression referring to another expression with externals */ + @Ignore("SOLR-XXXX can't sort by expression referencing the score") public void testSortScore2() { assertQ( "sort", From 38d1eb5a88073c0013e9b0d67983eb2897ffa6e1 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Thu, 12 Jun 2025 23:42:45 -0400 Subject: [PATCH 06/14] optimization (avoid needless subIndex) --- .../solr/response/transform/ValueSourceAugmenter.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/response/transform/ValueSourceAugmenter.java b/solr/core/src/java/org/apache/solr/response/transform/ValueSourceAugmenter.java index 52a41bd60be..21d1434b64e 100644 --- a/solr/core/src/java/org/apache/solr/response/transform/ValueSourceAugmenter.java +++ b/solr/core/src/java/org/apache/solr/response/transform/ValueSourceAugmenter.java @@ -110,15 +110,16 @@ public float score() throws IOException { cachedValuesById = new IntObjectHashMap<>(ids.length); FunctionValues values = null; int docBase = -1; - int currentIdx = -1; + int nextDocBase = 0; // i.e. this segment's maxDoc for (int docid : ids) { - int idx = ReaderUtil.subIndex(docid, readerContexts); - if (currentIdx != idx) { - currentIdx = idx; + if (docid >= nextDocBase) { + int idx = ReaderUtil.subIndex(docid, readerContexts); LeafReaderContext rcontext = readerContexts.get(idx); docBase = rcontext.docBase; + nextDocBase = docBase + rcontext.reader().maxDoc(); values = valueSource.getValues(fcontext, rcontext); } + int localId = docid - docBase; if (scorable != null) { From 3270f77a4976b33b3a36ae09365fea3421e959cf Mon Sep 17 00:00:00 2001 From: David Smiley Date: Mon, 23 Jun 2025 09:41:48 -0400 Subject: [PATCH 07/14] Ref Guide page --- .../search/ExpressionValueSourceParser.java | 2 - .../pages/expression-value-source-parser.adoc | 112 ++++++++++++++++++ .../modules/query-guide/querying-nav.adoc | 1 + 3 files changed, 113 insertions(+), 2 deletions(-) create mode 100644 solr/solr-ref-guide/modules/query-guide/pages/expression-value-source-parser.adoc diff --git a/solr/core/src/java/org/apache/solr/search/ExpressionValueSourceParser.java b/solr/core/src/java/org/apache/solr/search/ExpressionValueSourceParser.java index 80de3eca49e..44a2fd274e6 100644 --- a/solr/core/src/java/org/apache/solr/search/ExpressionValueSourceParser.java +++ b/solr/core/src/java/org/apache/solr/search/ExpressionValueSourceParser.java @@ -43,8 +43,6 @@ */ public class ExpressionValueSourceParser extends ValueSourceParser { - // TODO: more docs, example configs, example usage - public static final String SCORE_KEY = "score-name"; public static final String EXPRESSIONS_KEY = "expressions"; diff --git a/solr/solr-ref-guide/modules/query-guide/pages/expression-value-source-parser.adoc b/solr/solr-ref-guide/modules/query-guide/pages/expression-value-source-parser.adoc new file mode 100644 index 00000000000..7271b9f5785 --- /dev/null +++ b/solr/solr-ref-guide/modules/query-guide/pages/expression-value-source-parser.adoc @@ -0,0 +1,112 @@ += Expression Value Source Parser +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +The `ExpressionValueSourceParser` allows you to configure named JavaScript expressions in your `solrconfig.xml` that can be referenced in function queries. +This provides a powerful way to define reusable expressions that can be combined and referenced in queries. +The expressions are based on the Lucene Expressions module, which you can learn more about in the {lucene-javadocs}/expressions/org/apache/lucene/expressions/js/package-summary.html[Lucene Expressions documentation]. + +== Configuration + +To use the `ExpressionValueSourceParser`, you need to configure it in your `solrconfig.xml` file: + +[source,xml] +---- + + + sin(1) + cos(sin1) + sqrt(int1_i) + 1 + score + + +---- + +The configuration consists of: + +* A `name` attribute that defines the parser name to be used in queries (e.g., `expr`) +* A `class` attribute that specifies the implementation class (`solr.ExpressionValueSourceParser`) +* A list of named expressions in the `expressions` element, where each expression is defined with a name and a JavaScript expression + +The parser binds the special name `score` to the document's score. + +== Using Expressions + +Once configured, you can use the named expressions in xref:function-queries.adoc[function queries] in any place that function queries are supported. +This first example shows it used for sorting: + +[source,text] +---- +q=*:*&sort=expr(sqrt_int1_i) desc +---- + +This example shows it in the field list to return calculated values: + +[source,text] +---- +fl=id,expr(one_plus_score),score +---- + + + +== Score Manipulation with Expressions + +A particularly powerful feature of the Expression Value Source Parser is the ability to reference the document score in expressions. +This allows for convenient score manipulation and boosting in a more natural way. +For example, you can create expressions that combine the score with other factors: + +[source,xml] +---- + + + sqrt(popularity) * score + + +---- + +Using expressions to manipulate scores is more efficient than using the "query" function query when you need to reference the score. +The "query" function effectively executes the score calculation twice internally, whereas expressions can reference the score directly. + +== Expression Features + +* Expressions can reference other expressions defined in the same parser +* Expressions can reference field values directly by field name +* Expressions can use the document's score +* Expressions support standard JavaScript math operations and functions +* The parser automatically detects and prevents circular references in expressions + +== Example + +This example shows how to configure expressions that build on each other: + +[source,xml] +---- + + + sqrt(popularity) + log(price) + sqrt_popularity / log_price + + +---- + +You can then use these expressions in queries: + +[source,text] +---- +q=title:solr&sort=expr(boost_formula) desc +---- diff --git a/solr/solr-ref-guide/modules/query-guide/querying-nav.adoc b/solr/solr-ref-guide/modules/query-guide/querying-nav.adoc index 973bd80f005..c66b1f85016 100644 --- a/solr/solr-ref-guide/modules/query-guide/querying-nav.adoc +++ b/solr/solr-ref-guide/modules/query-guide/querying-nav.adoc @@ -23,6 +23,7 @@ ** xref:dismax-query-parser.adoc[] ** xref:edismax-query-parser.adoc[] ** xref:function-queries.adoc[] +*** xref:expression-value-source-parser.adoc[] ** xref:local-params.adoc[] ** xref:json-request-api.adoc[] *** xref:json-query-dsl.adoc[] From f47cfa3a3a26483a68ff32dcba9e3aa85cc307e8 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Tue, 24 Jun 2025 23:36:52 -0400 Subject: [PATCH 08/14] Single expression, and support positional args --- .../search/ExpressionValueSourceParser.java | 195 ++++---------- .../conf/solrconfig-expressions-vs.xml | 56 ++-- .../ExpressionValueSourceParserTest.java | 253 +++++------------- .../pages/expression-value-source-parser.adoc | 100 ++----- 4 files changed, 183 insertions(+), 421 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/search/ExpressionValueSourceParser.java b/solr/core/src/java/org/apache/solr/search/ExpressionValueSourceParser.java index 44a2fd274e6..c27aaa3e837 100644 --- a/solr/core/src/java/org/apache/solr/search/ExpressionValueSourceParser.java +++ b/solr/core/src/java/org/apache/solr/search/ExpressionValueSourceParser.java @@ -20,13 +20,11 @@ import java.text.ParseException; import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; -import java.util.LinkedHashSet; import java.util.List; -import java.util.Map; import java.util.Objects; -import java.util.Set; +import java.util.Optional; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import org.apache.lucene.expressions.Bindings; import org.apache.lucene.expressions.Expression; import org.apache.lucene.expressions.js.JavascriptCompiler; @@ -38,16 +36,17 @@ import org.apache.solr.schema.SchemaField; /** - * A ValueSource parser that can be configured with (named) pre-complied Expressions that can then - * be evaluated at request time. + * A ValueSource parser configured with a pre-compiled expression that can then be evaluated at + * request time. It's powered by the Lucene Expressions module, which is a subset of JavaScript */ public class ExpressionValueSourceParser extends ValueSourceParser { - public static final String SCORE_KEY = "score-name"; - public static final String EXPRESSIONS_KEY = "expressions"; + public static final String SCORE_KEY = "score-name"; // TODO get rid of this? Why have it? + public static final String EXPRESSION_KEY = "expression"; - private Map expressions; - private String scoreKey = SolrReturnFields.SCORE; + private Expression expression; + private String scoreKey; + private int numPositionalArgs = 0; // Number of positional arguments in the expression @Override public void init(NamedList args) { @@ -58,158 +57,73 @@ public void init(NamedList args) { /** Checks for optional scoreKey override */ private void initScoreKey(NamedList args) { - final int scoreIdx = args.indexOf(SCORE_KEY, 0); - if (scoreIdx < 0) { - // if it's not in the list at all, use the existing default... - return; - } - - Object arg = args.remove(scoreIdx); - // null is valid if they want to prevent default binding - scoreKey = (null == arg) ? null : arg.toString(); + scoreKey = Optional.ofNullable((String) args.remove(SCORE_KEY)).orElse(SolrReturnFields.SCORE); } - /** Parses the pre-configured expressions */ + /** Parses the pre-configured expression */ private void initConfiguredExpression(NamedList args) { - Object arg = args.remove(EXPRESSIONS_KEY); - if (!(arg instanceof NamedList)) { - // :TODO: null arg may be ok if we want to later support dynamic expressions - throw new SolrException( - SERVER_ERROR, EXPRESSIONS_KEY + " must be configured as a list of named expressions"); - } - @SuppressWarnings("unchecked") - NamedList input = (NamedList) arg; - Map expr = new HashMap<>(); - for (Map.Entry item : input) { - String key = item.getKey(); - try { - Expression val = JavascriptCompiler.compile(item.getValue()); - expr.put(key, val); - } catch (ParseException e) { - throw new SolrException( - SERVER_ERROR, "Unable to parse javascript expression: " + item.getValue(), e); - } + String expressionStr = + Optional.ofNullable((String) args.remove(EXPRESSION_KEY)) + .orElseThrow( + () -> + new SolrException( + SERVER_ERROR, EXPRESSION_KEY + " must be configured with an expression")); + + // Find the highest positional argument in the expression + Pattern pattern = Pattern.compile("\\$(\\d+)"); + Matcher matcher = pattern.matcher(expressionStr); + while (matcher.find()) { + int argNum = Integer.parseInt(matcher.group(1)); + numPositionalArgs = Math.max(numPositionalArgs, argNum); } - // TODO: should we support mapping positional func args to names in bindings? - // - // ie: ... - // - // - // foo * bar / baz - // - // baz - // bar - // - // - // 32 - // ... - // - // and then: "expr(my_expr,42,56)" == "32 * 56 / 42" - - exceptionIfCycles(expr); - this.expressions = Collections.unmodifiableMap(expr); - } - - @Override - public ValueSource parse(FunctionQParser fp) throws SyntaxError { - assert null != fp; - - String key = fp.parseArg(); - // TODO: allow function params for overriding bindings? - // TODO: support dynamic expressions: expr("foo * bar / 32") ?? - - IndexSchema schema = fp.getReq().getSchema(); - - SolrBindings b = new SolrBindings(scoreKey, expressions, schema); - return ValueSource.fromDoubleValuesSource(b.getDoubleValuesSource(key)); - } - - /** Validates that a Map of named expressions does not contain any cycles. */ - public static void exceptionIfCycles(Map expressions) { - - // TODO: there's probably a more efficient way to do this - // TODO: be nice to just return the shortest cycles (ie: b->a->b instead of x->a->b->a) - - List cycles = new ArrayList<>(expressions.size()); - Set checkedKeys = new LinkedHashSet<>(); - - for (String key : expressions.keySet()) { - Set visitedKeys = new LinkedHashSet<>(); - String cycle = makeCycleErrString(key, expressions, visitedKeys, checkedKeys); - if (null != cycle) { - cycles.add(cycle); - } - } - if (0 < cycles.size()) { + // TODO add way to register additional functions + try { + this.expression = JavascriptCompiler.compile(expressionStr); + } catch (ParseException e) { throw new SolrException( - SERVER_ERROR, - "At least " - + cycles.size() - + " cycles detected in configured expressions: [" - + String.join("], [", cycles) - + "]"); + SERVER_ERROR, "Unable to parse javascript expression: " + expressionStr, e); } } - /** - * Recursively checks for cycles, returning null if none found - otherwise the string is a - * representation of the cycle found (may not be the shortest cycle) This method recursively adds - * to visitedKeys and checkedKeys - */ - private static String makeCycleErrString( - String key, - Map expressions, - Set visitedKeys, - Set checkedKeys) { - if (checkedKeys.contains(key)) { - return null; - } - if (visitedKeys.contains(key)) { - checkedKeys.add(key); - return String.join("=>", visitedKeys) + "=>" + key; - } - visitedKeys.add(key); + // TODO: support dynamic expressions: expr("foo * bar / 32") ?? - Expression expr = expressions.get(key); - if (null != expr) { - for (String var : expr.variables) { - assert null != var; + @Override + public ValueSource parse(FunctionQParser fp) throws SyntaxError { + assert null != fp; - String err = makeCycleErrString(var, expressions, visitedKeys, checkedKeys); - if (null != err) { - return err; - } - } + // Parse positional arguments if any + List positionalArgs = new ArrayList<>(); + for (int i = 0; i < numPositionalArgs; i++) { + ValueSource vs = fp.parseValueSource(); + positionalArgs.add(vs.asDoubleValuesSource()); } - checkedKeys.add(key); - return null; + IndexSchema schema = fp.getReq().getSchema(); + SolrBindings b = new SolrBindings(scoreKey, schema, positionalArgs); + return ValueSource.fromDoubleValuesSource(expression.getDoubleValuesSource(b)); } /** - * A bindings class that recognizes pre-configured named expression and uses schema fields to - * resolve variables that have not been configured as expressions. + * A bindings class that uses schema fields to resolve variables. * * @lucene.internal */ public static class SolrBindings extends Bindings { private final String scoreKey; - private final Map expressions; private final IndexSchema schema; + private final List positionalArgs; /** * @param scoreKey The binding name that should be used to represent the score, may be null - * @param expressions Mapping of expression names that will be consulted as the primary - * bindings, get() should return null if key is not bound to an expression (will be used - * read only, will *not* be defensively copied, must not contain cycles) * @param schema IndexSchema for field bindings + * @param positionalArgs List of positional arguments */ - public SolrBindings(String scoreKey, Map expressions, IndexSchema schema) { + public SolrBindings( + String scoreKey, IndexSchema schema, List positionalArgs) { this.scoreKey = scoreKey; - this.expressions = expressions; this.schema = schema; - // :TODO: add support for request time bindings (function args) + this.positionalArgs = positionalArgs != null ? positionalArgs : new ArrayList<>(); } @Override @@ -220,14 +134,13 @@ public DoubleValuesSource getDoubleValuesSource(String key) { return DoubleValuesSource.SCORES; } - Expression expr = expressions.get(key); - if (null != expr) { + // Check for positional arguments like $1, $2, etc. + if (key.startsWith("$")) { try { - return expr.getDoubleValuesSource(this); - } catch (IllegalArgumentException e) { - throw new IllegalArgumentException( - "Invalid binding for key '" + key + "' transative binding problem: " + e.getMessage(), - e); + int position = Integer.parseInt(key.substring(1)); + return positionalArgs.get(position - 1); // Convert to 0-based index + } catch (RuntimeException e) { + throw new IllegalArgumentException("Not a valid positional argument: " + key, e); } } diff --git a/solr/core/src/test-files/solr/collection1/conf/solrconfig-expressions-vs.xml b/solr/core/src/test-files/solr/collection1/conf/solrconfig-expressions-vs.xml index 64658c9ae7c..46b8756c9ff 100644 --- a/solr/core/src/test-files/solr/collection1/conf/solrconfig-expressions-vs.xml +++ b/solr/core/src/test-files/solr/collection1/conf/solrconfig-expressions-vs.xml @@ -27,26 +27,50 @@ - - + + - - sin(1) - cos(sin1) - sqrt(int1_i) - sqrt(double1_d) - date1_dt - 631036800000 - 1 + score - 1 + one_plus_score - sqrt_int1_i + one_plus_score - one_plus_score*ln(cos_sin1) - + sin(1) + + + + cos(sin(1)) + + + + sqrt(int1_i) + + + + sqrt(double1_d) + + + + date1_dt - 631036800000 + + + + 1 + score + + + + 1 + score + 1 + + + + sqrt(int1_i) + 1 + score + + + + (1 + score)*ln(cos(sin(1))) ssccoorree - - 1 + ssccoorree - + 1 + ssccoorree + + + + $1 + $2 + $3 diff --git a/solr/core/src/test/org/apache/solr/search/ExpressionValueSourceParserTest.java b/solr/core/src/test/org/apache/solr/search/ExpressionValueSourceParserTest.java index 3370db0f6fd..483422611bc 100644 --- a/solr/core/src/test/org/apache/solr/search/ExpressionValueSourceParserTest.java +++ b/solr/core/src/test/org/apache/solr/search/ExpressionValueSourceParserTest.java @@ -17,13 +17,11 @@ package org.apache.solr.search; import java.text.ParseException; -import java.util.HashMap; -import java.util.Map; -import org.apache.lucene.expressions.Expression; -import org.apache.lucene.expressions.js.JavascriptCompiler; +import java.util.ArrayList; +import java.util.List; import org.apache.lucene.queries.function.ValueSource; +import org.apache.lucene.search.DoubleValuesSource; import org.apache.solr.SolrTestCaseJ4; -import org.apache.solr.common.SolrException; import org.apache.solr.schema.IndexSchema; import org.apache.solr.search.ExpressionValueSourceParser.SolrBindings; import org.apache.solr.util.DateMathParser; @@ -32,6 +30,8 @@ public class ExpressionValueSourceParserTest extends SolrTestCaseJ4 { + private final List positionalArgs = new ArrayList<>(); + // TODO need "bad-solrconfig..." level test of cycle expressions, using score w/null score // binding, etc... @@ -54,10 +54,7 @@ public static void beforeTests() throws Exception { public void testValidBindings() throws ParseException { IndexSchema schema = h.getCore().getLatestSchema(); - Map exprs = new HashMap<>(); - exprs.put("foo", JavascriptCompiler.compile("((0.3*popularity)/10.0)+(0.7*ScOrE)")); - exprs.put("popularity", JavascriptCompiler.compile("foo_i")); - SolrBindings bindings = new SolrBindings("ScOrE", exprs, schema); + SolrBindings bindings = new SolrBindings("ScOrE", schema, this.positionalArgs); assertEquals( "foo_i from bindings is wrong", @@ -66,8 +63,6 @@ public void testValidBindings() throws ParseException { .getValueSource(schema.getField("foo_i"), null) .asDoubleValuesSource(), bindings.getDoubleValuesSource("foo_i")); - assertNotNull("pop bindings failed", bindings.getDoubleValuesSource("popularity")); - assertNotNull("foo bindings failed", bindings.getDoubleValuesSource("foo")); ValueSource scoreBind = ValueSource.fromDoubleValuesSource(bindings.getDoubleValuesSource("ScOrE")); assertNotNull("ScOrE bindings failed", scoreBind); @@ -82,19 +77,18 @@ public void testValidBindings() throws ParseException { } // change things up a bit - - exprs.put("ScOrE", JavascriptCompiler.compile("42")); - - bindings = new SolrBindings(null, exprs, schema); - assertNotNull("foo bindings failed", bindings.getDoubleValuesSource("foo")); + bindings = new SolrBindings(null, schema, this.positionalArgs); + try { + bindings.getDoubleValuesSource("ScOrE"); + fail("ScOrE should not have bindings"); + } catch (IllegalArgumentException e) { + // NOOP + } } public void testBogusBindings() throws ParseException { IndexSchema schema = h.getCore().getLatestSchema(); - Map exprs = new HashMap<>(); - exprs.put("foo", JavascriptCompiler.compile("((0.3*popularity)/10.0)+(0.7*ScOrE)")); - exprs.put("popularity", JavascriptCompiler.compile("yak")); - SolrBindings bindings = new SolrBindings("ScOrE", exprs, schema); + SolrBindings bindings = new SolrBindings("ScOrE", schema, this.positionalArgs); try { bindings.getDoubleValuesSource("yak"); @@ -103,19 +97,8 @@ public void testBogusBindings() throws ParseException { // NOOP } - try { - bindings.getDoubleValuesSource("foo"); - fail("foo should be an invalid transative binding"); - } catch (IllegalArgumentException e) { - String err = e.getMessage(); - assertTrue( - "wrong exception message: " + err, - (err.contains("foo") && err.contains("popularity") && err.contains("yak"))); - } - // change things up a bit - - bindings = new SolrBindings(null, exprs, schema); + bindings = new SolrBindings(null, schema, this.positionalArgs); try { bindings.getDoubleValuesSource("ScOrE"); fail("ScOrE should not have bindings"); @@ -130,70 +113,6 @@ public void testBogusBindings() throws ParseException { } } - /** - * @see ExpressionValueSourceParser#exceptionIfCycles(Map) - */ - public void testCycleChecker() throws Exception { - Map exprs = new HashMap<>(); - - exprs.put("foo", JavascriptCompiler.compile("bar * ack")); - ExpressionValueSourceParser.exceptionIfCycles(exprs); - - exprs.put("bar", JavascriptCompiler.compile("ack * ack")); - ExpressionValueSourceParser.exceptionIfCycles(exprs); - - exprs.put("baz", JavascriptCompiler.compile("bar * foo")); - ExpressionValueSourceParser.exceptionIfCycles(exprs); - - // now we start to get some errors - exprs.put("ack", JavascriptCompiler.compile("ack * ack")); - - try { - ExpressionValueSourceParser.exceptionIfCycles(exprs); - fail("no error about ack depending on ack"); - } catch (SolrException e) { - assertTrue(e.getMessage(), e.getMessage().contains("ack=>ack")); - } - - // different, deeper error - exprs.put("ack", JavascriptCompiler.compile("foo * 2")); - - try { - ExpressionValueSourceParser.exceptionIfCycles(exprs); - fail("no error about ack/foo/bar"); - } catch (SolrException e) { - String msg = e.getMessage(); - // the thing about cycles: they might be found in either order - assertTrue(msg, msg.contains("foo=>ack") || msg.contains("ack=>foo")); - assertTrue(msg, msg.contains("bar=>ack")); - } - - exprs.remove("bar"); - exprs.put("wack", JavascriptCompiler.compile("sqrt(wack)")); - - try { - ExpressionValueSourceParser.exceptionIfCycles(exprs); - fail("no error about ack depending on ack"); - } catch (SolrException e) { - String msg = e.getMessage(); - // the thing about cycles: they might be found in either order - assertTrue(msg, msg.contains("foo=>ack") || msg.contains("ack=>foo")); - assertTrue(msg, msg.contains("wack=>wack")); - assertTrue(msg, msg.contains("At least 2 cycles")); - } - } - - /** tests clean error when no such binding */ - public void testNoSuchBinding() { - assertQEx( - "should have gotten user error for invalid binding", - req( - "fl", "id", - "q", "{!func}field(int1_i)", - "sort", "expr(this_expression_is_not_bound) desc"), - 400); - } - /** tests an expression referring to a score field using an overridden score binding */ @Ignore("SOLR-XXXX can't sort by expression referencing the score") public void testSortSsccoorree() { @@ -215,7 +134,7 @@ public void testSortSsccoorree() { public void testSortConstant() { assertQ( "sort", - req("fl", "id", "q", "*:*", "sort", "expr(sin1) desc,id asc"), + req("fl", "id", "q", "*:*", "sort", "sin1() desc,id asc"), "//*[@numFound='5']", "//result/doc[1]/str[@name='id'][.='1']", "//result/doc[2]/str[@name='id'][.='2']", @@ -224,24 +143,13 @@ public void testSortConstant() { "//result/doc[5]/str[@name='id'][.='5']"); } - /** tests an expression referring to another expression */ - public void testSortExpression() { - assertQ( - "sort", - req("fl", "id", "q", "*:*", "sort", "expr(cos_sin1) desc,id asc"), - "//*[@numFound='5']", - "//result/doc[1]/str[@name='id'][.='1']", - "//result/doc[2]/str[@name='id'][.='2']", - "//result/doc[3]/str[@name='id'][.='3']", - "//result/doc[4]/str[@name='id'][.='4']", - "//result/doc[5]/str[@name='id'][.='5']"); - } + // Removed testSortExpression as expressions can no longer reference other expressions /** tests an expression referring to an int field */ public void testSortInt() { assertQ( "sort", - req("fl", "id", "q", "*:*", "sort", "expr(sqrt_int1_i) desc,id asc"), + req("fl", "id", "q", "*:*", "sort", "sqrt_int1_i() desc,id asc"), "//*[@numFound='5']", "//result/doc[1]/str[@name='id'][.='2']", // NaN "//result/doc[2]/str[@name='id'][.='1']", @@ -254,7 +162,7 @@ public void testSortInt() { public void testSortDouble() { assertQ( "sort", - req("fl", "id", "q", "*:*", "sort", "expr(sqrt_double1_d) desc,id asc"), + req("fl", "id", "q", "*:*", "sort", "sqrt_double1_d() desc,id asc"), "//*[@numFound='5']", "//result/doc[1]/str[@name='id'][.='1']", // NaN "//result/doc[2]/str[@name='id'][.='4']", // NaN @@ -267,7 +175,7 @@ public void testSortDouble() { public void testSortDate() { assertQ( "sort", - req("fl", "id", "q", "*:*", "sort", "expr(date1_dt_minus_1990) desc,id asc"), + req("fl", "id", "q", "*:*", "sort", "date1_dt_minus_1990() desc,id asc"), "//*[@numFound='5']", "//result/doc[1]/str[@name='id'][.='2']", "//result/doc[2]/str[@name='id'][.='5']", @@ -281,7 +189,7 @@ public void testSortDate() { public void testSortScore() { assertQ( "sort", - req("fl", "id", "q", "{!func}field(int1_i)", "sort", "expr(one_plus_score) desc,id asc"), + req("fl", "id", "q", "{!func}field(int1_i)", "sort", "one_plus_score() desc,id asc"), "//*[@numFound='5']", "//result/doc[1]/str[@name='id'][.='1']", "//result/doc[2]/str[@name='id'][.='4']", @@ -290,60 +198,12 @@ public void testSortScore() { "//result/doc[5]/str[@name='id'][.='2']"); } - /** tests an expression referring to another expression with externals */ - @Ignore("SOLR-XXXX can't sort by expression referencing the score") - public void testSortScore2() { - assertQ( - "sort", - req("fl", "id", "q", "{!func}field(int1_i)", "sort", "expr(two_plus_score) desc,id asc"), - "//*[@numFound='5']", - "//result/doc[1]/str[@name='id'][.='1']", - "//result/doc[2]/str[@name='id'][.='4']", - "//result/doc[3]/str[@name='id'][.='5']", - "//result/doc[4]/str[@name='id'][.='3']", - "//result/doc[5]/str[@name='id'][.='2']"); - } - - /** tests an expression referring to two expressions with externals */ - @Ignore("SOLR-XXXX can't sort by expression referencing the score") - public void testSortScore3() { - assertQ( - "sort", - req( - "fl", - "id", - "q", - "{!func}field(int1_i)", - "sort", - "expr(sqrt_int1_i_plus_one_plus_score) desc,id asc"), - "//*[@numFound='5']", - "//result/doc[1]/str[@name='id'][.='2']", - "//result/doc[2]/str[@name='id'][.='1']", - "//result/doc[3]/str[@name='id'][.='4']", - "//result/doc[4]/str[@name='id'][.='5']", - "//result/doc[5]/str[@name='id'][.='3']"); - } - - /** tests an expression referring to another expression and a function */ - @Ignore("SOLR-XXXX can't sort by expression referencing the score") - public void testSortMixed() { - assertQ( - "sort", - req("fl", "id", "q", "{!func}field(int1_i)", "sort", "expr(mixed_expr) desc,id asc"), - "//*[@numFound='5']", - "//result/doc[1]/str[@name='id'][.='2']", - "//result/doc[2]/str[@name='id'][.='3']", - "//result/doc[3]/str[@name='id'][.='5']", - "//result/doc[4]/str[@name='id'][.='4']", - "//result/doc[5]/str[@name='id'][.='1']"); - } - /** tests a constant expression */ public void testReturnConstant() { final float expected = (float) Math.sin(1); assertQ( "return", - req("fl", "sin1:expr(sin1)", "q", "*:*", "sort", "id asc"), + req("fl", "sin1:sin1()", "q", "*:*", "sort", "id asc"), "//*[@numFound='5']", "//result/doc[1]/float[@name='sin1'][.='" + expected + "']", "//result/doc[2]/float[@name='sin1'][.='" + expected + "']", @@ -352,25 +212,13 @@ public void testReturnConstant() { "//result/doc[5]/float[@name='sin1'][.='" + expected + "']"); } - /** tests an expression referring to another expression */ - public void testReturnExpression() { - final float expected = (float) Math.cos(Math.sin(1)); - assertQ( - "sort", - req("fl", "expr(cos_sin1)", "q", "*:*", "sort", "id asc"), - "//*[@numFound='5']", - "//result/doc[1]/float[@name='expr(cos_sin1)'][.=" + expected + "]", - "//result/doc[2]/float[@name='expr(cos_sin1)'][.=" + expected + "]", - "//result/doc[3]/float[@name='expr(cos_sin1)'][.=" + expected + "]", - "//result/doc[4]/float[@name='expr(cos_sin1)'][.=" + expected + "]", - "//result/doc[5]/float[@name='expr(cos_sin1)'][.=" + expected + "]"); - } + // Removed testReturnExpression as expressions can no longer reference other expressions /** tests an expression referring to an int field */ public void testReturnInt() { assertQ( "return", - req("fl", "foo:expr(sqrt_int1_i)", "q", "*:*", "sort", "id asc"), + req("fl", "foo:sqrt_int1_i()", "q", "*:*", "sort", "id asc"), "//*[@numFound='5']", "//result/doc[1]/float[@name='foo'][.=" + (float) Math.sqrt(50) + "]", "//result/doc[2]/float[@name='foo'][.='NaN']", @@ -383,7 +231,7 @@ public void testReturnInt() { public void testReturnDouble() { assertQ( "return", - req("fl", "bar:expr(sqrt_double1_d)", "q", "*:*", "sort", "id asc"), + req("fl", "bar:sqrt_double1_d()", "q", "*:*", "sort", "id asc"), "//*[@numFound='5']", "//result/doc[1]/float[@name='bar'][.='NaN']", "//result/doc[2]/float[@name='bar'][.=" + (float) Math.sqrt(10.3d) + "]", @@ -396,7 +244,7 @@ public void testReturnDouble() { public void testReturnDate() { assertQ( "return", - req("fl", "date1_dt_minus_1990:expr(date1_dt_minus_1990)", "q", "*:*", "sort", "id asc"), + req("fl", "date1_dt_minus_1990:date1_dt_minus_1990()", "q", "*:*", "sort", "id asc"), "//*[@numFound='5']", "//result/doc[1]/float[@name='date1_dt_minus_1990'][.='" + (float) @@ -426,15 +274,15 @@ public void testReturnScores() { "return", // :nocommit: see ValueSourceAugmenter's nocommit for why fl needs "score" req( - "fl", "expr(one_plus_score),score", + "fl", "one_plus_score(),score", "q", "{!func}field(int1_i)", "sort", "id asc"), "//*[@numFound='5']", - "//result/doc[1]/float[@name='expr(one_plus_score)'][.='51.0']", - "//result/doc[2]/float[@name='expr(one_plus_score)'][.='1.0']", - "//result/doc[3]/float[@name='expr(one_plus_score)'][.='11.0']", - "//result/doc[4]/float[@name='expr(one_plus_score)'][.='41.0']", - "//result/doc[5]/float[@name='expr(one_plus_score)'][.='21.0']"); + "//result/doc[1]/float[@name='one_plus_score()'][.='51.0']", + "//result/doc[2]/float[@name='one_plus_score()'][.='1.0']", + "//result/doc[3]/float[@name='one_plus_score()'][.='11.0']", + "//result/doc[4]/float[@name='one_plus_score()'][.='41.0']", + "//result/doc[5]/float[@name='one_plus_score()'][.='21.0']"); } public void testReturnScores2() { @@ -442,7 +290,7 @@ public void testReturnScores2() { "return", // :nocommit: see ValueSourceAugmenter's nocommit for why fl needs "score" req( - "fl", "two_plus_score:expr(two_plus_score),score", + "fl", "two_plus_score:two_plus_score(),score", "q", "{!func}field(int1_i)", "sort", "id asc"), "//*[@numFound='5']", @@ -458,7 +306,7 @@ public void testReturnScores3() { "return", // :nocommit: see ValueSourceAugmenter's nocommit for why fl needs "score" req( - "fl", "foo:expr(sqrt_int1_i_plus_one_plus_score),score", + "fl", "foo:sqrt_int1_i_plus_one_plus_score(),score", "q", "{!func}field(int1_i)", "sort", "id asc"), "//*[@numFound='5']", @@ -468,4 +316,37 @@ public void testReturnScores3() { "//result/doc[4]/float[@name='foo'][.='" + (float) (Math.sqrt(40) + 1 + 40) + "']", "//result/doc[5]/float[@name='foo'][.='" + (float) (Math.sqrt(20) + 1 + 20) + "']"); } + + /** tests an expression with positional arguments */ + public void testPositionalArgs() { + assertQ( + "return", + req( + "fl", "sum:positional_args(10,20,30)", + "q", "*:*", + "sort", "id asc"), + "//*[@numFound='5']", + "//result/doc[1]/float[@name='sum'][.='60.0']", + "//result/doc[2]/float[@name='sum'][.='60.0']", + "//result/doc[3]/float[@name='sum'][.='60.0']", + "//result/doc[4]/float[@name='sum'][.='60.0']", + "//result/doc[5]/float[@name='sum'][.='60.0']"); + } + + /** tests embedding an expression in another value source */ + public void testEmbeddedExpression() { + final float expected = (float) Math.sin(1); + assertQ( + "return", + req( + "fl", "embedded:sum(sin1(),5)", + "q", "*:*", + "sort", "id asc"), + "//*[@numFound='5']", + "//result/doc[1]/float[@name='embedded'][.='" + (expected + 5.0f) + "']", + "//result/doc[2]/float[@name='embedded'][.='" + (expected + 5.0f) + "']", + "//result/doc[3]/float[@name='embedded'][.='" + (expected + 5.0f) + "']", + "//result/doc[4]/float[@name='embedded'][.='" + (expected + 5.0f) + "']", + "//result/doc[5]/float[@name='embedded'][.='" + (expected + 5.0f) + "']"); + } } diff --git a/solr/solr-ref-guide/modules/query-guide/pages/expression-value-source-parser.adoc b/solr/solr-ref-guide/modules/query-guide/pages/expression-value-source-parser.adoc index 7271b9f5785..3e1b3a83ae8 100644 --- a/solr/solr-ref-guide/modules/query-guide/pages/expression-value-source-parser.adoc +++ b/solr/solr-ref-guide/modules/query-guide/pages/expression-value-source-parser.adoc @@ -16,97 +16,41 @@ // specific language governing permissions and limitations // under the License. -The `ExpressionValueSourceParser` allows you to configure named JavaScript expressions in your `solrconfig.xml` that can be referenced in function queries. -This provides a powerful way to define reusable expressions that can be combined and referenced in queries. -The expressions are based on the Lucene Expressions module, which you can learn more about in the {lucene-javadocs}/expressions/org/apache/lucene/expressions/js/package-summary.html[Lucene Expressions documentation]. +The `ExpressionValueSourceParser` allows you to implement a custom valueSource merely by adding a concise JavaScript expression to your `solrconfig.xml`. +The expression is precompiled, and offers competitive performance to those written in Java. +The syntax is a limited subset of JavaScript that is purely numerical oriented, and only certain built-in fuctions can be called. +The implementation is based on the Lucene Expressions module, which you can learn more about in the {lucene-javadocs}/expressions/org/apache/lucene/expressions/js/package-summary.html[Lucene Expressions documentation]. -== Configuration +== Examples -To use the `ExpressionValueSourceParser`, you need to configure it in your `solrconfig.xml` file: +Expressions can reference field values directly by field name, the document's score using the special name `score`, and positional arguments as `$1`, `$2`, etc. +The arguments might be constants, fields, or other functions supplying a result. -[source,xml] ----- - - - sin(1) - cos(sin1) - sqrt(int1_i) - 1 + score - - ----- - -The configuration consists of: - -* A `name` attribute that defines the parser name to be used in queries (e.g., `expr`) -* A `class` attribute that specifies the implementation class (`solr.ExpressionValueSourceParser`) -* A list of named expressions in the `expressions` element, where each expression is defined with a name and a JavaScript expression - -The parser binds the special name `score` to the document's score. - -== Using Expressions - -Once configured, you can use the named expressions in xref:function-queries.adoc[function queries] in any place that function queries are supported. -This first example shows it used for sorting: - -[source,text] ----- -q=*:*&sort=expr(sqrt_int1_i) desc ----- - -This example shows it in the field list to return calculated values: - -[source,text] ----- -fl=id,expr(one_plus_score),score ----- - - - -== Score Manipulation with Expressions - -A particularly powerful feature of the Expression Value Source Parser is the ability to reference the document score in expressions. -This allows for convenient score manipulation and boosting in a more natural way. -For example, you can create expressions that combine the score with other factors: +Here are some example definitions designed to illustrate these features: [source,xml] ---- - - - sqrt(popularity) * score - + + sqrt(popularity) ----- - -Using expressions to manipulate scores is more efficient than using the "query" function query when you need to reference the score. -The "query" function effectively executes the score calculation twice internally, whereas expressions can reference the score directly. -== Expression Features - -* Expressions can reference other expressions defined in the same parser -* Expressions can reference field values directly by field name -* Expressions can use the document's score -* Expressions support standard JavaScript math operations and functions -* The parser automatically detects and prevents circular references in expressions - -== Example - -This example shows how to configure expressions that build on each other: + + $1 * 0.8 + $2 * 0.2 + -[source,xml] ----- - - - sqrt(popularity) - log(price) - sqrt_popularity / log_price - + + log(sum(popularity,recency)) * max(score,0.1) ---- -You can then use these expressions in queries: +Here is one unrealistic query using multiple function queries to illustrate its features: [source,text] ---- -q=title:solr&sort=expr(boost_formula) desc +&q=my query +&fq={!frange l=1000}sqrt_popularity() +&sort=complex_score() desc +&fl=id,weighted_sum(field1,field2) ---- + +Using this VSP to boost/manipulate scores is more efficient than using the `query()` function query, since the latter would execute the underlying `q` an additional time. From 164603d79afef8bbc62f05bb37912a5a4059a8f0 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Tue, 24 Jun 2025 23:43:40 -0400 Subject: [PATCH 09/14] refer to it in function-queries.adoc --- .../modules/query-guide/pages/function-queries.adoc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/solr/solr-ref-guide/modules/query-guide/pages/function-queries.adoc b/solr/solr-ref-guide/modules/query-guide/pages/function-queries.adoc index 7c6f1a9d0ea..f361faf884f 100644 --- a/solr/solr-ref-guide/modules/query-guide/pages/function-queries.adoc +++ b/solr/solr-ref-guide/modules/query-guide/pages/function-queries.adoc @@ -25,6 +25,8 @@ The functions can be a constant (numeric or string literal), a field, another fu You can use these functions to modify the ranking of results for users. These could be used to change the ranking of results based on a user's location, or some other calculation. + + == Using Function Query Functions must be expressed as function calls (for example, `sum(a,b)` instead of simply `a+b`). @@ -86,6 +88,8 @@ Only functions with fast random access are recommended. The table below summarizes the functions available for function queries. +Additionally, you can write a custom one using a tiny bit of JavaScript with the xref:expression-value-source-parser.adoc[Expression Value Source Parser]. + === abs Function Returns the absolute value of the specified value or function. From c634796d84412369d47124fc0c08d7e4f7b29b7e Mon Sep 17 00:00:00 2001 From: David Smiley Date: Wed, 25 Jun 2025 00:42:27 -0400 Subject: [PATCH 10/14] CHANGES.txt --- solr/CHANGES.txt | 12 ++++++++---- .../solr/search/ExpressionValueSourceParser.java | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 275a8f734fe..681b75b9e95 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -194,10 +194,6 @@ Other Changes ================== 9.9.0 ================== New Features --------------------- -* SOLR-17582: The CLUSTERSTATUS API will now stream each collection's status to the response, - fetching and computing it on the fly. To avoid a backwards compatibility concern, this won't work - for wt=javabin. (Matthew Biscocho, David Smiley) - * SOLR-17626: Add RawTFSimilarityFactory class. (Christine Poerschke) * SOLR-17656: New 'skipLeaderRecovery' replica property allows PULL replicas with existing indexes to immediately become ACTIVE (hossman) @@ -214,6 +210,10 @@ New Features * SOLR-17749: Added linear function support for RankField via RankQParserPlugin. (Christine Poerschke) +* SOLR-5707: New ExpressionValueSourceParser that allows custom function queries / VSPs to be defined in a + subset of JavaScript, pre-compiled, and that which can access the score and fields. It's powered by + the Lucene Expressions module. (hossman, eeeeDavid Smiley, Ryan Ernst, Kevin Risden) + Improvements --------------------- * SOLR-15751: The v2 API now has parity with the v1 "COLSTATUS" and "segments" APIs, which can be used to fetch detailed information about @@ -250,6 +250,10 @@ Improvements Optimizations --------------------- +* SOLR-17582: The CLUSTERSTATUS API will now stream each collection's status to the response, + fetching and computing it on the fly. To avoid a backwards compatibility concern, this won't work + for wt=javabin. (Matthew Biscocho, David Smiley) + * SOLR-17578: Remove ZkController internal core supplier, for slightly faster reconnection after Zookeeper session loss. (Pierre Salagnac) * SOLR-17669: Reduced memory usage in SolrJ getBeans() method when handling dynamic fields with wildcards. (Martin Anzinger) diff --git a/solr/core/src/java/org/apache/solr/search/ExpressionValueSourceParser.java b/solr/core/src/java/org/apache/solr/search/ExpressionValueSourceParser.java index c27aaa3e837..3812357c3d9 100644 --- a/solr/core/src/java/org/apache/solr/search/ExpressionValueSourceParser.java +++ b/solr/core/src/java/org/apache/solr/search/ExpressionValueSourceParser.java @@ -37,7 +37,7 @@ /** * A ValueSource parser configured with a pre-compiled expression that can then be evaluated at - * request time. It's powered by the Lucene Expressions module, which is a subset of JavaScript + * request time. It's powered by the Lucene Expressions module, which is a subset of JavaScript. */ public class ExpressionValueSourceParser extends ValueSourceParser { From d6a388e3b1088d36f2580b40ab5ebca9b896d37d Mon Sep 17 00:00:00 2001 From: David Smiley Date: Mon, 7 Jul 2025 22:31:39 -0400 Subject: [PATCH 11/14] un-ignore tests --- .../search/ExpressionValueSourceParserTest.java | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/solr/core/src/test/org/apache/solr/search/ExpressionValueSourceParserTest.java b/solr/core/src/test/org/apache/solr/search/ExpressionValueSourceParserTest.java index 483422611bc..97f9c4d7418 100644 --- a/solr/core/src/test/org/apache/solr/search/ExpressionValueSourceParserTest.java +++ b/solr/core/src/test/org/apache/solr/search/ExpressionValueSourceParserTest.java @@ -26,15 +26,11 @@ import org.apache.solr.search.ExpressionValueSourceParser.SolrBindings; import org.apache.solr.util.DateMathParser; import org.junit.BeforeClass; -import org.junit.Ignore; public class ExpressionValueSourceParserTest extends SolrTestCaseJ4 { private final List positionalArgs = new ArrayList<>(); - // TODO need "bad-solrconfig..." level test of cycle expressions, using score w/null score - // binding, etc... - @BeforeClass public static void beforeTests() throws Exception { initCore("solrconfig-expressions-vs.xml", "schema15.xml"); @@ -114,14 +110,13 @@ public void testBogusBindings() throws ParseException { } /** tests an expression referring to a score field using an overridden score binding */ - @Ignore("SOLR-XXXX can't sort by expression referencing the score") public void testSortSsccoorree() { assertQ( "sort", req( "fl", "id", "q", "{!func}field(int1_i)", - "sort", "expr_ssccoorree(one_plus_score) desc,id asc"), + "sort", "expr_ssccoorree() desc,id asc"), "//*[@numFound='5']", "//result/doc[1]/str[@name='id'][.='1']", "//result/doc[2]/str[@name='id'][.='4']", @@ -185,7 +180,6 @@ public void testSortDate() { } /** tests an expression referring to a score field */ - @Ignore("SOLR-XXXX can't sort by expression referencing the score") public void testSortScore() { assertQ( "sort", @@ -272,7 +266,7 @@ public void testReturnDate() { public void testReturnScores() { assertQ( "return", - // :nocommit: see ValueSourceAugmenter's nocommit for why fl needs "score" + // unfortunately, need to add fl=score for ValueSourceAugmenter to access it req( "fl", "one_plus_score(),score", "q", "{!func}field(int1_i)", @@ -288,7 +282,7 @@ public void testReturnScores() { public void testReturnScores2() { assertQ( "return", - // :nocommit: see ValueSourceAugmenter's nocommit for why fl needs "score" + // unfortunately, need to add fl=score for ValueSourceAugmenter to access it req( "fl", "two_plus_score:two_plus_score(),score", "q", "{!func}field(int1_i)", @@ -304,7 +298,7 @@ public void testReturnScores2() { public void testReturnScores3() { assertQ( "return", - // :nocommit: see ValueSourceAugmenter's nocommit for why fl needs "score" + // unfortunately, need to add fl=score for ValueSourceAugmenter to access it req( "fl", "foo:sqrt_int1_i_plus_one_plus_score(),score", "q", "{!func}field(int1_i)", From 8234a814c0df74ffd9bb662413f721a482cb72e2 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Wed, 9 Jul 2025 01:00:25 -0400 Subject: [PATCH 12/14] fix lucene "expressions" javadoc reference in ref guide --- gradle/documentation/pull-lucene-javadocs.gradle | 9 +++++---- solr/documentation/gradle.lockfile | 1 + solr/solr-ref-guide/gradle.lockfile | 2 +- solr/solrj-streaming/gradle.lockfile | 2 +- solr/solrj/gradle.lockfile | 2 +- 5 files changed, 9 insertions(+), 7 deletions(-) diff --git a/gradle/documentation/pull-lucene-javadocs.gradle b/gradle/documentation/pull-lucene-javadocs.gradle index e3fcaecef25..aa6990ebf8b 100644 --- a/gradle/documentation/pull-lucene-javadocs.gradle +++ b/gradle/documentation/pull-lucene-javadocs.gradle @@ -41,11 +41,12 @@ configure(project(":solr:documentation")) { // - For now this list is focused solely on the javadocs needed for ref-guide link validation. // - If/when additional links are added from the ref-guide to additional lucene modules not listed here, // they can be added. - // - If/when we need the lucene javadocs for "all" lucene depdencies in Solr (ie: to do link checking - // from all Solr javadocs?) then perhaps we can find a way to build this list programatically? - // - If these javadocs are (only every) consumed by the ref guide only, then these deps & associated tasks + // - If/when we need the lucene javadocs for "all" lucene dependencies in Solr (ie: to do link checking + // from all Solr javadocs?) then perhaps we can find a way to build this list programmatically? + // - If these javadocs are only consumed by the ref guide, then these deps & associated tasks // should just be moved to the ref-guide build.gradle javadocs variantOf(libs.apache.lucene.core) { classifier 'javadoc' } + javadocs variantOf(libs.apache.lucene.expressions) { classifier 'javadoc' } javadocs variantOf(libs.apache.lucene.analysis.common) { classifier 'javadoc' } javadocs variantOf(libs.apache.lucene.analysis.stempel) { classifier 'javadoc' } javadocs variantOf(libs.apache.lucene.queryparser) { classifier 'javadoc' } @@ -65,7 +66,7 @@ configure(project(":solr:documentation")) { def resolved = configurations.javadocs.resolvedConfiguration resolved.resolvedArtifacts.each { artifact -> def id = artifact.moduleVersion.id - // This mimics the directory stucture used on lucene.apache.org for the javadocs of all modules. + // This mimics the directory structure used on lucene.apache.org for the javadocs of all modules. // // HACK: the lucene.apache.org javadocs are organized to match the module directory structure in the repo, // not the "flat" artifact names -- so there is no one size fits all way to determine the directory name. diff --git a/solr/documentation/gradle.lockfile b/solr/documentation/gradle.lockfile index 64bcc86b277..29100635f9f 100644 --- a/solr/documentation/gradle.lockfile +++ b/solr/documentation/gradle.lockfile @@ -4,6 +4,7 @@ org.apache.lucene:lucene-analysis-common:9.12.2=javadocs org.apache.lucene:lucene-analysis-stempel:9.12.2=javadocs org.apache.lucene:lucene-core:9.12.2=javadocs +org.apache.lucene:lucene-expressions:9.12.2=javadocs org.apache.lucene:lucene-queries:9.12.2=javadocs org.apache.lucene:lucene-queryparser:9.12.2=javadocs org.apache.lucene:lucene-spatial-extras:9.12.2=javadocs diff --git a/solr/solr-ref-guide/gradle.lockfile b/solr/solr-ref-guide/gradle.lockfile index 83242de546c..9c18839eb18 100644 --- a/solr/solr-ref-guide/gradle.lockfile +++ b/solr/solr-ref-guide/gradle.lockfile @@ -89,7 +89,7 @@ org.apache.lucene:lucene-backward-codecs:9.12.2=testRuntimeClasspath org.apache.lucene:lucene-classification:9.12.2=testRuntimeClasspath org.apache.lucene:lucene-codecs:9.12.2=testRuntimeClasspath org.apache.lucene:lucene-core:9.12.2=localJavadocs,testCompileClasspath,testRuntimeClasspath -org.apache.lucene:lucene-expressions:9.12.2=testRuntimeClasspath +org.apache.lucene:lucene-expressions:9.12.2=localJavadocs,testRuntimeClasspath org.apache.lucene:lucene-facet:9.12.2=testRuntimeClasspath org.apache.lucene:lucene-grouping:9.12.2=testRuntimeClasspath org.apache.lucene:lucene-highlighter:9.12.2=testRuntimeClasspath diff --git a/solr/solrj-streaming/gradle.lockfile b/solr/solrj-streaming/gradle.lockfile index 141544a9f1b..943508f2eca 100644 --- a/solr/solrj-streaming/gradle.lockfile +++ b/solr/solrj-streaming/gradle.lockfile @@ -177,7 +177,7 @@ org.ow2.asm:asm:9.7.1=jarValidation,testRuntimeClasspath org.pcollections:pcollections:4.0.1=annotationProcessor,errorprone,testAnnotationProcessor org.semver4j:semver4j:5.3.0=jarValidation,permitTestUsedUndeclared,runtimeClasspath,testRuntimeClasspath org.slf4j:jcl-over-slf4j:2.0.16=jarValidation,permitTestUsedUndeclared,runtimeClasspath,testRuntimeClasspath -org.slf4j:slf4j-api:1.7.32=permitTestUnusedDeclared +org.slf4j:slf4j-api:1.7.30=permitTestUnusedDeclared org.slf4j:slf4j-api:2.0.16=compileClasspath,jarValidation,permitTestUsedUndeclared,runtimeClasspath,testCompileClasspath,testRuntimeClasspath org.xerial.snappy:snappy-java:1.1.10.5=jarValidation,testRuntimeClasspath empty=apiHelper,apiHelperTest,compileOnlyHelper,compileOnlyHelperTest,missingdoclet,permitAggregatorUse,permitTestAggregatorUse,permitUnusedDeclared,permitUsedUndeclared,signatures diff --git a/solr/solrj/gradle.lockfile b/solr/solrj/gradle.lockfile index e12274f0a74..cccf1fc6802 100644 --- a/solr/solrj/gradle.lockfile +++ b/solr/solrj/gradle.lockfile @@ -170,7 +170,7 @@ org.ow2.asm:asm:9.7.1=jarValidation,testRuntimeClasspath org.pcollections:pcollections:4.0.1=annotationProcessor,errorprone,testAnnotationProcessor org.semver4j:semver4j:5.3.0=apiHelper,jarValidation,runtimeClasspath,testRuntimeClasspath org.slf4j:jcl-over-slf4j:2.0.16=jarValidation,runtimeClasspath,testRuntimeClasspath -org.slf4j:slf4j-api:1.7.32=permitTestUnusedDeclared +org.slf4j:slf4j-api:1.7.30=permitTestUnusedDeclared org.slf4j:slf4j-api:2.0.16=apiHelper,compileClasspath,jarValidation,runtimeClasspath,testCompileClasspath,testRuntimeClasspath org.xerial.snappy:snappy-java:1.1.10.5=jarValidation,testRuntimeClasspath empty=apiHelperTest,compileOnlyHelperTest,missingdoclet,openApiSpecFile,permitAggregatorUse,permitTestAggregatorUse,permitTestUsedUndeclared,permitUnusedDeclared,permitUsedUndeclared,signatures From 992ba1a390b8da0b549f9293504353b0f7694a46 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Thu, 10 Jul 2025 17:44:14 -0400 Subject: [PATCH 13/14] code review feedback --- .../solr/collection1/conf/solrconfig-expressions-vs.xml | 2 +- .../query-guide/pages/expression-value-source-parser.adoc | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/solr/core/src/test-files/solr/collection1/conf/solrconfig-expressions-vs.xml b/solr/core/src/test-files/solr/collection1/conf/solrconfig-expressions-vs.xml index 46b8756c9ff..9081d634eff 100644 --- a/solr/core/src/test-files/solr/collection1/conf/solrconfig-expressions-vs.xml +++ b/solr/core/src/test-files/solr/collection1/conf/solrconfig-expressions-vs.xml @@ -29,7 +29,6 @@ - sin(1) @@ -66,6 +65,7 @@ + ssccoorree 1 + ssccoorree diff --git a/solr/solr-ref-guide/modules/query-guide/pages/expression-value-source-parser.adoc b/solr/solr-ref-guide/modules/query-guide/pages/expression-value-source-parser.adoc index 3e1b3a83ae8..ae9a9989a4e 100644 --- a/solr/solr-ref-guide/modules/query-guide/pages/expression-value-source-parser.adoc +++ b/solr/solr-ref-guide/modules/query-guide/pages/expression-value-source-parser.adoc @@ -17,8 +17,8 @@ // under the License. The `ExpressionValueSourceParser` allows you to implement a custom valueSource merely by adding a concise JavaScript expression to your `solrconfig.xml`. -The expression is precompiled, and offers competitive performance to those written in Java. -The syntax is a limited subset of JavaScript that is purely numerical oriented, and only certain built-in fuctions can be called. +The expression is precompiled and offers competitive performance to those written in Java. +The syntax is a limited subset of JavaScript that is purely numerically oriented, and only certain built-in functions can be called. The implementation is based on the Lucene Expressions module, which you can learn more about in the {lucene-javadocs}/expressions/org/apache/lucene/expressions/js/package-summary.html[Lucene Expressions documentation]. == Examples From b53e50f6f3852b8f30501e998db6aeda67576a5a Mon Sep 17 00:00:00 2001 From: David Smiley Date: Sun, 13 Jul 2025 16:34:18 -0400 Subject: [PATCH 14/14] typo/formatting --- solr/CHANGES.txt | 2 +- .../modules/query-guide/pages/function-queries.adoc | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index f50ada21c9c..8acd321b85d 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -241,7 +241,7 @@ New Features * SOLR-5707: New ExpressionValueSourceParser that allows custom function queries / VSPs to be defined in a subset of JavaScript, pre-compiled, and that which can access the score and fields. It's powered by - the Lucene Expressions module. (hossman, eeeeDavid Smiley, Ryan Ernst, Kevin Risden) + the Lucene Expressions module. (hossman, David Smiley, Ryan Ernst, Kevin Risden) Improvements --------------------- diff --git a/solr/solr-ref-guide/modules/query-guide/pages/function-queries.adoc b/solr/solr-ref-guide/modules/query-guide/pages/function-queries.adoc index f361faf884f..05abe1a114f 100644 --- a/solr/solr-ref-guide/modules/query-guide/pages/function-queries.adoc +++ b/solr/solr-ref-guide/modules/query-guide/pages/function-queries.adoc @@ -25,8 +25,6 @@ The functions can be a constant (numeric or string literal), a field, another fu You can use these functions to modify the ranking of results for users. These could be used to change the ranking of results based on a user's location, or some other calculation. - - == Using Function Query Functions must be expressed as function calls (for example, `sum(a,b)` instead of simply `a+b`).