gerlowskija commented on code in PR #3922:
URL: https://github.com/apache/solr/pull/3922#discussion_r2594016342


##########
solr/benchmark/src/resources/configs/cloud-minimal/conf/schema.xml:
##########
@@ -15,11 +15,13 @@
  See the License for the specific language governing permissions and
  limitations under the License.
 -->
-<schema name="minimal" version="1.7">
+<schema name="minimal" version="1.6">

Review Comment:
   [Q] Why is this going backwards from 1.7?



##########
solr/core/src/java/org/apache/solr/schema/PointField.java:
##########
@@ -158,15 +167,23 @@ public Query getFieldQuery(QParser parser, SchemaField 
field, String externalVal
       // currently implemented as singleton range
       return getRangeQuery(parser, field, externalVal, externalVal, true, 
true);
     } else if (field.indexed() && field.hasDocValues()) {
-      Query pointsQuery = getExactQuery(field, externalVal);
+      Query indexQuery = getExactQuery(parser, field, externalVal);
       Query dvQuery = getDocValuesRangeQuery(parser, field, externalVal, 
externalVal, true, true);
-      return new IndexOrDocValuesQuery(pointsQuery, dvQuery);
+      return new IndexOrDocValuesQuery(indexQuery, dvQuery);
     } else {
-      return getExactQuery(field, externalVal);
+      return getExactQuery(parser, field, externalVal);
     }
   }
 
-  protected abstract Query getExactQuery(SchemaField field, String 
externalVal);
+  final Query getExactQuery(QParser parser, SchemaField field, String 
externalVal) {
+    if (hasIndexedTerms(field)) {
+      BytesRefBuilder br = new BytesRefBuilder();
+      readableToIndexed(externalVal, br);
+      return new TermQuery(new Term(field.getName(), br));
+    } else {
+      return getPointRangeQuery(parser, field, externalVal, externalVal, true, 
true);

Review Comment:
   [Q] Prior to this PR, most of the implementing subclasses handled the 
no-terms-index case by calling Lucene's `IntPoint.newExactQuery(...)`.  
Currently, that factory also creates a range-query under the hood, so it's 
(afaict) functionally equivalent to the `getPointRangeQuery` call here.
   
   If the code's largely equivalent then, is there a reason for the switch?  
The Lucene `IntPoint.newExactQuery` call gives us a bit less control I guess, 
but it has the benefit of being code that we don't have to maintain ourselves.
   
   Are there optimizations in `getPointRangeQuery` that the Lucene equivalent 
doesn't have?  Or are there other reasons that make the switch worthwhile?



##########
solr/benchmark/src/java/org/apache/solr/bench/search/NumericSearch.java:
##########
@@ -147,20 +147,24 @@ public void 
setupIteration(MiniClusterState.MiniClusterBenchState miniClusterSta
       
miniClusterState.client.requestWithBaseUrl(miniClusterState.nodes.get(0), 
reload, null);
     }
 
-    public QueryRequest intSetQuery(boolean dvs) {
-      return setQuery("numbers_i" + (dvs ? "_dv" : ""));
+    public QueryRequest intTrieSetQuery(boolean dvs, boolean enhancedIndex) {

Review Comment:
   [Q] Why provide the "enhancedIndex" flag here and then not use it?



-- 
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]

Reply via email to