Skip to content

Commit 759df4a

Browse files
dsmileydsmiley
authored
Refactoring: add QParser.parseAsValueSource (#3343)
It's common to need to parse a ValueSource from a QParser. Callers were typically doing the same thing including unfortunate instanceof checks. * Disable FunctionQParser.parseMultipleSources by default and deprecate it. Test the exception. Doesn't break any existing tests. This aspect is very exotic and could go away once we remove some old things relying on it.
1 parent b91905a commit 759df4a

11 files changed

Lines changed: 142 additions & 178 deletions

File tree

‎solr/core/src/java/org/apache/solr/handler/component/StatsField.java‎

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,8 @@
3030
import java.util.Map;
3131
import java.util.Set;
3232
import org.apache.lucene.index.LeafReaderContext;
33-
import org.apache.lucene.queries.function.FunctionQuery;
3433
import org.apache.lucene.queries.function.ValueSource;
3534
import org.apache.lucene.queries.function.valuesource.FieldCacheSource;
36-
import org.apache.lucene.queries.function.valuesource.QueryValueSource;
3735
import org.apache.lucene.search.Query;
3836
import org.apache.solr.common.SolrException;
3937
import org.apache.solr.common.SolrException.ErrorCode;
@@ -276,7 +274,7 @@ public StatsField(ResponseBuilder rb, String statsParam) {
276274
qplug.createParser(localParams.get(QueryParsing.V), localParams, params, rb.req);
277275

278276
// figure out what type of query we are dealing, get the most direct ValueSource
279-
vs = extractValueSource(qp.parse());
277+
vs = qp.parseAsValueSource();
280278

281279
// if this ValueSource directly corresponds to a SchemaField, act as if
282280
// we were asked to compute stats on it directly
@@ -338,23 +336,6 @@ public StatsField(ResponseBuilder rb, String statsParam) {
338336
: "exactly one of valueSource & schemaField must be null";
339337
}
340338

341-
/**
342-
* Inspects a {@link Query} to see if it directly maps to a {@link ValueSource}, and if so returns
343-
* it -- otherwise wraps it as needed.
344-
*
345-
* @param q Query whose scores we have been asked to compute stats of
346-
* @returns a ValueSource to use for computing the stats
347-
*/
348-
private static ValueSource extractValueSource(Query q) {
349-
return (q instanceof FunctionQuery)
350-
?
351-
// Common case: we're wrapping a func, so we can directly pull out ValueSource
352-
((FunctionQuery) q).getValueSource()
353-
:
354-
// asked to compute stats over a query, wrap it up as a ValueSource
355-
new QueryValueSource(q, 0.0F);
356-
}
357-
358339
/**
359340
* Inspects a {@link ValueSource} to see if it directly maps to a {@link SchemaField}, and if so
360341
* returns it.

‎solr/core/src/java/org/apache/solr/search/BoostQParserPlugin.java‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,8 @@
1616
*/
1717
package org.apache.solr.search;
1818

19-
import org.apache.lucene.queries.function.FunctionQuery;
2019
import org.apache.lucene.queries.function.FunctionScoreQuery;
2120
import org.apache.lucene.queries.function.ValueSource;
22-
import org.apache.lucene.queries.function.valuesource.QueryValueSource;
2321
import org.apache.lucene.search.Query;
2422
import org.apache.solr.common.params.SolrParams;
2523
import org.apache.solr.common.util.NamedList;
@@ -56,12 +54,7 @@ public Query parse() throws SyntaxError {
5654
Query q = baseParser.getQuery();
5755

5856
if (b == null) return q;
59-
Query bq = subQuery(b, FunctionQParserPlugin.NAME).getQuery();
60-
if (bq instanceof FunctionQuery) {
61-
vs = ((FunctionQuery) bq).getValueSource();
62-
} else {
63-
vs = new QueryValueSource(bq, 0.0f);
64-
}
57+
vs = subQuery(b, FunctionQParserPlugin.NAME).parseAsValueSource();
6558
return FunctionScoreQuery.boostByValue(q, vs.asDoubleValuesSource());
6659
}
6760

‎solr/core/src/java/org/apache/solr/search/ExtendedDismaxQParser.java‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import org.apache.lucene.analysis.TokenFilterFactory;
3030
import org.apache.lucene.analysis.core.StopFilterFactory;
3131
import org.apache.lucene.index.Term;
32-
import org.apache.lucene.queries.function.FunctionQuery;
3332
import org.apache.lucene.queries.function.FunctionScoreQuery;
3433
import org.apache.lucene.queries.function.ValueSource;
3534
import org.apache.lucene.queries.function.valuesource.ProductFloatFunction;
@@ -532,13 +531,11 @@ protected List<ValueSource> getMultiplicativeBoosts() throws SyntaxError {
532531
List<ValueSource> boosts = new ArrayList<>();
533532
if (config.hasMultiplicativeBoosts()) {
534533
for (String boostStr : config.multBoosts) {
535-
if (boostStr == null || boostStr.length() == 0) continue;
536-
Query boost = subQuery(boostStr, FunctionQParserPlugin.NAME).getQuery();
537-
ValueSource vs;
538-
if (boost instanceof FunctionQuery) {
539-
vs = ((FunctionQuery) boost).getValueSource();
540-
} else {
541-
vs = new QueryValueSource(boost, 1.0f);
534+
if (boostStr == null || boostStr.isEmpty()) continue;
535+
ValueSource vs = subQuery(boostStr, FunctionQParserPlugin.NAME).parseAsValueSource();
536+
// the default score should be 1, not 0
537+
if (vs instanceof QueryValueSource qvs && qvs.getDefaultValue() == 0.0f) {
538+
vs = new QueryValueSource(qvs.getQuery(), 1.0f);
542539
}
543540
boosts.add(vs);
544541
}

‎solr/core/src/java/org/apache/solr/search/FunctionQParser.java‎

Lines changed: 45 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import org.apache.lucene.queries.function.valuesource.ConstValueSource;
2727
import org.apache.lucene.queries.function.valuesource.DoubleConstValueSource;
2828
import org.apache.lucene.queries.function.valuesource.LiteralValueSource;
29-
import org.apache.lucene.queries.function.valuesource.QueryValueSource;
3029
import org.apache.lucene.queries.function.valuesource.VectorValueSource;
3130
import org.apache.lucene.search.Query;
3231
import org.apache.solr.common.params.ModifiableSolrParams;
@@ -36,6 +35,13 @@
3635
import org.apache.solr.search.facet.AggValueSource;
3736
import org.apache.solr.search.function.FieldNameValueSource;
3837

38+
/**
39+
* Does "function query" parsing of function-call like strings, producing a {@link ValueSource}. As
40+
* this implements {@link QParser}, we produce a {@link Query}, but more often {@link
41+
* #parseAsValueSource()} is called instead.
42+
*
43+
* @see ValueSourceParser
44+
*/
3945
public class FunctionQParser extends QParser {
4046

4147
public static final int FLAG_CONSUME_DELIMITER = 0x01; // consume delimiter after parsing arg
@@ -53,14 +59,27 @@ public class FunctionQParser extends QParser {
5359
*/
5460
public StrParser sp;
5561

56-
boolean parseMultipleSources = true;
57-
boolean parseToEnd = true;
62+
@Deprecated private boolean parseMultipleSources = false;
63+
private boolean parseToEnd = true;
5864

5965
public FunctionQParser(
6066
String qstr, SolrParams localParams, SolrParams params, SolrQueryRequest req) {
6167
super(qstr, localParams, params, req);
6268
setFlags(FLAG_DEFAULT);
6369
setString(qstr);
70+
if (localParams != null && localParams.getPrimitiveBool("multiple")) {
71+
setParseMultipleSources(true);
72+
}
73+
}
74+
75+
/**
76+
* Parses the string to a {@link ValueSource}. Typically, this is not used, however.
77+
*
78+
* @see QParser#parseAsValueSource()
79+
*/
80+
public static ValueSource parseAsValueSource(String string, SolrQueryRequest request)
81+
throws SyntaxError {
82+
return getParser(string, FunctionQParserPlugin.NAME, request).parseAsValueSource();
6483
}
6584

6685
@Override
@@ -71,11 +90,18 @@ public void setString(String s) {
7190
}
7291
}
7392

93+
@Deprecated
7494
public void setParseMultipleSources(boolean parseMultipleSources) {
7595
this.parseMultipleSources = parseMultipleSources;
7696
}
7797

78-
/** parse multiple comma separated value sources */
98+
/**
99+
* Parse multiple comma separated value sources encapsulated into a {@link VectorValueSource} when
100+
* {@link #getQuery()} or {@link #parseAsValueSource()} is called.
101+
*
102+
* @deprecated this is only needed for an unusual use-case and seems hard to support
103+
*/
104+
@Deprecated
79105
public boolean getParseMultipleSources() {
80106
return parseMultipleSources;
81107
}
@@ -86,11 +112,23 @@ public void setParseToEnd(boolean parseToEnd) {
86112

87113
/** throw exception if there is extra stuff at the end of the parsed valuesource(s). */
88114
public boolean getParseToEnd() {
89-
return parseMultipleSources;
115+
return parseToEnd;
90116
}
91117

92118
@Override
93119
public Query parse() throws SyntaxError {
120+
return new FunctionQuery(parseAsValueSource());
121+
}
122+
123+
/**
124+
* Parses as a ValueSource, not a Query. <em>NOT</em> intended to be called by {@link
125+
* ValueSourceParser#parse(FunctionQParser)}; it's intended for general code that has a {@link
126+
* QParser} but actually wants to parse a ValueSource.
127+
*
128+
* @return A {@link VectorValueSource} for multiple VS, otherwise just the single VS.
129+
*/
130+
@Override
131+
public ValueSource parseAsValueSource() throws SyntaxError {
94132
ValueSource vs = null;
95133
List<ValueSource> lst = null;
96134

@@ -126,8 +164,7 @@ public Query parse() throws SyntaxError {
126164
if (lst != null) {
127165
vs = new VectorValueSource(lst);
128166
}
129-
130-
return new FunctionQuery(vs);
167+
return vs;
131168
}
132169

133170
/**
@@ -428,14 +465,7 @@ protected ValueSource parseValueSource(int flags) throws SyntaxError {
428465
subFunc.setParseMultipleSources(true);
429466
subFunc.setFlags(flags);
430467
}
431-
Query subQuery = subParser.getQuery();
432-
if (subQuery == null) {
433-
valueSource = new ConstValueSource(0.0f);
434-
} else if (subQuery instanceof FunctionQuery) {
435-
valueSource = ((FunctionQuery) subQuery).getValueSource();
436-
} else {
437-
valueSource = new QueryValueSource(subQuery, 0.0f);
438-
}
468+
valueSource = subParser.parseAsValueSource();
439469
}
440470

441471
/*

‎solr/core/src/java/org/apache/solr/search/FunctionRangeQParserPlugin.java‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,7 @@
1616
*/
1717
package org.apache.solr.search;
1818

19-
import org.apache.lucene.queries.function.FunctionQuery;
2019
import org.apache.lucene.queries.function.ValueSource;
21-
import org.apache.lucene.queries.function.valuesource.QueryValueSource;
2220
import org.apache.lucene.search.Query;
2321
import org.apache.solr.common.params.SolrParams;
2422
import org.apache.solr.request.SolrQueryRequest;
@@ -50,12 +48,7 @@ public Query parse() throws SyntaxError {
5048
QParser subParser = subQuery(funcStr, FunctionQParserPlugin.NAME);
5149
subParser.setIsFilter(
5250
false); // the range can be based on the relevancy score of embedded queries.
53-
Query funcQ = subParser.getQuery();
54-
if (funcQ instanceof FunctionQuery) {
55-
vs = ((FunctionQuery) funcQ).getValueSource();
56-
} else {
57-
vs = new QueryValueSource(funcQ, 0.0f);
58-
}
51+
vs = subParser.parseAsValueSource();
5952

6053
String l = localParams.get("l");
6154
String u = localParams.get("u");

‎solr/core/src/java/org/apache/solr/search/Grouping.java‎

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,7 @@
2828
import java.util.Set;
2929
import org.apache.lucene.index.ExitableDirectoryReader;
3030
import org.apache.lucene.index.IndexableField;
31-
import org.apache.lucene.queries.function.FunctionQuery;
3231
import org.apache.lucene.queries.function.ValueSource;
33-
import org.apache.lucene.queries.function.valuesource.QueryValueSource;
3432
import org.apache.lucene.search.CachingCollector;
3533
import org.apache.lucene.search.Collector;
3634
import org.apache.lucene.search.MatchNoDocsQuery;
@@ -179,24 +177,16 @@ public void addFieldCommand(String field, SolrQueryRequest request) throws Synta
179177
}
180178

181179
public void addFunctionCommand(String groupByStr, SolrQueryRequest request) throws SyntaxError {
182-
QParser parser = QParser.getParser(groupByStr, FunctionQParserPlugin.NAME, request);
183-
Query q = parser.getQuery();
180+
ValueSource valueSource = FunctionQParser.parseAsValueSource(groupByStr, request);
184181
final Grouping.Command<?> gc;
185-
if (q instanceof FunctionQuery) {
186-
ValueSource valueSource = ((FunctionQuery) q).getValueSource();
187-
if (valueSource instanceof StrFieldSource) {
188-
String field = ((StrFieldSource) valueSource).getField();
189-
CommandField commandField = new CommandField();
190-
commandField.groupBy = field;
191-
gc = commandField;
192-
} else {
193-
CommandFunc commandFunc = new CommandFunc();
194-
commandFunc.groupBy = valueSource;
195-
gc = commandFunc;
196-
}
182+
if (valueSource instanceof StrFieldSource) {
183+
String field = ((StrFieldSource) valueSource).getField();
184+
CommandField commandField = new CommandField();
185+
commandField.groupBy = field;
186+
gc = commandField;
197187
} else {
198188
CommandFunc commandFunc = new CommandFunc();
199-
commandFunc.groupBy = new QueryValueSource(q, 0.0f);
189+
commandFunc.groupBy = valueSource;
200190
gc = commandFunc;
201191
}
202192
gc.withinGroupSort = withinGroupSort;

‎solr/core/src/java/org/apache/solr/search/QParser.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@
2222
import java.util.List;
2323
import java.util.Map;
2424
import java.util.Objects;
25+
import org.apache.lucene.queries.function.FunctionQuery;
26+
import org.apache.lucene.queries.function.FunctionScoreQuery;
27+
import org.apache.lucene.queries.function.ValueSource;
28+
import org.apache.lucene.queries.function.valuesource.QueryValueSource;
2529
import org.apache.lucene.search.Query;
2630
import org.apache.solr.common.SolrException;
2731
import org.apache.solr.common.params.CommonParams;
@@ -31,6 +35,7 @@
3135
import org.apache.solr.common.util.StrUtils;
3236
import org.apache.solr.core.SolrConfig;
3337
import org.apache.solr.request.SolrQueryRequest;
38+
import org.apache.solr.search.ValueSourceParser.LongConstValueSource;
3439

3540
/**
3641
* <b>Note: This API is experimental and may change in non backward-compatible ways in the
@@ -326,6 +331,21 @@ public int getPrefixQueryMinPrefixLength() {
326331
return getReq().getCore().getSolrConfig().prefixQueryMinPrefixLength;
327332
}
328333

334+
/**
335+
* Parse the string into a {@link ValueSource} <em>instead of a {@link Query}</em>. Solr calls
336+
* this in most places that "function queries" go. Overridden by {@link FunctionQParser}.
337+
*/
338+
public ValueSource parseAsValueSource() throws SyntaxError {
339+
Query q = getQuery();
340+
return switch (q) {
341+
case null -> new LongConstValueSource(0);
342+
case FunctionQuery functionQuery -> functionQuery.getValueSource();
343+
case FunctionScoreQuery functionQuery -> ValueSource.fromDoubleValuesSource(
344+
functionQuery.getSource());
345+
default -> new QueryValueSource(q, 0.0f);
346+
};
347+
}
348+
329349
/**
330350
* Create a {@link QParser} to parse <code>qstr</code>, using the "lucene"
331351
* (QParserPlugin.DEFAULT_QTYPE) query parser. The query parser may be overridden by local-params

‎solr/core/src/java/org/apache/solr/search/SolrReturnFields.java‎

Lines changed: 1 addition & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,7 @@
2828
import java.util.Map;
2929
import java.util.Set;
3030
import java.util.function.Supplier;
31-
import org.apache.lucene.queries.function.FunctionQuery;
3231
import org.apache.lucene.queries.function.ValueSource;
33-
import org.apache.lucene.queries.function.valuesource.QueryValueSource;
34-
import org.apache.lucene.search.Query;
3532
import org.apache.solr.common.SolrException;
3633
import org.apache.solr.common.params.CommonParams;
3734
import org.apache.solr.common.params.ModifiableSolrParams;
@@ -382,47 +379,10 @@ private void add(
382379

383380
// let's try it as a function instead
384381
QParser parser = QParser.getParser(funcStr, FunctionQParserPlugin.NAME, req);
385-
Query q = null;
386-
ValueSource vs = null;
387-
388382
try {
389-
if (parser instanceof FunctionQParser fparser) {
390-
fparser.setParseMultipleSources(false);
391-
fparser.setParseToEnd(false);
392-
393-
q = fparser.getQuery();
394-
395-
if (fparser.localParams != null) {
396-
if (fparser.valFollowedParams) {
397-
// need to find the end of the function query via the string parser
398-
int leftOver = fparser.sp.end - fparser.sp.pos;
399-
sp.pos = sp.end - leftOver; // reset our parser to the same amount of leftover
400-
} else {
401-
// the value was via the "v" param in localParams, so we need to find
402-
// the end of the local params themselves to pick up where we left off
403-
sp.pos = start + fparser.localParamsEnd;
404-
}
405-
} else {
406-
// need to find the end of the function query via the string parser
407-
int leftOver = fparser.sp.end - fparser.sp.pos;
408-
sp.pos = sp.end - leftOver; // reset our parser to the same amount of leftover
409-
}
410-
} else {
411-
// A QParser that's not for function queries.
412-
// It must have been specified via local params.
413-
q = parser.getQuery();
414-
415-
assert parser.getLocalParams() != null;
416-
sp.pos = start + parser.localParamsEnd;
417-
}
383+
ValueSource vs = SortSpecParsing.parseValueSource(parser, sp, start);
418384
funcStr = sp.val.substring(start, sp.pos);
419385

420-
if (q instanceof FunctionQuery) {
421-
vs = ((FunctionQuery) q).getValueSource();
422-
} else {
423-
vs = new QueryValueSource(q, 0.0f);
424-
}
425-
426386
if (key == null) {
427387
SolrParams localParams = parser.getLocalParams();
428388
if (localParams != null) {

0 commit comments

Comments
 (0)