You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/06/01 18:06:48 UTC
[GitHub] [solr] kiranchitturi opened a new pull request, #887: SOLR-16227 Use local params value syntax for phrase queries
kiranchitturi opened a new pull request, #887:
URL: https://github.com/apache/solr/pull/887
https://issues.apache.org/jira/browse/SOLR-16227
# Description
Please provide a short description of the changes you're making with this pull request.
# Solution
Please provide a short description of the approach taken to implement your solution.
# Tests
Please describe the tests you've developed or run to confirm this patch implements the feature or solves the problem.
# Checklist
Please review the following and check all that apply:
- [ ] 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.
- [ ] I have created a Jira issue and added the issue ID to my pull request title.
- [ ] 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)
- [ ] I have developed this patch against the `main` branch.
- [ ] I have run `./gradlew check`.
- [ ] I have added tests for my changes.
- [ ] 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
[GitHub] [solr] janhoy commented on pull request #887: SOLR-16227 Use local params value syntax for phrase queries
Posted by GitBox <gi...@apache.org>.
janhoy commented on PR #887:
URL: https://github.com/apache/solr/pull/887#issuecomment-1143979653
Thanks! Even a test case; beautiful!
Can you also add a CHANGES.txt line? For now you can add it for the 9.1 section.
Likely this will not make it to 8.11.2 in time, but you can ask in the 8.11.2 release mail-thread whether the Release Manager wants to wait for it to be back ported. We'll anyway merge it for 9.1 first.
--
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
[GitHub] [solr] dsmiley closed pull request #887: SOLR-16227 Use local params value syntax for phrase queries
Posted by GitBox <gi...@apache.org>.
dsmiley closed pull request #887: SOLR-16227 Use local params value syntax for phrase queries
URL: https://github.com/apache/solr/pull/887
--
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
[GitHub] [solr] janhoy commented on a diff in pull request #887: SOLR-16227 Use local params value syntax for phrase queries
Posted by GitBox <gi...@apache.org>.
janhoy commented on code in PR #887:
URL: https://github.com/apache/solr/pull/887#discussion_r887170885
##########
solr/modules/sql/src/java/org/apache/solr/handler/sql/SolrFilter.java:
##########
@@ -362,8 +362,9 @@ protected String translateLike(RexNode like) {
// but that expects the terms wrapped in double-quotes, not parens
boolean hasMultipleTerms = terms.split("\\s+").length > 1;
if (hasMultipleTerms && (terms.contains("*") || terms.contains("?"))) {
- String quotedTerms = "\"" + terms.substring(1, terms.length() - 1) + "\"";
- return "{!complexphrase}" + pair.getKey() + ":" + quotedTerms;
+ String quotedTerms = "\\\"" + terms.substring(1, terms.length() - 1) + "\\\"";
+ String query = pair.getKey() + ":" + quotedTerms;
+ return String.format("{!complexphrase v=\"%s\"}", query);
Review Comment:
```suggestion
return String.format(Locale.ROOT, "{!complexphrase v=\"%s\"}", query);
```
This should fix the precommit failure
--
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
[GitHub] [solr] janhoy commented on pull request #887: SOLR-16227 Use local params value syntax for phrase queries
Posted by GitBox <gi...@apache.org>.
janhoy commented on PR #887:
URL: https://github.com/apache/solr/pull/887#issuecomment-1146884819
I’m away for the weekend and will not have chance to merge and back port this. There is still a CHANGES entry missing.
Please can another committer shepherd this for 8.11.2 inclusion?
--
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
[GitHub] [solr] dsmiley commented on pull request #887: SOLR-16227 Use local params value syntax for phrase queries
Posted by GitBox <gi...@apache.org>.
dsmiley commented on PR #887:
URL: https://github.com/apache/solr/pull/887#issuecomment-1147394865
Merged into main manually because permissions were not granted for me to commit the CHANGES.txt. Commit a3d59161ef472f879089012e62876542575d07a2
I also did to 8.11.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
[GitHub] [solr] dsmiley commented on a diff in pull request #887: SOLR-16227 Use local params value syntax for phrase queries
Posted by GitBox <gi...@apache.org>.
dsmiley commented on code in PR #887:
URL: https://github.com/apache/solr/pull/887#discussion_r888078349
##########
solr/modules/sql/src/java/org/apache/solr/handler/sql/SolrFilter.java:
##########
@@ -362,8 +363,9 @@ protected String translateLike(RexNode like) {
// but that expects the terms wrapped in double-quotes, not parens
boolean hasMultipleTerms = terms.split("\\s+").length > 1;
if (hasMultipleTerms && (terms.contains("*") || terms.contains("?"))) {
- String quotedTerms = "\"" + terms.substring(1, terms.length() - 1) + "\"";
- return "{!complexphrase}" + pair.getKey() + ":" + quotedTerms;
+ String quotedTerms = "\\\"" + terms.substring(1, terms.length() - 1) + "\\\"";
Review Comment:
see `org.apache.solr.client.solrj.util.ClientUtils#encodeLocalParamVal` instead
--
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
[GitHub] [solr] janhoy commented on pull request #887: SOLR-16227 Use local params value syntax for phrase queries
Posted by GitBox <gi...@apache.org>.
janhoy commented on PR #887:
URL: https://github.com/apache/solr/pull/887#issuecomment-1145875835
Ready to merge once you add a CHANGES.txt line for 9.1..
--
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