You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "AndreyBozhko (via GitHub)" <gi...@apache.org> on 2023/10/18 18:06:12 UTC
[PR] SOLR-13748: Add support for mm parameter to bool query parser [solr]
AndreyBozhko opened a new pull request, #2025:
URL: https://github.com/apache/solr/pull/2025
https://issues.apache.org/jira/browse/SOLR-13748
<!--
_(If you are a project committer then you may remove some/all of the following template.)_
Before creating a pull request, please file an issue in the ASF Jira system for Solr:
* https://issues.apache.org/jira/projects/SOLR
For something minor (i.e. that wouldn't be worth putting in release notes), you can skip JIRA.
To create a Jira issue, you will need to create an account there first.
The title of the PR should reference the Jira issue number in the form:
* SOLR-####: <short description of problem or changes>
SOLR must be fully capitalized. A short description helps people scanning pull requests for items they can work on.
Properly referencing the issue in the title ensures that Jira is correctly updated with code review comments and commits. -->
# Description
Added support for `mm` parameter to {!bool} query parser
# Tests
Added test suite for BoolQParserPlugin
# Checklist
Please review the following and check all that apply:
- [x] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) and my code conforms to the standards described there to the best of my ability.
- [x] I have created a Jira issue and added the issue ID to my pull request title.
- [x] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
- [x] I have developed this patch against the `main` branch.
- [x] I have run `./gradlew check`.
- [x] I have added tests for my changes.
- [x] I have added documentation for the [Reference Guide](https://github.com/apache/solr/tree/main/solr/solr-ref-guide)
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-13748: Add support for mm parameter to bool query parser [solr]
Posted by "mkhludnev (via GitHub)" <gi...@apache.org>.
mkhludnev commented on code in PR #2025:
URL: https://github.com/apache/solr/pull/2025#discussion_r1385654200
##########
solr/core/src/java/org/apache/solr/search/join/FiltersQParser.java:
##########
@@ -61,6 +61,9 @@ protected BooleanQuery parseImpl() throws SyntaxError {
for (Map.Entry<QParser, Occur> clause : clauses.entrySet()) {
builder.add(unwrapQuery(clause.getKey().getQuery(), clause.getValue()), clause.getValue());
}
+
+ builder.setMinimumNumberShouldMatch(minShouldMatch());
Review Comment:
> Could it be because FILTER clauses are considered required, and never optional?
indeed [Like MUST](https://lucene.apache.org/core/9_4_1/core/org/apache/lucene/search/BooleanClause.Occur.html#FILTER)
>
> Also, if one needs to use, say, three clauses in the filter query and make sure that at least two match, - it should be possible to construct such filter query as `{!bool filter=...}`, where the only FILTER clause is itself a boolean query with three SHOULD clauses and mm=2.
perhaps,
1. wrap a such of bool of should via single clause filter.
2. wipe the score of that `{!bool should=... mm=2}^=0`
3. or wipe scores of underneath filters via `{!bool should=A:b^=0 should=C:d^=0 should=E:f^=0 mm=2}`
NB: I'm not sure whether parser is capable to parse those options above. If it isn't, one can trick it with local param referencing.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-13748: Add support for mm parameter to bool query parser [solr]
Posted by "AndreyBozhko (via GitHub)" <gi...@apache.org>.
AndreyBozhko commented on code in PR #2025:
URL: https://github.com/apache/solr/pull/2025#discussion_r1385606641
##########
solr/solr-ref-guide/modules/query-guide/pages/other-parsers.adoc:
##########
@@ -75,6 +75,16 @@ However, unlike `must`, the score of filter queries is ignored.
Also, these queries are cached in filter cache.
To avoid caching add either `cache=false` as local parameter, or `"cache":"false"` property to underneath Query DLS Object.
+`mm`::
++
+[%autowidth,frame=none]
+|===
+|Optional |Default: `0`
+|===
++
+The number of optional clauses that must match. By default, no optional clauses are necessary for a match
+(unless there are no required clauses). If this parameter is set, then the specified number of `should` clauses is required.
Review Comment:
Thanks for the suggestion @mkhludnev, I updated the description.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-13748: Add support for mm parameter to bool query parser [solr]
Posted by "mkhludnev (via GitHub)" <gi...@apache.org>.
mkhludnev commented on code in PR #2025:
URL: https://github.com/apache/solr/pull/2025#discussion_r1384447321
##########
solr/solr-ref-guide/modules/query-guide/pages/other-parsers.adoc:
##########
@@ -75,6 +75,16 @@ However, unlike `must`, the score of filter queries is ignored.
Also, these queries are cached in filter cache.
To avoid caching add either `cache=false` as local parameter, or `"cache":"false"` property to underneath Query DLS Object.
+`mm`::
++
+[%autowidth,frame=none]
+|===
+|Optional |Default: `0`
+|===
++
+The number of optional clauses that must match. By default, no optional clauses are necessary for a match
+(unless there are no required clauses). If this parameter is set, then the specified number of `should` clauses is required.
Review Comment:
It's probably worth to clarify that when there are no required clauses it works like mm=1.
##########
solr/core/src/java/org/apache/solr/search/join/FiltersQParser.java:
##########
@@ -61,6 +61,9 @@ protected BooleanQuery parseImpl() throws SyntaxError {
for (Map.Entry<QParser, Occur> clause : clauses.entrySet()) {
builder.add(unwrapQuery(clause.getKey().getQuery(), clause.getValue()), clause.getValue());
}
+
+ builder.setMinimumNumberShouldMatch(minShouldMatch());
Review Comment:
Not sure why. MinShouldMatch matters only for SHOULD clauses, but never for FILTER ever.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-13748: Add support for mm parameter to bool query parser [solr]
Posted by "mkhludnev (via GitHub)" <gi...@apache.org>.
mkhludnev merged PR #2025:
URL: https://github.com/apache/solr/pull/2025
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-13748: Add support for mm parameter to bool query parser [solr]
Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on PR #2025:
URL: https://github.com/apache/solr/pull/2025#issuecomment-1769084229
I do like `minShouldMatch` a lot more than `mm`.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-13748: Add support for mm parameter to bool query parser [solr]
Posted by "AndreyBozhko (via GitHub)" <gi...@apache.org>.
AndreyBozhko commented on code in PR #2025:
URL: https://github.com/apache/solr/pull/2025#discussion_r1385621132
##########
solr/core/src/java/org/apache/solr/search/join/FiltersQParser.java:
##########
@@ -61,6 +61,9 @@ protected BooleanQuery parseImpl() throws SyntaxError {
for (Map.Entry<QParser, Occur> clause : clauses.entrySet()) {
builder.add(unwrapQuery(clause.getKey().getQuery(), clause.getValue()), clause.getValue());
}
+
+ builder.setMinimumNumberShouldMatch(minShouldMatch());
Review Comment:
Could it be because FILTER clauses are considered required, and never optional?
Also, if one needs to use, say, three clauses in the filter query and make sure that at least two match, - it should be possible to construct such filter query as `{!bool filter=...}`, where the only FILTER clause is itself a boolean query with three SHOULD clauses and mm=2.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-13748: Add support for mm parameter to bool query parser [solr]
Posted by "mkhludnev (via GitHub)" <gi...@apache.org>.
mkhludnev commented on PR #2025:
URL: https://github.com/apache/solr/pull/2025#issuecomment-1842173386
Thanks @AndreyBozhko I'll check it next week
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-13748: Add support for mm parameter to bool query parser [solr]
Posted by "AndreyBozhko (via GitHub)" <gi...@apache.org>.
AndreyBozhko commented on PR #2025:
URL: https://github.com/apache/solr/pull/2025#issuecomment-1771097514
Since the similar parameter for DisMax/eDisMax query parsers is also named `mm`, perhaps it makes sense to keep the naming consistent?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-13748: Add support for mm parameter to bool query parser [solr]
Posted by "AndreyBozhko (via GitHub)" <gi...@apache.org>.
AndreyBozhko commented on PR #2025:
URL: https://github.com/apache/solr/pull/2025#issuecomment-1783071590
Thanks for the review @HoustonPutman, feel free to merge this whenever you have time.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-13748: Add support for mm parameter to bool query parser [solr]
Posted by "AndreyBozhko (via GitHub)" <gi...@apache.org>.
AndreyBozhko commented on PR #2025:
URL: https://github.com/apache/solr/pull/2025#issuecomment-1841596601
Hi @HoustonPutman @mkhludnev - anything else to address for this PR? If not, then it should be good to merge, but I don't have access to do so myself.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-13748: Add support for mm parameter to bool query parser [solr]
Posted by "mkhludnev (via GitHub)" <gi...@apache.org>.
mkhludnev commented on PR #2025:
URL: https://github.com/apache/solr/pull/2025#issuecomment-1845865301
@AndreyBozhko wdyt abt my changes?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org