Ian Maxon has posted comments on this change.

Change subject: [ASTERIXDB-2339] Add a new inverted index merge cursor
......................................................................


Patch Set 3:

(3 comments)

good stuff, couple questions

https://asterix-gerrit.ics.uci.edu/#/c/2519/3/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/impls/LSMInvertedIndexMergeCursor.java
File 
hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/impls/LSMInvertedIndexMergeCursor.java:

PS3, Line 242:    protected void checkKeyQueue() throws HyracksDataException {
             :         while (!keyQueue.isEmpty() || 
needPushElementIntoKeyQueue) {
             :             if (!keyQueue.isEmpty()) {
             :                 PriorityQueueElement checkElement = 
keyQueue.peek();
             :                 // If there is no previous tuple or the previous 
tuple can be ignored
             :                 if (outputKeyElement == null) {
             :                     if (isDeleted(checkElement)) {
             :                         // If the key has been deleted then pop 
it and set needPush to true.
             :                         // We cannot push immediately because 
the tuple may be
             :                         // modified if hasNext() is called
             :                         outputKeyElement = checkElement;
             :                         needPushElementIntoKeyQueue = true;
             :                     } else {
             :                         // we have found the next record
             :                         return;
             :                     }
             :                 } else {
             :                     // Compare the previous tuple and the head 
tuple in the PQ
             :                     if 
(keyCmp.compare(outputKeyElement.getTuple(), checkElement.getTuple()) == 0) {
             :                         // If the previous tuple and the head 
tuple are
             :                         // identical
             :                         // then pop the head tuple and push the 
next tuple from
             :                         // the tree of head tuple
             : 
             :                         // the head element of PQ is useless now
             :                         PriorityQueueElement e = keyQueue.poll();
             :                         pushIntoKeyQueueAndReplace(e);
             :                     } else {
             :                         // If the previous tuple and the head 
tuple are different
             :                         // the info of previous tuple is useless
             :                         if (needPushElementIntoKeyQueue) {
             :                             
pushIntoKeyQueueAndReplace(outputKeyElement);
             :                             needPushElementIntoKeyQueue = false;
             :                         }
             :                         outputKeyElement = null;
             :                     }
             :                 }
             :             } else {
             :                 // the priority queue is empty and needPush
             :                 pushIntoKeyQueueAndReplace(outputKeyElement);
             :                 needPushElementIntoKeyQueue = false;
             :                 outputKeyElement = null;
             :             }
             :         }
             :     }
no way to deduplicate this from checkPriorityQueue()?


https://asterix-gerrit.ics.uci.edu/#/c/2519/3/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/impls/LSMInvertedIndexRangeSearchCursor.java
File 
hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/impls/LSMInvertedIndexRangeSearchCursor.java:

PS3, Line 117: doClose(
what's the rationale behind changing this method?


https://asterix-gerrit.ics.uci.edu/#/c/2519/3/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/ondisk/OnDiskInvertedIndex.java
File 
hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/ondisk/OnDiskInvertedIndex.java:

PS3, Line 319:    if (tuple instanceof TokenKeyPairTuple) {
             :                 addTokenKeyPairTuple((TokenKeyPairTuple) tuple);
             :             } else {
             :                 addTuple(tuple);
             :             }
Is there some other way to do this besides instanceof?


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/2519
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I57d039cd7e08033884529a204bff9acffd96d9bb
Gerrit-PatchSet: 3
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Luo Chen <cl...@uci.edu>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Ian Maxon <ima...@apache.org>
Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Taewoo Kim <taew...@uci.edu>
Gerrit-Reviewer: abdullah alamoudi <bamou...@gmail.com>
Gerrit-HasComments: Yes

Reply via email to