> On Aug. 24, 2015, 8:59 p.m., Dan Smith wrote:
> > gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/mergers/TopDocsResultMerger.java,
> >  line 29
> > <https://reviews.apache.org/r/37720/diff/1/?file=1048453#file1048453line29>
> >
> >     Is is possible to lose a document if two keys have the same hash code? 
> > If so, this risks losing a doc. If not, why include this hashcode 
> > comparison? Probably better to leave it out and just test adding docs with 
> > duplicate scores. I believe priority queue should handle this.
> 
> Ashvin A wrote:
>     Good point. Based on my understanding, the merger will operate on 
> disjoint keysets received from shards. So I guess I should not worry about 
> duplicate keys while merging and skip this check.
>     
>     While this is true, the comparator is used for ordering the results and 
> will not loose any doc. I will add a test for duplicate keys for coverage.
>     
>     Thanks !

I misunderstood your comment. So ignore my earlier reply. A doc will not be 
lost if two keys with the same hashcode are seen. I have added a test for this.


- Ashvin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37720/#review96205
-----------------------------------------------------------


On Aug. 24, 2015, 5:25 p.m., Ashvin A wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37720/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2015, 5:25 p.m.)
> 
> 
> Review request for geode and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> Aggregator merges results returned from shards and orders the result based on 
> lucene hit score
> 
> 
> Diffs
> -----
> 
>   
> gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/LuceneQueryResults.java
>  d660a4b 
>   
> gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/LuceneResultStruct.java
>  a5b16b7 
>   
> gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneQueryResultsImpl.java
>  ecb6370 
>   
> gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneResultStructImpl.java
>  0db8f97 
>   
> gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/mergers/ResultMerger.java
>  PRE-CREATION 
>   
> gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/mergers/TopDocsResultMerger.java
>  PRE-CREATION 
>   
> gemfire-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/TopDocsMergeJUnitTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37720/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ashvin A
> 
>

Reply via email to