risdenk commented on code in PR #1146:
URL: https://github.com/apache/solr/pull/1146#discussion_r1008364253
##########
solr/core/src/java/org/apache/solr/query/FilterQuery.java:
##########
@@ -46,20 +46,6 @@ public FilterQuery(Query q) {
this.q = q;
}
- @Override
- public final void setCache(boolean cache) {
- /*
- NOTE: at the implementation level, we silently ignore explicit `setCache`
directives. But at a higher level
- (i.e., from the client's perspective) by ignoring at the implementation
level, we in fact respect the semantics
- both of explicit `{!cache=false}filter(q)` and `{!cache=true}filter(q)`.
Since the purpose of FilterQuery
- is to handle caching _internal_ to the query, external `cache=false`
_should_ have no effect. Slightly
- less intuitive: the `cache=true` case should be interpreted as directing
"ensure that we consult the
- filterCache for this query" -- and indeed because caching is handled
internally, the essence of the
- top-level `cache=true` directive is most appropriately respected by having
`getCache()` continue to return
- `false` at the level of the FilterQuery _per se_.
- */
- }
-
Review Comment:
It is kinda covered in `getCache()`
##########
solr/core/src/java/org/apache/solr/query/FilterQuery.java:
##########
@@ -46,20 +46,6 @@ public FilterQuery(Query q) {
this.q = q;
}
- @Override
- public final void setCache(boolean cache) {
- /*
- NOTE: at the implementation level, we silently ignore explicit `setCache`
directives. But at a higher level
- (i.e., from the client's perspective) by ignoring at the implementation
level, we in fact respect the semantics
- both of explicit `{!cache=false}filter(q)` and `{!cache=true}filter(q)`.
Since the purpose of FilterQuery
- is to handle caching _internal_ to the query, external `cache=false`
_should_ have no effect. Slightly
- less intuitive: the `cache=true` case should be interpreted as directing
"ensure that we consult the
- filterCache for this query" -- and indeed because caching is handled
internally, the essence of the
- top-level `cache=true` directive is most appropriately respected by having
`getCache()` continue to return
- `false` at the level of the FilterQuery _per se_.
- */
- }
-
Review Comment:
TODO find a better place for this comment?
--
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]
