>From Wail Alkowaileet <[email protected]>: Wail Alkowaileet has submitted this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17455 )
Change subject: [ASTERIXDB-3159][STO] Fix point lookups on columnar dataseet ...................................................................... [ASTERIXDB-3159][STO] Fix point lookups on columnar dataseet - user model changes: no - storage format changes: no - interface changes: no Details: This patch fixes the issue of running out of values when point lookups are performed against columnar datasets. The issue was that the next point lookup (using the stateful batch point lookup) would proceed without testing if: - the tuple key == the required key Change-Id: I46075b62daa2389f83f214cbe76e41a48763b9ed Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17455 Reviewed-by: Murtadha Hubail <[email protected]> Integration-Tests: Jenkins <[email protected]> Tested-by: Jenkins <[email protected]> --- M asterixdb/asterix-column/src/main/java/org/apache/asterix/column/assembler/PrimitiveValueAssembler.java M hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree-column/src/main/java/org/apache/hyracks/storage/am/lsm/btree/column/impls/btree/ColumnBTreePointSearchCursor.java M asterixdb/asterix-column/src/main/java/org/apache/asterix/column/assembler/RepeatedPrimitiveValueAssembler.java M asterixdb/asterix-column/src/main/java/org/apache/asterix/column/operation/query/ColumnAssembler.java M hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree-column/src/main/java/org/apache/hyracks/storage/am/lsm/btree/column/impls/btree/ColumnBTreeRangeSearchCursor.java 5 files changed, 54 insertions(+), 11 deletions(-) Approvals: Murtadha Hubail: Looks good to me, approved Jenkins: Verified; Verified Anon. E. Moose #1000171: diff --git a/asterixdb/asterix-column/src/main/java/org/apache/asterix/column/assembler/PrimitiveValueAssembler.java b/asterixdb/asterix-column/src/main/java/org/apache/asterix/column/assembler/PrimitiveValueAssembler.java index 9592a12..4f7778c 100644 --- a/asterixdb/asterix-column/src/main/java/org/apache/asterix/column/assembler/PrimitiveValueAssembler.java +++ b/asterixdb/asterix-column/src/main/java/org/apache/asterix/column/assembler/PrimitiveValueAssembler.java @@ -31,7 +31,7 @@ @Override public int next() throws HyracksDataException { if (!reader.next()) { - throw new IllegalAccessError("no more values"); + throw new IllegalAccessError("no more values, column index: " + getColumnIndex()); } else if (reader.isNull() && (isDelegate() || reader.getLevel() + 1 == level)) { addNullToAncestor(reader.getLevel()); } else if (reader.isValue()) { diff --git a/asterixdb/asterix-column/src/main/java/org/apache/asterix/column/assembler/RepeatedPrimitiveValueAssembler.java b/asterixdb/asterix-column/src/main/java/org/apache/asterix/column/assembler/RepeatedPrimitiveValueAssembler.java index 8fa228f..d4455ec 100644 --- a/asterixdb/asterix-column/src/main/java/org/apache/asterix/column/assembler/RepeatedPrimitiveValueAssembler.java +++ b/asterixdb/asterix-column/src/main/java/org/apache/asterix/column/assembler/RepeatedPrimitiveValueAssembler.java @@ -41,7 +41,7 @@ @Override public int next() throws HyracksDataException { if (!reader.next()) { - throw new IllegalStateException("No more values"); + throw new IllegalAccessError("no more values, column index: " + getColumnIndex()); } else if (reader.isNull() && (!arrays.isEmpty() || reader.getLevel() + 1 == level)) { /* * There are two cases here for where the null belongs to: diff --git a/asterixdb/asterix-column/src/main/java/org/apache/asterix/column/operation/query/ColumnAssembler.java b/asterixdb/asterix-column/src/main/java/org/apache/asterix/column/operation/query/ColumnAssembler.java index c329d67..2e60ee7 100644 --- a/asterixdb/asterix-column/src/main/java/org/apache/asterix/column/operation/query/ColumnAssembler.java +++ b/asterixdb/asterix-column/src/main/java/org/apache/asterix/column/operation/query/ColumnAssembler.java @@ -92,6 +92,7 @@ } public void skip(int count) throws HyracksDataException { + tupleIndex += count; for (int i = 0; i < assemblers.size(); i++) { assemblers.get(i).skip(count); } diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree-column/src/main/java/org/apache/hyracks/storage/am/lsm/btree/column/impls/btree/ColumnBTreePointSearchCursor.java b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree-column/src/main/java/org/apache/hyracks/storage/am/lsm/btree/column/impls/btree/ColumnBTreePointSearchCursor.java index c93e77e..683e099 100644 --- a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree-column/src/main/java/org/apache/hyracks/storage/am/lsm/btree/column/impls/btree/ColumnBTreePointSearchCursor.java +++ b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree-column/src/main/java/org/apache/hyracks/storage/am/lsm/btree/column/impls/btree/ColumnBTreePointSearchCursor.java @@ -33,6 +33,17 @@ } @Override + public boolean doHasNext() { + // If we found the exact key, return true + return yieldFirstCall; + } + + @Override + protected boolean shouldYieldFirstCall() throws HyracksDataException { + return pred.getLowKeyComparator().compare(lowKey, frameTuple) == 0; + } + + @Override public void doClose() throws HyracksDataException { pageId = IBufferCache.INVALID_PAGEID; super.doClose(); diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree-column/src/main/java/org/apache/hyracks/storage/am/lsm/btree/column/impls/btree/ColumnBTreeRangeSearchCursor.java b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree-column/src/main/java/org/apache/hyracks/storage/am/lsm/btree/column/impls/btree/ColumnBTreeRangeSearchCursor.java index 7ebdc8b..3323494 100644 --- a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree-column/src/main/java/org/apache/hyracks/storage/am/lsm/btree/column/impls/btree/ColumnBTreeRangeSearchCursor.java +++ b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree-column/src/main/java/org/apache/hyracks/storage/am/lsm/btree/column/impls/btree/ColumnBTreeRangeSearchCursor.java @@ -53,7 +53,7 @@ protected RangePredicate pred; protected ITupleReference lowKey; protected ITupleReference highKey; - protected boolean firstNextCall; + protected boolean yieldFirstCall; protected final IIndexCursorStats stats; @@ -92,7 +92,7 @@ @Override public boolean doHasNext() throws HyracksDataException { int nextLeafPage; - if (frameTuple.isConsumed() && !firstNextCall) { + if (frameTuple.isConsumed() && !yieldFirstCall) { frameTuple.lastTupleReached(); nextLeafPage = frame.getNextLeaf(); if (nextLeafPage >= 0) { @@ -132,14 +132,14 @@ reusablePredicate.setLowKeyComparator(originalKeyCmp); reusablePredicate.setHighKeyComparator(pred.getHighKeyComparator()); reusablePredicate.setHighKey(pred.getHighKey(), pred.isHighKeyInclusive()); - firstNextCall = true; + yieldFirstCall = false; advanceTupleToLowKey(); } protected boolean isNextIncluded() throws HyracksDataException { - if (firstNextCall) { + if (yieldFirstCall) { //The first call of frameTuple.next() was done during the opening of the cursor - firstNextCall = false; + yieldFirstCall = false; return true; } else if (frameTuple.isConsumed()) { //All tuple were consumed @@ -162,12 +162,10 @@ * Then: * No tuple will satisfy the search key. Consume the frameTuple to stop the search. */ - firstNextCall = false; frameTuple.consume(); return; } else if (lowKey == null) { // no lowKey was specified, start from tupleIndex = 0 - frameTuple.next(); return; } @@ -179,12 +177,20 @@ stop = isLessOrEqual(lowKey, frameTuple, pred.isLowKeyInclusive()); counter++; } - //Advance all columns to the proper position if needed - if (counter - 1 > 0) { + + // Only proceed if needed + yieldFirstCall = shouldYieldFirstCall(); + // Advance all columns to the proper position if needed + if (yieldFirstCall && counter - 1 > 0) { frameTuple.skip(counter - 1); } } + protected boolean shouldYieldFirstCall() throws HyracksDataException { + // Proceed if the highKey is null or the current tuple's key is less than (or equal) the highKey + return highKey == null || isLessOrEqual(frameTuple, highKey, pred.isHighKeyInclusive()); + } + protected void releasePages() throws HyracksDataException { //Unpin all column pages first frameTuple.unpinColumnsPages(); -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17455 To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Change-Id: I46075b62daa2389f83f214cbe76e41a48763b9ed Gerrit-Change-Number: 17455 Gerrit-PatchSet: 3 Gerrit-Owner: Wail Alkowaileet <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Murtadha Hubail <[email protected]> Gerrit-Reviewer: Wail Alkowaileet <[email protected]> Gerrit-MessageType: merged
