asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jianfeng Jia (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
Subject Change in asterixdb[master]: Carry filter in 2ndary-to-primary index search
Date Thu, 04 May 2017 07:37:37 GMT
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 <jianfeng.jia@gmail.com>
Gerrit-Reviewer: Ian Maxon <imaxon@apache.org>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Jianfeng Jia <jianfeng.jia@gmail.com>
Gerrit-Reviewer: Luo Chen <cluo8@uci.edu>
Gerrit-Reviewer: Yingyi Bu <buyingyi@gmail.com>
Gerrit-HasComments: Yes

Mime
View raw message