You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ge...@apache.org on 2020/08/11 13:02:41 UTC
[lucene-solr] branch branch_8x updated: SOLR-14692: Allow 'method'
specification on JSON Facet join domain transforms (#1707)
This is an automated email from the ASF dual-hosted git repository.
gerlowskija pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
The following commit(s) were added to refs/heads/branch_8x by this push:
new d6992f7 SOLR-14692: Allow 'method' specification on JSON Facet join domain transforms (#1707)
d6992f7 is described below
commit d6992f74e0673d2ed5593c6d9312651e94267446
Author: Jason Gerlowski <ge...@apache.org>
AuthorDate: Tue Aug 11 08:21:07 2020 -0400
SOLR-14692: Allow 'method' specification on JSON Facet join domain transforms (#1707)
---
.../org/apache/solr/search/JoinQParserPlugin.java | 49 +++++++++++++++++++++-
.../org/apache/solr/search/facet/FacetRequest.java | 39 ++++++++++++-----
.../solr/search/join/ScoreJoinQParserPlugin.java | 14 +++++++
.../search/facet/TestCloudJSONFacetJoinDomain.java | 38 ++++++++++++++++-
.../src/json-faceting-domain-changes.adoc | 6 ++-
5 files changed, 130 insertions(+), 16 deletions(-)
diff --git a/solr/core/src/java/org/apache/solr/search/JoinQParserPlugin.java b/solr/core/src/java/org/apache/solr/search/JoinQParserPlugin.java
index 0aab8db..76544d5 100644
--- a/solr/core/src/java/org/apache/solr/search/JoinQParserPlugin.java
+++ b/solr/core/src/java/org/apache/solr/search/JoinQParserPlugin.java
@@ -20,6 +20,7 @@ import java.io.Closeable;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.EnumSet;
import java.util.HashSet;
import java.util.List;
import java.util.Objects;
@@ -99,12 +100,22 @@ public class JoinQParserPlugin extends QParserPlugin {
q.fromCoreOpenTime = jParams.fromCoreOpenTime;
return q;
}
+
+ @Override
+ Query makeJoinDirectFromParams(JoinParams jParams) {
+ return new JoinQuery(jParams.fromField, jParams.toField, null, jParams.fromQuery);
+ }
},
dvWithScore {
@Override
Query makeFilter(QParser qparser, JoinQParserPlugin plugin) throws SyntaxError {
return new ScoreJoinQParserPlugin().createParser(qparser.qstr, qparser.localParams, qparser.params, qparser.req).parse();
}
+
+ @Override
+ Query makeJoinDirectFromParams(JoinParams jParams) {
+ return ScoreJoinQParserPlugin.createJoinQuery(jParams.fromQuery, jParams.fromField, jParams.toField, org.apache.lucene.search.join.ScoreMode.None);
+ }
},
topLevelDV {
@Override
@@ -114,6 +125,11 @@ public class JoinQParserPlugin extends QParserPlugin {
q.fromCoreOpenTime = jParams.fromCoreOpenTime;
return q;
}
+
+ @Override
+ Query makeJoinDirectFromParams(JoinParams jParams) {
+ return new TopLevelJoinQuery(jParams.fromField, jParams.toField, null, jParams.fromQuery);
+ }
},
crossCollection {
@Override
@@ -125,6 +141,10 @@ public class JoinQParserPlugin extends QParserPlugin {
abstract Query makeFilter(QParser qparser, JoinQParserPlugin plugin) throws SyntaxError;
+ Query makeJoinDirectFromParams(JoinParams jParams) {
+ throw new IllegalStateException("Join method [" + name() + "] doesn't support qparser-less creation");
+ }
+
JoinParams parseJoin(QParser qparser) throws SyntaxError {
final String fromField = qparser.getParam("from");
final String fromIndex = qparser.getParam("fromIndex");
@@ -208,15 +228,40 @@ public class JoinQParserPlugin extends QParserPlugin {
};
}
+ private static final EnumSet<Method> JOIN_METHOD_WHITELIST = EnumSet.of(Method.index, Method.topLevelDV, Method.dvWithScore);
/**
* A helper method for other plugins to create (non-scoring) JoinQueries wrapped around arbitrary queries against the same core.
*
* @param subQuery the query to define the starting set of documents on the "left side" of the join
* @param fromField "left side" field name to use in the join
* @param toField "right side" field name to use in the join
+ * @param method indicates which implementation should be used to process the join. Currently only 'index',
+ * 'dvWithScore', and 'topLevelDV' are supported.
*/
- public static Query createJoinQuery(Query subQuery, String fromField, String toField) {
- return new JoinQuery(fromField, toField, null, subQuery);
+ public static Query createJoinQuery(Query subQuery, String fromField, String toField, String method) {
+ // no method defaults to 'index' for back compatibility
+ if ( method == null ) {
+ return new JoinQuery(fromField, toField, null, subQuery);
+ }
+
+
+ final Method joinMethod = parseMethodString(method);
+ if (! JOIN_METHOD_WHITELIST.contains(joinMethod)) {
+ // TODO Throw something that the callers here (FacetRequest) can catch and produce a more domain-appropriate error message for?
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
+ "Join method " + method + " not supported for non-scoring, same-core joins");
+ }
+
+ final JoinParams jParams = new JoinParams(fromField, null, subQuery, 0L, toField);
+ return joinMethod.makeJoinDirectFromParams(jParams);
+ }
+
+ private static Method parseMethodString(String method) {
+ try {
+ return Method.valueOf(method);
+ } catch (IllegalArgumentException iae) {
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Provided join method '" + method + "' not supported");
+ }
}
}
diff --git a/solr/core/src/java/org/apache/solr/search/facet/FacetRequest.java b/solr/core/src/java/org/apache/solr/search/facet/FacetRequest.java
index 35e13cb..104a571 100644
--- a/solr/core/src/java/org/apache/solr/search/facet/FacetRequest.java
+++ b/solr/core/src/java/org/apache/solr/search/facet/FacetRequest.java
@@ -19,9 +19,12 @@ package org.apache.solr.search.facet;
import java.io.IOException;
import java.util.LinkedHashMap;
import java.util.List;
+import java.util.Locale;
import java.util.Map;
import java.util.Objects;
+import java.util.Set;
+import com.google.common.collect.Sets;
import org.apache.lucene.search.Query;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.params.SolrParams;
@@ -179,15 +182,23 @@ public abstract class FacetRequest {
* Are we doing a query time join across other documents
*/
public static class JoinField {
+ private static final String FROM_PARAM = "from";
+ private static final String TO_PARAM = "to";
+ private static final String METHOD_PARAM = "method";
+ private static final Set<String> SUPPORTED_JOIN_PROPERTIES = Sets.newHashSet(FROM_PARAM, TO_PARAM, METHOD_PARAM);
+
public final String from;
public final String to;
+ public final String method;
- private JoinField(String from, String to) {
+ private JoinField(String from, String to, String method) {
assert null != from;
assert null != to;
+ assert null != method;
this.from = from;
this.to = to;
+ this.method = method;
}
/**
@@ -208,18 +219,26 @@ public abstract class FacetRequest {
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
"'join' domain change requires a map containing the 'from' and 'to' fields");
}
- @SuppressWarnings({"unchecked"}) final Map<String, String> join = (Map<String, String>) queryJoin;
- if (!(join.containsKey("from") && join.containsKey("to") &&
- null != join.get("from") && null != join.get("to"))) {
+
+ @SuppressWarnings({"unchecked"})
+ final Map<String,String> join = (Map<String,String>) queryJoin;
+ if (! (join.containsKey(FROM_PARAM) && join.containsKey(TO_PARAM) &&
+ null != join.get(FROM_PARAM) && null != join.get(TO_PARAM)) ) {
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
"'join' domain change requires non-null 'from' and 'to' field names");
}
- if (2 != join.size()) {
- throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
- "'join' domain change contains unexpected keys, only 'from' and 'to' supported: "
- + join.toString());
+
+ for (String providedKey : join.keySet()) {
+ if (! SUPPORTED_JOIN_PROPERTIES.contains(providedKey)) {
+ final String supportedPropsStr = String.join(", ", SUPPORTED_JOIN_PROPERTIES);
+ final String message = String.format(Locale.ROOT,
+ "'join' domain change contains unexpected key [%s], only %s supported", providedKey, supportedPropsStr);
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, message);
+ }
}
- domain.joinField = new JoinField(join.get("from"), join.get("to"));
+
+ final String method = join.containsKey(METHOD_PARAM) ? join.get(METHOD_PARAM) : "index";
+ domain.joinField = new JoinField(join.get(FROM_PARAM), join.get(TO_PARAM), method);
}
}
@@ -236,7 +255,7 @@ public abstract class FacetRequest {
// this shouldn't matter once we're wrapped in a join query, but just in case it ever does...
fromQuery.setCache(false);
- return JoinQParserPlugin.createJoinQuery(fromQuery, this.from, this.to);
+ return JoinQParserPlugin.createJoinQuery(fromQuery, this.from, this.to, this.method);
}
diff --git a/solr/core/src/java/org/apache/solr/search/join/ScoreJoinQParserPlugin.java b/solr/core/src/java/org/apache/solr/search/join/ScoreJoinQParserPlugin.java
index 423fd25..51f2dcc 100644
--- a/solr/core/src/java/org/apache/solr/search/join/ScoreJoinQParserPlugin.java
+++ b/solr/core/src/java/org/apache/solr/search/join/ScoreJoinQParserPlugin.java
@@ -291,6 +291,20 @@ public class ScoreJoinQParserPlugin extends QParserPlugin {
return fromIndex;
}
+ /**
+ * A helper method for other plugins to create single-core JoinQueries
+ *
+ * @param subQuery the query to define the starting set of documents on the "left side" of the join
+ * @param fromField "left side" field name to use in the join
+ * @param toField "right side" field name to use in the join
+ * @param scoreMode the score statistic to produce while joining
+ *
+ * @see JoinQParserPlugin#createJoinQuery(Query, String, String, String)
+ */
+ public static Query createJoinQuery(Query subQuery, String fromField, String toField, ScoreMode scoreMode) {
+ return new SameCoreJoinQuery(subQuery, fromField, toField, scoreMode);
+ }
+
private static String resolveAlias(String fromIndex, ZkController zkController) {
final Aliases aliases = zkController.getZkStateReader().getAliases();
try {
diff --git a/solr/core/src/test/org/apache/solr/search/facet/TestCloudJSONFacetJoinDomain.java b/solr/core/src/test/org/apache/solr/search/facet/TestCloudJSONFacetJoinDomain.java
index 4c87746..a88ed95 100644
--- a/solr/core/src/test/org/apache/solr/search/facet/TestCloudJSONFacetJoinDomain.java
+++ b/solr/core/src/test/org/apache/solr/search/facet/TestCloudJSONFacetJoinDomain.java
@@ -208,14 +208,48 @@ public class TestCloudJSONFacetJoinDomain extends SolrCloudTestCase {
SolrException e = expectThrows(SolrException.class, () -> {
final SolrParams req = params("q", "*:*", "json.facet",
"{ x : { type:terms, field:x_s, domain: { join:"+join+" } } }");
- @SuppressWarnings({"rawtypes"})
- final NamedList trash = getRandClient(random()).request(new QueryRequest(req));
+ getRandClient(random()).request(new QueryRequest(req));
});
assertEquals(join + " -> " + e, SolrException.ErrorCode.BAD_REQUEST.code, e.code());
assertTrue(join + " -> " + e, e.getMessage().contains("'join' domain change"));
}
}
+ public void testJoinMethodSyntax() throws Exception {
+ // 'method' value that doesn't exist at all
+ {
+ final String joinJson = "{from:foo, to:bar, method:invalidValue}";
+ SolrException e = expectThrows(SolrException.class, () -> {
+ final SolrParams req = params("q", "*:*", "json.facet",
+ "{ x : { type:terms, field:x_s, domain: { join:"+joinJson+" } } }");
+ getRandClient(random()).request(new QueryRequest(req));
+ });
+ assertEquals(joinJson + " -> " + e, SolrException.ErrorCode.BAD_REQUEST.code, e.code());
+ assertTrue(joinJson + " -> " + e, e.getMessage().contains("join method 'invalidValue' not supported"));
+ }
+
+ // 'method' value that exists on joins generally but isn't supported for join domain transforms
+ {
+ final String joinJson = "{from:foo, to:bar, method:crossCollection}";
+ SolrException e = expectThrows(SolrException.class, () -> {
+ final SolrParams req = params("q", "*:*", "json.facet",
+ "{ x : { type:terms, field:x_s, domain: { join:"+joinJson+" } } }");
+ getRandClient(random()).request(new QueryRequest(req));
+ });
+ assertEquals(joinJson + " -> " + e, SolrException.ErrorCode.BAD_REQUEST.code, e.code());
+ assertTrue(joinJson + " -> " + e, e.getMessage().contains("Join method crossCollection not supported"));
+ }
+
+
+ // Valid, supported method value
+ {
+ final String joinJson = "{from:" +strfield(1)+ ", to:"+strfield(1)+", method:index}";
+ final SolrParams req = params("q", "*:*", "json.facet", "{ x : { type:terms, field:x_s, domain: { join:"+joinJson+" } } }");
+ getRandClient(random()).request(new QueryRequest(req));
+ // For the purposes of this test, we're not interested in the response so much as that Solr will accept a valid 'method' value
+ }
+ }
+
public void testSanityCheckDomainMethods() throws Exception {
{
final JoinDomain empty = new JoinDomain(null, null, null);
diff --git a/solr/solr-ref-guide/src/json-faceting-domain-changes.adoc b/solr/solr-ref-guide/src/json-faceting-domain-changes.adoc
index d8d543a..34f9a14 100644
--- a/solr/solr-ref-guide/src/json-faceting-domain-changes.adoc
+++ b/solr/solr-ref-guide/src/json-faceting-domain-changes.adoc
@@ -77,7 +77,7 @@ The referred parameter might have 0 (absent) or many values.
** When no values are specified, no filter is applied and no error is thrown.
** When many values are specified, each value is parsed and used as filters in conjunction.
-Here is the example of referencing DSL queries:
+Here is the example of referencing DSL queries:
[source,bash]
----
@@ -241,7 +241,7 @@ Both of these options work similarly to the corresponding <<other-parsers.adoc#b
A `join` domain change option can be used to specify arbitrary `from` and `to` fields to use in transforming from the existing domain to a related set of documents.
-This works very similar to the <<other-parsers.adoc#join-query-parser,Join Query Parser>>, and has the same limitations when dealing with multi-shard collections.
+This works similarly to the <<other-parsers.adoc#join-query-parser,Join Query Parser>>, and has the same limitations when dealing with multi-shard collections.
Example:
[source,json]
@@ -268,6 +268,8 @@ Example:
----
+`join` domain changes support an optional `method` parameter, which allows users to specify which join implementation they would like to use in this domain transform. Solr offers several join implementations, each with different performance characteristics. For more information on these implementations and their tradeoffs, see the `method` parameter documentation <<other-parsers.adoc#parameters,here>>. Join domain changes support all `method` values except `crossCollection`.
+
== Graph Traversal Domain Changes
A `graph` domain change option works similarly to the `join` domain option, but can do traversal multiple hops `from` the existing domain `to` other documents.