asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Taewoo Kim (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
Subject Change in asterixdb[master]: Index-only plan step 2: Added SplitOperator
Date Thu, 29 Sep 2016 22:24:48 GMT
Taewoo Kim has posted comments on this change.

Change subject: Index-only plan step 2: Added SplitOperator
......................................................................


Patch Set 10:

(6 comments)

@Jianfeng: thanks for the comments. Regarding your first comments, I think we may need to
check https://github.com/apache/asterixdb/commit/f6596f23ce61eb3b430ba6d9a33c6265ec4b39c4
patch first.

https://asterix-gerrit.ics.uci.edu/#/c/1196/10/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/SplitOperator.java
File hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/SplitOperator.java:

Line 32:     private Mutable<ILogicalExpression> branchingExpression;
> Can this one be Immutable? we should not change the expression.
Done


https://asterix-gerrit.ics.uci.edu/#/c/1196/10/hyracks-fullstack/algebricks/algebricks-runtime/src/main/java/org/apache/hyracks/algebricks/runtime/operators/std/SplitOperatorDescriptor.java
File hyracks-fullstack/algebricks/algebricks-runtime/src/main/java/org/apache/hyracks/algebricks/runtime/operators/std/SplitOperatorDescriptor.java:

Line 73:             builder.addTargetEdge(i, sma, pipelineOutputIndex++);
> isn't this `pipelineOutputIndex` == `i`?
Done


Line 91:                 private final boolean[] isOpen = new boolean[numberOfNonMaterializedOutputs];
> where does this `numberOfNonMaterializedOutputs` come from?
Initialized in the super class - AbstractReplicateOperatorDescriptor. For this split operator,
the number of output is the same as numberOfNonMaterializedOutputs.


Line 111:                     builders = new ArrayTupleBuilder[numberOfNonMaterializedOutputs];
> this builder seems not be used
Done


Line 112:                     builderDatas = new GrowableArray[numberOfNonMaterializedOutputs];
> ditto
Done


https://asterix-gerrit.ics.uci.edu/#/c/1196/10/hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/base/AbstractReplicateOperatorDescriptor.java
File hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/base/AbstractReplicateOperatorDescriptor.java:

Line 70:         for (boolean flag : outputMaterializationFlags) {
> I'm confused by the original code. Based on the outputMaterializationFlags 
This was introduced by https://github.com/apache/asterixdb/commit/f6596f23ce61eb3b430ba6d9a33c6265ec4b39c4.
So, you may check there. Frankly, I'm not understanding the code 100% for this part.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ice190827513cd8632764b52c9d0338d65c830740
Gerrit-PatchSet: 10
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Taewoo Kim <wangsaeu@yahoo.com>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Jianfeng Jia <jianfeng.jia@gmail.com>
Gerrit-Reviewer: Taewoo Kim <wangsaeu@yahoo.com>
Gerrit-Reviewer: Till Westmann <tillw@apache.org>
Gerrit-Reviewer: Yingyi Bu <buyingyi@gmail.com>
Gerrit-HasComments: Yes

Mime
View raw message