dsmiley commented on code in PR #4402:
URL: https://github.com/apache/solr/pull/4402#discussion_r3270542992
##########
changelog/unreleased/SOLR-18227_named_queries_matched_component.yml:
##########
@@ -0,0 +1,7 @@
+title: "SOLR-18227: Add `_name` local parameter support and
`MatchedQueriesComponent` to identify which named sub-queries matched each
document."
Review Comment:
The title shouldn't contain redundant SOLR-XXXX issue
##########
solr/core/src/java/org/apache/solr/search/PrefixQParserPlugin.java:
##########
Review Comment:
please revert your change to QParsers you didn't need to modify
##########
solr/core/src/java/org/apache/solr/search/QParser.java:
##########
@@ -196,6 +197,11 @@ public Query getQuery() throws SyntaxError {
query = parse();
if (localParams != null) {
+ String name = localParams.get(QueryParsing.NAME);
Review Comment:
Lets ensure we have a test that shows that either cache or cost have their
effect. I was worried your change would interfere. It's subtle. Had you put
your change last, they wouldn't. I'd even say you should add a comment here
that this MUST come _before_ extendedQuery() calls below.
##########
solr/core/src/java/org/apache/solr/search/QueryParsing.java:
##########
@@ -53,6 +53,7 @@ public class QueryParsing {
public static final char LOCALPARAM_END = '}';
// true if the value was specified by the "v" param (i.e. v=myval, or
v=$param)
public static final String VAL_EXPLICIT = "__VAL_EXPLICIT__";
+ public static final String NAME = "_name";
Review Comment:
Hmm. `type` and `tag` and `v` and `cache` and ... etc. are underscore
free, yet `_name` has an underscore? How do you justify this? I could see
"name" being maybe used by someone's custom parser but isn't this benign?
##########
solr/core/src/test-files/solr/collection1/conf/solrconfig.xml:
##########
@@ -519,7 +527,7 @@
</restManager>
<!-- warning: not a best practice; requests generally ought to be explicit
to thus not require this -->
- <initParams
path="/select,/dismax,/defaults,/lazy,/spellCheckCompRH,/spellCheckWithWordbreak,/spellCheckCompRH_Direct,/spellCheckCompRH1,/mltrh,/tvrh,/search-facet-def,/search-facet-invariants">
+ <initParams
path="/select,/dismax,/defaults,/lazy,/spellCheckCompRH,/spellCheckWithWordbreak,/spellCheckCompRH_Direct,/spellCheckCompRH1,/mltrh,/tvrh,/matched-queries,/search-facet-def,/search-facet-invariants">
Review Comment:
don't add to this list. the preceding comment advises against.
##########
solr/solr-ref-guide/modules/query-guide/pages/edismax-query-parser.adoc:
##########
@@ -132,6 +132,18 @@ The default is to allow all fields and no embedded Solr
queries, equivalent to `
Per-field overrides of the `qf` parameter may be specified to provide
1-to-many aliasing from field names specified in the query string, to field
names used in the underlying query.
By default, no aliasing is used and field names specified in the query string
are treated as literal field names in the index.
+== `_name` Parameter
Review Comment:
this isn't specific to any query parser
##########
solr/core/src/test-files/solr/collection1/conf/solrconfig.xml:
##########
Review Comment:
This configuration is used by tons of tests. Please add another
solrconfig.xml based off of a minimal one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
