asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jianfeng Jia (Code Review)" <>
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:

File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/

PS6, Line 358: here
> What does this TODO item mean?

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
File hyracks-fullstack/hyracks/hyracks-examples/hyracks-integration-tests/src/test/java/org/apache/hyracks/tests/am/btree/

PS6, Line 66: .*;
> expand the imports?
File hyracks-fullstack/hyracks/hyracks-examples/hyracks-integration-tests/src/test/java/org/apache/hyracks/tests/am/btree/

PS6, Line 43: .*;
> expand import?

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
File hyracks-fullstack/hyracks/hyracks-examples/hyracks-integration-tests/src/test/java/org/apache/hyracks/tests/am/btree/

PS6, Line 44: DataSetConstants
> expand import?
File hyracks-fullstack/hyracks/hyracks-examples/hyracks-integration-tests/src/test/java/org/apache/hyracks/tests/am/rtree/

PS6, Line 26: .*;
> expand imports
File hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/dataflow/

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.
File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/

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.
File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/

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...
File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/dataflow/

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.
File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/ondisk/

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 :-)
File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/ondisk/

PS6, Line 98: null
> always null?
see the previous explanation :-)

PS6, Line 103: null
> always null?
see the previous explanation :-)
File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/impls/

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 :-)
File hyracks-fullstack/hyracks/hyracks-storage-am-rtree/src/main/java/org/apache/hyracks/storage/am/rtree/dataflow/

PS6, Line 77: false
> if appendFilter is always false, this constructor doesn't seem to need int[
see the previous explanation :-)
File hyracks-fullstack/hyracks/hyracks-storage-am-rtree/src/main/java/org/apache/hyracks/storage/am/rtree/impls/

PS6, Line 84: null
> always null?
see the previous explanation :-)

PS6, Line 88: ITupleReference
> always null?
see the previous explanation :-)

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I287f1dbd230aa649f1350114abf0a1d47e2bb53c
Gerrit-PatchSet: 6
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Jianfeng Jia <>
Gerrit-Reviewer: Ian Maxon <>
Gerrit-Reviewer: Jenkins <>
Gerrit-Reviewer: Jianfeng Jia <>
Gerrit-Reviewer: Luo Chen <>
Gerrit-Reviewer: Yingyi Bu <>
Gerrit-HasComments: Yes

View raw message