SOLR-14802: geodist: Support most spatial field types as an arg

FunctionQParser: overload parseValueSourceList with flags
This commit is contained in:
Tom Edge 2020-09-20 23:19:48 -04:00 committed by David Smiley
parent aa071f72b1
commit fa756b1c31
No known key found for this signature in database
GPG Key ID: 6FDFF3BF6796FD4A
4 changed files with 129 additions and 34 deletions

View File

@ -181,6 +181,9 @@ Improvements
* SOLR-13438: On deleting a collection, its config set will also be deleted iff it has been auto-created, and not used by any other collection (Anderson Dorow)
* SOLR-14802: geodist: Support most (all?) spatial field types as an argument like LLPSF, SRPTFT, and others.
(Tom Edge, Craig Wrigglesworth)
Optimizations
---------------------

View File

@ -239,9 +239,21 @@ public class FunctionQParser extends QParser {
* @return List<ValueSource>
*/
public List<ValueSource> parseValueSourceList() throws SyntaxError {
return parseValueSourceList(FLAG_DEFAULT | FLAG_CONSUME_DELIMITER);
}
/**
* Parse a list of ValueSource. Must be the final set of arguments
* to a ValueSource.
*
* @param flags - customize parsing behavior
*
* @return List&lt;ValueSource&gt;
*/
public List<ValueSource> parseValueSourceList(int flags) throws SyntaxError {
List<ValueSource> sources = new ArrayList<>(3);
while (hasMoreArguments()) {
sources.add(parseValueSource(FLAG_DEFAULT | FLAG_CONSUME_DELIMITER));
sources.add(parseValueSource(flags));
}
return sources;
}

View File

@ -16,6 +16,7 @@
*/
package org.apache.solr.search.function.distance;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
@ -26,7 +27,6 @@ import org.apache.lucene.queries.function.valuesource.DoubleConstValueSource;
import org.apache.lucene.queries.function.valuesource.MultiValueSource;
import org.apache.lucene.queries.function.valuesource.VectorValueSource;
import org.apache.lucene.spatial.SpatialStrategy;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.params.SpatialParams;
import org.apache.solr.schema.AbstractSpatialFieldType;
import org.apache.solr.schema.FieldType;
@ -34,12 +34,17 @@ import org.apache.solr.schema.SchemaField;
import org.apache.solr.search.FunctionQParser;
import org.apache.solr.search.SyntaxError;
import org.apache.solr.search.ValueSourceParser;
import org.apache.solr.search.function.FieldNameValueSource;
import org.apache.solr.util.DistanceUnits;
import org.apache.solr.util.SpatialUtils;
import org.locationtech.spatial4j.context.SpatialContext;
import org.locationtech.spatial4j.distance.DistanceUtils;
import org.locationtech.spatial4j.shape.Point;
import static org.apache.solr.search.FunctionQParser.FLAG_CONSUME_DELIMITER;
import static org.apache.solr.search.FunctionQParser.FLAG_DEFAULT;
import static org.apache.solr.search.FunctionQParser.FLAG_USE_FIELDNAME_SOURCE;
/**
* Parses "geodist" creating {@link HaversineConstFunction} or {@link HaversineFunction}
* or calling {@link SpatialStrategy#makeDistanceValueSource(org.locationtech.spatial4j.shape.Point,double)}.
@ -50,21 +55,8 @@ public class GeoDistValueSourceParser extends ValueSourceParser {
public ValueSource parse(FunctionQParser fp) throws SyntaxError {
// TODO: dispatch through SpatialQueryable in the future?
//note: parseValueSourceList can't handle a field reference to an AbstractSpatialFieldType,
// so those fields are expressly handled via sfield=
List<ValueSource> sources;
try {
sources = fp.parseValueSourceList();
} catch (SolrException e) {
if (e.getMessage().equals("A ValueSource isn't directly available from this field. " +
"Instead try a query using the distance as the score.")) {
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "geodist() does not support field names in its arguments " +
"when stated fields are solr.LatLonPointSpatialField spatial type, requires sfield param instead");
}
else {
throw e;
}
}
//note: return fields as FieldNameValueSource from parser to support AbstractSpatialFieldType as geodist argument
List<ValueSource> sources = transformFieldSources(fp, fp.parseValueSourceList(FLAG_DEFAULT | FLAG_CONSUME_DELIMITER | FLAG_USE_FIELDNAME_SOURCE));
// "m" is a multi-value source, "x" is a single-value source
// allow (m,m) (m,x,x) (x,x,m) (x,x,x,x)
@ -93,7 +85,6 @@ public class GeoDistValueSourceParser extends ValueSourceParser {
}
} else if (sources.size()==3) {
ValueSource vs1 = sources.get(0);
ValueSource vs2 = sources.get(1);
if (vs1 instanceof MultiValueSource) { // (m,x,x)
mv1 = (MultiValueSource)vs1;
mv2 = makeMV(sources.subList(1, 3), sources);
@ -108,7 +99,7 @@ public class GeoDistValueSourceParser extends ValueSourceParser {
} else if (sources.size()==4) {
mv1 = makeMV(sources.subList(0, 2), sources);
mv2 = makeMV(sources.subList(2, 4), sources);
} else if (sources.size() > 4) {
} else {
throw new SyntaxError("geodist - invalid parameters:" + sources);
}
@ -139,14 +130,14 @@ public class GeoDistValueSourceParser extends ValueSourceParser {
// * HaversineConstFunction
// * HaversineFunction
// sfield can only be in mv2, according to the logic above
if (mv2 instanceof SpatialStrategyMultiValueSource) {
SpatialStrategyMultiValueSource spatialStrategyMultiValueSource = findSpatialStrategyMultiValueSource(mv1, mv2);
if (spatialStrategyMultiValueSource != null) {
if (constants == null)
throw new SyntaxError("When using AbstractSpatialFieldType (e.g. RPT not LatLonType)," +
" the point must be supplied as constants");
// note: uses Haversine by default but can be changed via distCalc=...
SpatialStrategy strategy = ((SpatialStrategyMultiValueSource) mv2).strategy;
DistanceUnits distanceUnits = ((SpatialStrategyMultiValueSource) mv2).distanceUnits;
SpatialStrategy strategy = spatialStrategyMultiValueSource.strategy;
DistanceUnits distanceUnits = spatialStrategyMultiValueSource.distanceUnits;
Point queryPoint = strategy.getSpatialContext().makePoint(constants[1], constants[0]);
return ValueSource.fromDoubleValuesSource(strategy.makeDistanceValueSource(queryPoint, distanceUnits.multiplierFromDegreesToThisUnit()));
}
@ -158,6 +149,29 @@ public class GeoDistValueSourceParser extends ValueSourceParser {
return new HaversineFunction(mv1, mv2, DistanceUtils.EARTH_MEAN_RADIUS_KM, true);
}
private List<ValueSource> transformFieldSources(FunctionQParser fp, List<ValueSource> sources) throws SyntaxError {
List<ValueSource> result = new ArrayList<>(sources.size());
for (ValueSource valueSource : sources) {
if (valueSource instanceof FieldNameValueSource) {
String fieldName = ((FieldNameValueSource) valueSource).getFieldName();
result.add(getMultiValueSource(fp, fieldName));
} else {
result.add(valueSource);
}
}
return result;
}
private SpatialStrategyMultiValueSource findSpatialStrategyMultiValueSource(MultiValueSource mv1, MultiValueSource mv2) {
if (mv1 instanceof SpatialStrategyMultiValueSource) {
return (SpatialStrategyMultiValueSource) mv1;
} else if (mv2 instanceof SpatialStrategyMultiValueSource) {
return (SpatialStrategyMultiValueSource) mv2;
} else {
return null;
}
}
/** make a MultiValueSource from two non MultiValueSources */
private VectorValueSource makeMV(List<ValueSource> sources, List<ValueSource> orig) throws SyntaxError {
ValueSource vs1 = sources.get(0);
@ -169,17 +183,17 @@ public class GeoDistValueSourceParser extends ValueSourceParser {
return new VectorValueSource(sources);
}
private MultiValueSource parsePoint(FunctionQParser fp) throws SyntaxError {
private MultiValueSource parsePoint(FunctionQParser fp) {
String ptStr = fp.getParam(SpatialParams.POINT);
if (ptStr == null) return null;
Point point = SpatialUtils.parsePointSolrException(ptStr, SpatialContext.GEO);
//assume Lat Lon order
return new VectorValueSource(
Arrays.<ValueSource>asList(new DoubleConstValueSource(point.getY()), new DoubleConstValueSource(point.getX())));
Arrays.asList(new DoubleConstValueSource(point.getY()), new DoubleConstValueSource(point.getX())));
}
private double[] getConstants(MultiValueSource vs) {
if (!(vs instanceof VectorValueSource)) return null;
if (vs instanceof SpatialStrategyMultiValueSource || !(vs instanceof VectorValueSource)) return null;
List<ValueSource> sources = ((VectorValueSource)vs).getSources();
if (sources.get(0) instanceof ConstNumberSource && sources.get(1) instanceof ConstNumberSource) {
return new double[] { ((ConstNumberSource) sources.get(0)).getDouble(), ((ConstNumberSource) sources.get(1)).getDouble()};
@ -190,6 +204,10 @@ public class GeoDistValueSourceParser extends ValueSourceParser {
private MultiValueSource parseSfield(FunctionQParser fp) throws SyntaxError {
String sfield = fp.getParam(SpatialParams.FIELD);
if (sfield == null) return null;
return getMultiValueSource(fp, sfield);
}
private MultiValueSource getMultiValueSource(FunctionQParser fp, String sfield) throws SyntaxError {
SchemaField sf = fp.getReq().getSchema().getField(sfield);
FieldType type = sf.getType();
if (type instanceof AbstractSpatialFieldType) {

View File

@ -373,14 +373,76 @@ public class TestSolr4Spatial2 extends SolrTestCaseJ4 {
assertQ(req(params), "*[count(//doc)=1]", "count(//lst[@name='highlighting']/*)=1");
}
@Test
public void testErrorHandlingGeodist() throws Exception{
assertU(adoc("id", "1", "llp", "32.7693246, -79.9289094"));
assertQEx("wrong test exception message","sort param could not be parsed as a query, " +
"and is not a field that exists in the index: geodist(llp,47.36667,8.55)",
req(
@Test // SOLR-14802
public void testGeodistSortPossibleWithLatLonPointSpatialFieldOrSpatialRecursivePrefixTreeField() throws Exception {
assertU(adoc("id", "1", "llp", "53.4721936,-2.24703", "srpt_quad", "53.425272,-2.322356"));
assertU(commit());
assertJQ(req(
"q", "*:*",
"sort", "geodist(llp,47.36667,8.55) asc"
), SolrException.ErrorCode.BAD_REQUEST);
"fq", "{!geofilt}",
"d", "50",
"pt", "53.4721936,-2.24703",
"sfield", "srpt_quad",
"sort", "min(geodist(),geodist(llp,53.4721936,-2.24703)) asc"
),
"/response/docs/[0]/id=='1'");
assertJQ(req(
"q", "*:*",
"fq", "{!geofilt}",
"d", "50",
"pt", "53.4721936,-2.24703",
"sfield", "srpt_quad",
"sort", "min(geodist(),geodist(53.4721936,-2.24703,llp)) asc" // moved llp to the end
),
"/response/docs/[0]/id=='1'");
assertJQ(req(
"q", "*:*",
"fq", "{!geofilt}",
"d", "50",
"pt", "53.4721936,-2.24703",
"sfield", "llp", // trying another field type
"sort", "min(geodist(),geodist(53.4721936,-2.24703,srpt_quad)) asc"
),
"/response/docs/[0]/id=='1'");
}
@Test // SOLR-14802
public void testGeodistSortOrderCorrectWithLatLonPointSpatialFieldAndSpatialRecursivePrefixTreeField() throws Exception {
assertU(adoc("id", "1", "llp", "53.4721936,-2.24703", "srpt_quad", "53.4721936,-2.24703"));
assertU(adoc("id", "2", "llp", "53.425272,-2.322356", "srpt_quad", "55.4721936,-2.24703"));
assertU(commit());
assertJQ(req(
"q", "*:*",
"fq", "{!geofilt}",
"d", "50",
"pt", "53.431669,-2.318720",
"sfield", "srpt_quad",
"sort", "min(geodist(),geodist(llp,53.431669,-2.318720)) asc"
),
"/response/docs/[0]/id=='2'");
assertJQ(req(
"q", "*:*",
"fq", "{!geofilt}",
"d", "50",
"pt", "53.4721936,-2.24703",
"sfield", "srpt_quad",
"sort", "min(geodist(),geodist(53.4721936,-2.24703,llp)) asc"
),
"/response/docs/[0]/id=='1'");
assertJQ(req(
"q", "*:*",
"fq", "{!geofilt}",
"d", "50",
"pt", "55.4721936,-2.24703",
"sfield", "srpt_quad",
"sort", "min(geodist(),geodist(55.4721936,-2.24703,llp)) asc"
),
"/response/docs/[0]/id=='2'");
}
}