Jianfeng Jia has posted comments on this change. Change subject: Carry filter in 2ndary-to-primary index search ......................................................................
Patch Set 6: (20 comments) https://asterix-gerrit.ics.uci.edu/#/c/1720/6/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/AccessMethodUtils.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/AccessMethodUtils.java: PS6, Line 358: here > What does this TODO item mean? removed. PS6, Line 358: // TODO look at here about the variables > Is this TODO necessary to this patch? Or just a TODO marker on local branch removed. https://asterix-gerrit.ics.uci.edu/#/c/1720/6/hyracks-fullstack/hyracks/hyracks-examples/hyracks-integration-tests/src/test/java/org/apache/hyracks/tests/am/btree/AbstractBTreeOperatorTest.java File hyracks-fullstack/hyracks/hyracks-examples/hyracks-integration-tests/src/test/java/org/apache/hyracks/tests/am/btree/AbstractBTreeOperatorTest.java: PS6, Line 66: .*; > expand the imports? Done https://asterix-gerrit.ics.uci.edu/#/c/1720/6/hyracks-fullstack/hyracks/hyracks-examples/hyracks-integration-tests/src/test/java/org/apache/hyracks/tests/am/btree/BTreePrimaryIndexScanOperatorTest.java File hyracks-fullstack/hyracks/hyracks-examples/hyracks-integration-tests/src/test/java/org/apache/hyracks/tests/am/btree/BTreePrimaryIndexScanOperatorTest.java: PS6, Line 43: .*; > expand import? Done PS6, Line 101: createSecondaryDataFlowHelperFactory > Is it necessary have two different methods that share the same code? The main requirement is to give the valid btreeFields and the filterFields if filter appears. I refactor the code to add these two as parameters for one interface function instead of two interfaces. https://asterix-gerrit.ics.uci.edu/#/c/1720/6/hyracks-fullstack/hyracks/hyracks-examples/hyracks-integration-tests/src/test/java/org/apache/hyracks/tests/am/btree/BTreePrimaryIndexSearchOperatorTest.java File hyracks-fullstack/hyracks/hyracks-examples/hyracks-integration-tests/src/test/java/org/apache/hyracks/tests/am/btree/BTreePrimaryIndexSearchOperatorTest.java: PS6, Line 44: DataSetConstants > expand import? Done https://asterix-gerrit.ics.uci.edu/#/c/1720/6/hyracks-fullstack/hyracks/hyracks-examples/hyracks-integration-tests/src/test/java/org/apache/hyracks/tests/am/rtree/AbstractRTreeOperatorTest.java File hyracks-fullstack/hyracks/hyracks-examples/hyracks-integration-tests/src/test/java/org/apache/hyracks/tests/am/rtree/AbstractRTreeOperatorTest.java: PS6, Line 26: .*; > expand imports Done https://asterix-gerrit.ics.uci.edu/#/c/1720/6/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/dataflow/BTreeSearchOperatorDescriptor.java File hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/dataflow/BTreeSearchOperatorDescriptor.java: PS6, Line 82: maxFilterFieldIndexes > if appendFilter is false, should we still have the two fields minFilterFiel These two types of filters are orthogonal. `minFilterFieldIndexes` and `maxFilterFieldIndexes` are indexes of the filter values in the input frame. They are given by the query parameters. `appendFilter` is to indicate if we need to append the index component filter values to the **output** frame. https://asterix-gerrit.ics.uci.edu/#/c/1720/6/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IIndexDataflowHelper.java File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/IIndexDataflowHelper.java: PS6, Line 44: public int getNumFilterFields(); > Is this method used for supporting multiple filters on an index? No. It returns the number of fields for a filter value. It mainly used for writing null filter values to the output. since we don't know how many fields should we write. https://asterix-gerrit.ics.uci.edu/#/c/1720/6/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexSearchOperatorNodePushable.java File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexSearchOperatorNodePushable.java: PS6, Line 266: int[] offsets = nonFilterTupleBuild.getFieldEndOffsets(); : for (int i = 0; i < offsets.length; i++) { : int start = i > 0 ? offsets[i - 1] : 0; : tb.addField(nonFilterTupleBuild.getByteArray(), start, offsets[i]); : } > Why would filter tuple == null? In Hyracks, the outputRecordDescriptor (i.e. how many fields in the record after an operator) is predefined. So if it said `append`, there must be a field waiting to be filled up. If we skip it, the bytearray will be messed up. We rely on the compiler to make a good plan. Hopefully, this kind of inconsistent will never happen. Here just a defensive logic in case something goes wrong... https://asterix-gerrit.ics.uci.edu/#/c/1720/6/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/dataflow/LSMInvertedIndexSearchOperatorDescriptor.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/dataflow/LSMInvertedIndexSearchOperatorDescriptor.java: PS6, Line 89: false > if appendFilter is false, why do you still need the two filter parameters? this false is to configure the output, the two filterIndex values are for the input. https://asterix-gerrit.ics.uci.edu/#/c/1720/6/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/ondisk/OnDiskInvertedIndexRangeSearchCursor.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/ondisk/OnDiskInvertedIndexRangeSearchCursor.java: PS6, Line 136: > always null? The filter only valid for the `LSMXXXCursor`. I was thinking of refactoring the relationship between the non-lsm and lsm cursor, but it turned out a huge task. So I end up add a interface for all cursors. All non-lsm cursors will return null. PS6, Line 141: null > always null? see the previous explanation :-) https://asterix-gerrit.ics.uci.edu/#/c/1720/6/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/ondisk/OnDiskInvertedIndexSearchCursor.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/ondisk/OnDiskInvertedIndexSearchCursor.java: PS6, Line 98: null > always null? see the previous explanation :-) PS6, Line 103: null > always null? see the previous explanation :-) https://asterix-gerrit.ics.uci.edu/#/c/1720/6/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/impls/LSMRTreeWithAntiMatterTuplesFlushCursor.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/impls/LSMRTreeWithAntiMatterTuplesFlushCursor.java: PS6, Line 149: null > always null? this case is slightly different, this is a `flush` cursor, which doesn't have the components information. and this method shouldn't be called. PS6, Line 154: null > always null? see the previous explanation :-) https://asterix-gerrit.ics.uci.edu/#/c/1720/6/hyracks-fullstack/hyracks/hyracks-storage-am-rtree/src/main/java/org/apache/hyracks/storage/am/rtree/dataflow/RTreeSearchOperatorDescriptor.java File hyracks-fullstack/hyracks/hyracks-storage-am-rtree/src/main/java/org/apache/hyracks/storage/am/rtree/dataflow/RTreeSearchOperatorDescriptor.java: PS6, Line 77: false > if appendFilter is always false, this constructor doesn't seem to need int[ see the previous explanation :-) https://asterix-gerrit.ics.uci.edu/#/c/1720/6/hyracks-fullstack/hyracks/hyracks-storage-am-rtree/src/main/java/org/apache/hyracks/storage/am/rtree/impls/RTreeSearchCursor.java File hyracks-fullstack/hyracks/hyracks-storage-am-rtree/src/main/java/org/apache/hyracks/storage/am/rtree/impls/RTreeSearchCursor.java: PS6, Line 84: null > always null? see the previous explanation :-) PS6, Line 88: ITupleReference > always null? see the previous explanation :-) -- To view, visit https://asterix-gerrit.ics.uci.edu/1720 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I287f1dbd230aa649f1350114abf0a1d47e2bb53c Gerrit-PatchSet: 6 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Jianfeng Jia <[email protected]> Gerrit-Reviewer: Ian Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Jianfeng Jia <[email protected]> Gerrit-Reviewer: Luo Chen <[email protected]> Gerrit-Reviewer: Yingyi Bu <[email protected]> Gerrit-HasComments: Yes
