Taewoo Kim has posted comments on this change.

Change subject: Applied the multiway fuzzyjoin based on the prefix-based join 
and the selectFuzzyJoin testCases.
......................................................................


Patch Set 10:

(15 comments)

https://asterix-gerrit.ics.uci.edu/#/c/1076/10/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/FuzzyJoinRule.java
File 
asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/FuzzyJoinRule.java:

Line 83:     // Step1: Initial the host embedding aql to substitute the fuzzy 
euqal condition such as $r.a ~= $s.b
What does "Initial the host embedding aql" mean?


Line 88:             + "((#LEFT_%d_0), " + "  (join((#RIGHT_%d_0), "
%d change is necessary because?


Line 201:         VariableUtilities.getLiveVariables(rightInputOp, liveVars);
What do we want to achieve for this change (line 201 - 220)? Can you put more 
comments to explain?


Line 250:         if (constVal.getObject() instanceof AFloat) {
instanceOf can be replaced by using constVal.getObject() and BuiltinType.


Line 253:             simThreshold = 
FuzzyUtils.getSimThreshold(metadataProvider);
Do we have this case?


Line 342:         for (int i = 1; i < 4; i++) {
Why this is fixed number? i < 4?


Line 378:         // Step 5. Bind the plan to the father op referred by the 
below opRef.
Can we put an example here? "the father op referred by the below opRef." is 
hard to understand.


Line 408:     private void setConditionForLeftOuterJoin(LeftOuterJoinOperator 
topJoin, Mutable<ILogicalExpression> expRef) {
In the above, can we put some comments? What does this function do?


Line 441:                     Mutable<ILogicalExpression> expRefRet = 
getSimilarityExpression(arg);
Is this change necessary?


Line 451:     private List<LogicalVariable> 
findPrimaryKeysInSubplan(Collection<LogicalVariable> liveVars,
In the above, can we put some comments? What does this function do?


https://asterix-gerrit.ics.uci.edu/#/c/1076/10/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/subplan/InlineSubplanInputForNestedTupleSourceRule.java
File 
asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/subplan/InlineSubplanInputForNestedTupleSourceRule.java:

Line 256: public class InlineSubplanInputForNestedTupleSourceRule implements 
IAlgebraicRewriteRule {
Why the "hasRun" boolean needs to be removed? Can you explain this to me?


https://asterix-gerrit.ics.uci.edu/#/c/1076/10/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/common/APIFramework.java
File 
asterixdb/asterix-app/src/main/java/org/apache/asterix/api/common/APIFramework.java:

Line 313:                 QueryLogicalExpressionJobGen.INSTANCE));
Not necessary change - can you check your formatter?


https://asterix-gerrit.ics.uci.edu/#/c/1076/10/asterixdb/asterix-app/src/test/resources/optimizerts/queries/fj-dblp-csx-hybrid.aql
File 
asterixdb/asterix-app/src/test/resources/optimizerts/queries/fj-dblp-csx-hybrid.aql:

Line 18:  */
Can you a description of this test? What it is intended for? And the expected 
result?

An example: 
/apache-asterixdb/asterix-app/src/test/resources/runtimets/queries/index-join/btree-primary-equi-join/btree-primary-equi-join.1.ddl.aql

Actually, this should be applied to the all test cases.


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

Line 81:     private static ILogicalOperator 
extractPKProduction(ILogicalOperator root, LogicalVariable pk)
Please add some comments in the above - what does this function do?


Line 105:     public static void mergeHomogeneousPK(ILogicalOperator op, 
List<LogicalVariable> pks) throws AlgebricksException {
Please add some comments in the above - what does this function do?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8736f104905eeda763d39709e002c2b9629278cc
Gerrit-PatchSet: 10
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Wenhai Li <[email protected]>
Gerrit-Reviewer: Chen Li <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Taewoo Kim <[email protected]>
Gerrit-Reviewer: Wenhai Li <[email protected]>
Gerrit-HasComments: Yes

Reply via email to