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 <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Jianfeng Jia <[email protected]>
Gerrit-Reviewer: Taewoo Kim <[email protected]>
Gerrit-Reviewer: Yingyi Bu <[email protected]>
Gerrit-Reviewer: Yingyi Bu <[email protected]>
Gerrit-HasComments: Yes

Reply via email to