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