cpoerschke commented on a change in pull request #175:
URL: https://github.com/apache/solr/pull/175#discussion_r655572267



##########
File path: solr/core/src/java/org/apache/solr/search/HashQParserPlugin.java
##########
@@ -114,7 +116,12 @@ public LongValues getValues(LeafReaderContext ctx, 
DoubleValues scores) throws I
             @Override
             public long longValue() throws IOException {
               //TODO: maybe cache hashCode if same ord as prev doc to save 
lookupOrd?
-              return atDoc ? values.lookupOrd(values.ordValue()).hashCode() : 
0;
+              return atDoc ? hashCode(values.lookupOrd(values.ordValue())) : 0;
+            }
+
+            private long hashCode(BytesRef bytesRef) {
+              // Use deterministic hashCode (seed is a constant).  
BytesRef.hashCode() varies!
+              return StringHelper.murmurhash3_x86_32(bytesRef, 0);

Review comment:
       Interestingly it seems that two other places do something similar i.e. 
https://github.com/apache/lucene-solr/blob/releases/lucene-solr/8.9.0/solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java#L273-L276
 and 
https://github.com/apache/lucene-solr/blob/releases/lucene-solr/8.9.0/solr/core/src/java/org/apache/solr/search/join/HashRangeQuery.java#L106-L109
 and whilst at first it seems that `murmurhash3_x86_32(byte[] data, int offset, 
int len, int seed)` in 
https://github.com/apache/lucene-solr/blob/releases/lucene-solr/8.9.0/solr/solrj/src/java/org/apache/solr/common/util/Hash.java#L243
 duplicates 
https://github.com/apache/lucene-solr/blob/releases/lucene-solr/8.9.0/lucene/core/src/java/org/apache/lucene/util/StringHelper.java#L149
 this could be for callers that wish to avoid a `lucene/core` dependency and/or 
maybe there are implementation variations too.

##########
File path: solr/core/src/java/org/apache/solr/search/HashQParserPlugin.java
##########
@@ -114,7 +116,12 @@ public LongValues getValues(LeafReaderContext ctx, 
DoubleValues scores) throws I
             @Override
             public long longValue() throws IOException {
               //TODO: maybe cache hashCode if same ord as prev doc to save 
lookupOrd?
-              return atDoc ? values.lookupOrd(values.ordValue()).hashCode() : 
0;
+              return atDoc ? hashCode(values.lookupOrd(values.ordValue())) : 0;
+            }
+
+            private long hashCode(BytesRef bytesRef) {

Review comment:
       ```suggestion
               private static int hashCode(BytesRef bytesRef) {
   ```
   
   or
   
   ```suggestion
               private static long hashCode(BytesRef bytesRef) {
   ```




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

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org

Reply via email to