----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37720/#review96205 -----------------------------------------------------------
Ship it! Looks pretty good - I had one comment on the use of hashcode, below. gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/mergers/TopDocsResultMerger.java (line 29) <https://reviews.apache.org/r/37720/#comment151527> 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. - Dan Smith 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 > >
