Taewoo Kim has posted comments on this change. Change subject: Full-text implementation step 1 ......................................................................
Patch Set 11: (14 comments) Thanks Ian! https://asterix-gerrit.ics.uci.edu/#/c/1228/11/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/FullTextContainsParameterCheckRule.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/FullTextContainsParameterCheckRule.java: Line 131: && ConstantExpressionUtil.getConstantIaObjectType(firstExpr) != ATypeTag.STRING) { > "The first expression should be a string" Sure. It looks like I was sleeping. :-) Line 144: default: > "The second expression should be a string, unordered list, or ordered list. Done Line 167: if (openRecConsExpr.getArguments().size() > FullTextContainsDescriptor.paramTypeMap.size() * 2) { > Why is it * 2? Because opRecConsExpr.getArguments() is like [expr,val,expr, Yes. You are correct. Line 176: throw new AlgebricksException( > "Options must be in the form of constant strings. Check that the option at Done Line 181: if (!FullTextContainsDescriptor.paramTypeMap.containsKey(option)) { > The English here is not right, it might be something like "The given option Again, I was sleeping. :-) Thanks. Line 206: throw new AlgebricksException( > "The given value for option " + option + " was not of the expected type" Done Line 225: boolean checkSearchModeOption(String optionVal) throws AlgebricksException { > Why's this a boolean when the value is never used? Done Line 229: } else { > "The given value for the search mode ("+optionVal+") is not valid. Valid mo Done https://asterix-gerrit.ics.uci.edu/#/c/1228/11/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/common/FullTextContainsEvaluator.java File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/common/FullTextContainsEvaluator.java: Line 177: */ > This method is really, really long, and quite nested. If it could be broken Done Line 178: private boolean fullTextContainsWithArg(ATypeTag typeTag2, ATypeTag[] typeTagOptions, IPointable arg1, > MAJOR SonarQube violation: Done Line 178: private boolean fullTextContainsWithArg(ATypeTag typeTag2, ATypeTag[] typeTagOptions, IPointable arg1, > MAJOR SonarQube violation: Done Line 313: // array that contains the key (this array will be set later.) > What is the meaning behind 101 and 32768? Can those be static final variabl 101 = # of hash set slot 32768 = frame size in the hash set: 32K. OK. Set these as static final variables. Line 394: protected boolean checkArgTypes(ATypeTag typeTag1, ATypeTag typeTag2) throws AlgebricksException { > The if/else here is superfluous. Done Line 398: > Likewise here. Done -- To view, visit https://asterix-gerrit.ics.uci.edu/1228 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: If00a871a8241d6aa6931f97b694d65f164d3ab8c Gerrit-PatchSet: 11 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Taewoo Kim <wangs...@yahoo.com> Gerrit-Reviewer: Heri Ramampiaro <heri...@gmail.com> Gerrit-Reviewer: Ian Maxon <ima...@apache.org> Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Jianfeng Jia <jianfeng....@gmail.com> Gerrit-Reviewer: Michael Blow <mb...@apache.org> Gerrit-Reviewer: Taewoo Kim <wangs...@yahoo.com> Gerrit-Reviewer: Till Westmann <ti...@apache.org> Gerrit-Reviewer: Yingyi Bu <buyin...@gmail.com> Gerrit-HasComments: Yes
