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 34:

(14 comments)

Wenhai, here are my other comments. Things gets more clearer now.

https://asterix-gerrit.ics.uci.edu/#/c/1076/34/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 166:         List<Mutable<ILogicalExpression>> inputExps = 
simFuncExpr.getArguments();
inputExp -> inputExprs


Line 199:         // leftInputPKs extract all the PKs derived from the 
downstream of the left branch.
Please rephrase these statements. Also, can you put more explanations on 
"previous" and "current"? How does that work?


Line 206:         Collection<LogicalVariable> currentPK = new HashSet<>();
currentPK -> currentPKs


Line 219:         // Avoid the duplicated pk generation in 
findPrimaryKeysInSubplan, especially for multiway fuzzy join.
pk -> PK


Line 223:         if (leftInputPKs == null || rightInputPKs == null) {
Check the above SonarQube comment. leftInputPKs or rightInputPKs will not be 
null always. So, "leftInputPKs == null" condition should be changed to 
leftInputPKs.isEmpty().


Line 229:         // left-hand side and right-hand side of "~=" has the same 
type
"~=" -> fuzzy join


Line 230:         IAType left2 = TypeComputeUtils.getActualType(leftType);
left2 -> actualLeftType


Line 231:         IAType right2 = TypeComputeUtils.getActualType(rightType);
right2 -> actualRightType


Line 304:                 throw new AlgebricksException("The given condition 
for the fuzzy join (" + joinOp.getJoinKind()
Follow the new Exception Handling proposal. AlgebricksException should be 
replaced with CompilationException.
https://cwiki.apache.org/confluence/display/ASTERIXDB/Exception+Handling


Line 311:         // the shared token to the actually tuples of both side. 
left(right)InputPK is used to extract those mappings
left(right)InputPK -> left(right)InputPKs


Line 382:         for (int i = 1; i < 4; i++) {
initialize constant 1 as like static final int STAGE_1_INDEX = 1;

initialize constant 3 as like static final int STAGE_3_INDEX = 3;

And let's use i <= 3 instead of i < 4;

Otherwise, it's confusing.


Line 417:     // Since the above substitution (line 150) generates the 
prefix-based join operators for the partial simJoin
Rather than using a line number such as 150, please refer to the method or 
exact statement.


Line 418:     // of expRef, we need to add the full condition 
expRef\getItemExprRef into the top-level operator of the plan.
It would be nice if you can provide an example here.


Line 478:     @Override public boolean rewritePre(Mutable<ILogicalOperator> 
opRef, IOptimizationContext context)
This method is not required since the it just repeats the default method of the 
parent here.


-- 
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: 34
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