Re: [PR] add percentage and threshold based minimum match functionality to BoolQParserPlugin [solr]
dsmiley commented on code in PR #4406:
URL: https://github.com/apache/solr/pull/4406#discussion_r3444329679
##
solr/core/src/java/org/apache/solr/search/BoolQParserPlugin.java:
##
@@ -46,10 +48,12 @@ public Query parse() throws SyntaxError {
}
@Override
- protected BooleanQuery.Builder createBuilder() {
-BooleanQuery.Builder builder = super.createBuilder();
-builder.setMinimumNumberShouldMatch(localParams.getInt("mm", 0));
-return builder;
+ protected BooleanQuery parseImpl() throws SyntaxError {
+BooleanQuery query = super.parseImpl();
+SolrParams solrParams = SolrParams.wrapDefaults(localParams, params);
+String minShouldMatch =
SolrPluginUtils.parseMinShouldMatch(req.getSchema(), solrParams);
+boolean mmAutoRelax = params.getBool(DisMaxParams.MM_AUTORELAX, false);
+return SolrPluginUtils.setMinShouldMatch(query, minShouldMatch,
mmAutoRelax);
Review Comment:
autoRelax isn't going to work if set to true because the internal logic is
assuming DisjunctionMax but BoolQParserPlugin isn't using that.
--
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]
Re: [PR] add percentage and threshold based minimum match functionality to BoolQParserPlugin [solr]
renatoh commented on code in PR #4406:
URL: https://github.com/apache/solr/pull/4406#discussion_r3444181095
##
solr/core/src/java/org/apache/solr/search/BoolQParserPlugin.java:
##
@@ -46,10 +48,12 @@ public Query parse() throws SyntaxError {
}
@Override
- protected BooleanQuery.Builder createBuilder() {
-BooleanQuery.Builder builder = super.createBuilder();
-builder.setMinimumNumberShouldMatch(localParams.getInt("mm", 0));
-return builder;
+ protected BooleanQuery parseImpl() throws SyntaxError {
+BooleanQuery query = super.parseImpl();
+SolrParams solrParams = SolrParams.wrapDefaults(localParams, params);
Review Comment:
I see you point, I think we can easily fix it by just doing:
SolrPluginUtils.parseMinShouldMatch(req.getSchema(), localParams);
and not calling SolrPluginUtils.parseMinShouldMatch(req.getSchema(),
localParams);
--
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]
Re: [PR] add percentage and threshold based minimum match functionality to BoolQParserPlugin [solr]
dsmiley commented on code in PR #4406:
URL: https://github.com/apache/solr/pull/4406#discussion_r3444070817
##
solr/core/src/java/org/apache/solr/search/BoolQParserPlugin.java:
##
@@ -46,10 +48,12 @@ public Query parse() throws SyntaxError {
}
@Override
- protected BooleanQuery.Builder createBuilder() {
-BooleanQuery.Builder builder = super.createBuilder();
-builder.setMinimumNumberShouldMatch(localParams.getInt("mm", 0));
-return builder;
+ protected BooleanQuery parseImpl() throws SyntaxError {
+BooleanQuery query = super.parseImpl();
+SolrParams solrParams = SolrParams.wrapDefaults(localParams, params);
Review Comment:
in retrospect, I think this parser should only consult localParams. Many
internal/system query parsers only look at localParams because they don't want
possible interference of request level params for the parser used in defType
(when they are not the same usage). These internal/system query parsers are
typically not used for 'q', but of course it's allowed.
--
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]
Re: [PR] add percentage and threshold based minimum match functionality to BoolQParserPlugin [solr]
renatoh commented on code in PR #4406: URL: https://github.com/apache/solr/pull/4406#discussion_r3328371987 ## changelog/unreleased/PR#4406-enhance-minimum-match-for-BoolQParserPlugin.yml: ## @@ -0,0 +1,7 @@ +title: add percentage and threshold based minimum match functionality, as we know it from ExtendedDismaxQParser, to BoolQParserPlugin +type: changed Review Comment: A user must use a non-integer value for mm, so I guess it is kind of an opt-in. Since the existing logic is unchanged I think you are right and 'added' is the right type. Will change that in the in the other PR: https://github.com/apache/solr/pull/4475 -- 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]
Re: [PR] add percentage and threshold based minimum match functionality to BoolQParserPlugin [solr]
dsmiley commented on code in PR #4406: URL: https://github.com/apache/solr/pull/4406#discussion_r3326452108 ## changelog/unreleased/PR#4406-enhance-minimum-match-for-BoolQParserPlugin.yml: ## @@ -0,0 +1,7 @@ +title: add percentage and threshold based minimum match functionality, as we know it from ExtendedDismaxQParser, to BoolQParserPlugin +type: changed Review Comment: BTW sounds like `added`, as I assume a user must opt-in for these enhancements -- 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]
Re: [PR] add percentage and threshold based minimum match functionality to BoolQParserPlugin [solr]
dsmiley merged PR #4406: URL: https://github.com/apache/solr/pull/4406 -- 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]
