alessandrobenedetti commented on code in PR #2157:
URL: https://github.com/apache/solr/pull/2157#discussion_r1431744945
##########
solr/core/src/java/org/apache/solr/search/neural/KnnQParser.java:
##########
@@ -82,20 +87,132 @@ public Query parse() throws SyntaxError {
}
private Query getFilterQuery() throws SolrException, SyntaxError {
- boolean isSubQuery = recurseCount != 0;
- if (!isFilter() && !isSubQuery) {
- String[] filterQueries = req.getParams().getParams(CommonParams.FQ);
- if (filterQueries != null && filterQueries.length != 0) {
- try {
- List<Query> filters = QueryUtils.parseFilterQueries(req);
- SolrIndexSearcher.ProcessedFilter processedFilter =
- req.getSearcher().getProcessedFilter(filters);
- return processedFilter.filter;
- } catch (IOException e) {
- throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
+
+ // Default behavior of FQ wrapping, and suitability of some local params
+ // depends on wether we are a sub-query or not
+ final boolean isSubQuery = recurseCount != 0;
+
+ // include/exclude tags for global fqs to wrap;
+ // Check these up front for error handling if combined with `fq` local
param.
+ final List<String> includedGlobalFQTags = getLocalParamTags(INCLUDE_TAGS);
+ final List<String> excludedGlobalFQTags = getLocalParamTags(EXCLUDE_TAGS);
+ final boolean haveGlobalFQTags =
+ !(includedGlobalFQTags.isEmpty() && excludedGlobalFQTags.isEmpty());
+
+ if (haveGlobalFQTags) {
+ // Some early error handling of incompatible options...
+
+ if (isFilter()) { // this knn query is itself a filter query
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "Knn Query Parser used as a filter does not support "
+ + INCLUDE_TAGS
+ + " or "
+ + EXCLUDE_TAGS
+ + " localparams");
+ }
+
+ if (isSubQuery) { // this knn query is a sub-query of a broader query
(possibly disjunction)
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "Knn Query Parser used as a sub-query does not support "
+ + INCLUDE_TAGS
+ + " or "
+ + EXCLUDE_TAGS
+ + " localparams");
+ }
+ }
+
+ // Explicit fq local params specifying the filter(s) to wrap
+ final String[] localFQs = getLocalParams().getParams(CommonParams.FQ);
+ if (null != localFQs) {
+
+ // We don't particularly care if localFQs is empty, the usage below will
still work,
+ // but SolrParams API says it should be null not empty...
+ assert 0 != localFQs.length : "SolrParams.getParams should return null,
never zero len array";
+
+ if (haveGlobalFQTags) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "Knn Query Parser does not support combining "
+ + CommonParams.FQ
+ + " localparam with either "
+ + INCLUDE_TAGS
+ + " or "
+ + EXCLUDE_TAGS
+ + " localparams");
+ }
+
+ final List<Query> localParamFilters = new ArrayList<>(localFQs.length);
+ for (String fq : localFQs) {
+ final QParser parser = subQuery(fq, null);
+ parser.setIsFilter(true);
+
+ // maybe null, ie: `fq=""`
Review Comment:
in general, I am not in favour of comments, sometimes I take more time to
decipher a comment than the related code.
In this instance, to me it's ok to just leave the code, it's probably more
readable
##########
solr/core/src/java/org/apache/solr/search/neural/KnnQParser.java:
##########
@@ -82,20 +87,132 @@ public Query parse() throws SyntaxError {
}
private Query getFilterQuery() throws SolrException, SyntaxError {
- boolean isSubQuery = recurseCount != 0;
- if (!isFilter() && !isSubQuery) {
- String[] filterQueries = req.getParams().getParams(CommonParams.FQ);
- if (filterQueries != null && filterQueries.length != 0) {
- try {
- List<Query> filters = QueryUtils.parseFilterQueries(req);
- SolrIndexSearcher.ProcessedFilter processedFilter =
- req.getSearcher().getProcessedFilter(filters);
- return processedFilter.filter;
- } catch (IOException e) {
- throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
+
+ // Default behavior of FQ wrapping, and suitability of some local params
+ // depends on wether we are a sub-query or not
+ final boolean isSubQuery = recurseCount != 0;
+
+ // include/exclude tags for global fqs to wrap;
+ // Check these up front for error handling if combined with `fq` local
param.
+ final List<String> includedGlobalFQTags = getLocalParamTags(INCLUDE_TAGS);
+ final List<String> excludedGlobalFQTags = getLocalParamTags(EXCLUDE_TAGS);
+ final boolean haveGlobalFQTags =
+ !(includedGlobalFQTags.isEmpty() && excludedGlobalFQTags.isEmpty());
+
+ if (haveGlobalFQTags) {
+ // Some early error handling of incompatible options...
+
+ if (isFilter()) { // this knn query is itself a filter query
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "Knn Query Parser used as a filter does not support "
+ + INCLUDE_TAGS
+ + " or "
+ + EXCLUDE_TAGS
+ + " localparams");
+ }
+
+ if (isSubQuery) { // this knn query is a sub-query of a broader query
(possibly disjunction)
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "Knn Query Parser used as a sub-query does not support "
+ + INCLUDE_TAGS
+ + " or "
+ + EXCLUDE_TAGS
+ + " localparams");
+ }
+ }
+
+ // Explicit fq local params specifying the filter(s) to wrap
+ final String[] localFQs = getLocalParams().getParams(CommonParams.FQ);
Review Comment:
as hinted in the jira, what about calling the parameter 'preFilter'?
I don't remember if camelCase is good for local params
--
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]
