You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@asterixdb.apache.org by "Yingyi Bu (Code Review)" <do...@asterixdb.incubator.apache.org> on 2016/02/10 02:39:17 UTC

Change in hyracks[master]: Intersect the 2ndary indexes before primary search

Yingyi Bu has posted comments on this change.

Change subject: Intersect the 2ndary indexes before primary search
......................................................................


Patch Set 2:

(8 comments)

https://asterix-gerrit.ics.uci.edu/#/c/577/2/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/IntersectOperator.java
File algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/IntersectOperator.java:

Line 92:         //TODO introduce an UNION type if possible
Why do we need a UNION type?


Line 93:         IVariableTypeEnvironment env = createPropagatingAllInputsTypeEnvironment(ctx);
Why do we need to propagate all input types?
It seems the returned env just need to keep track of the types of output variables.


Line 108:     public List<LogicalVariable> getInputVariables(int i) {
i->inputIndex


Line 112:     private void checkIfTwoTypeEnvIsEqual(IVariableTypeEnvironment expected, List<LogicalVariable> expectedVariables,
checkIfTwoTypeEnvIsEqual --> checkTypeConsistency

"checkIfTwoTypeEnvIsEqual" makes me think that you're going to check whether two IVariableTypeEnvironments are equal.


Line 118:                 AlgebricksConfig.ALGEBRICKS_LOGGER.warning("Type of two variables are not equal." +
Throw an exception for that case?
Different indexes should return the same type for the primary key variables?


https://asterix-gerrit.ics.uci.edu/#/c/577/2/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/visitors/LogicalOperatorDeepCopyWithNewVariablesVisitor.java
File algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/visitors/LogicalOperatorDeepCopyWithNewVariablesVisitor.java:

Line 434:         List<List<LogicalVariable>> liveVarsInInputs = getLiveVarsInInputs(op);
getLiveVarsInInputs --> visitInputs
because the method also does deep copy of the inputs.


https://asterix-gerrit.ics.uci.edu/#/c/577/2/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/physical/IntersectPOperator.java
File algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/physical/IntersectPOperator.java:

Line 75:             // when should we care about the parent requirement?
This method is to express the required properties for its children.
I think the logic here should be more or less similar to HybridHashJoinPOperator, which require all its children to be partitioned based on the primary key variables.


Line 89:         List<ILocalStructuralProperty> propsLocal = op.getDeliveredPhysicalProperties().getLocalProperties();
The property of the first child is based on its available variables. Therefore, you might want to turn them to be based on the delivered variables of the intersect operator.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic16c67c529ca19d8b1a5439ddef22760945fd0d7
Gerrit-PatchSet: 2
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: Jianfeng Jia <ji...@gmail.com>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Jianfeng Jia <ji...@gmail.com>
Gerrit-Reviewer: Taewoo Kim <wa...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-HasComments: Yes