Dmitry Lychagin has posted comments on this change. Change subject: [ASTERIXDB-2153][COMP][RT] Ensure the fulltext search option is properly handled ......................................................................
Patch Set 7: (4 comments) https://asterix-gerrit.ics.uci.edu/#/c/2116/7/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 77: for (Mutable<ILogicalOperator> inputOpRef : op.getInputs()) { why go recursively through inputs here? The rule would be executed on every operator in the plan by the rule controller, so it should only look at expressions of the current operator. Line 245: String option = ConstantExpressionUtil.getStringArgument(openRecConsExpr, i).toLowerCase(); you already have optionExpr at this point. Why fetch it again from arguments. You could use ConstantExpressionUtil.getStringConstant(optionExpr) instead of getStringArgument(). btw, you could combine it with the previous line (240-243). If it returns null (i.e. was not a STRING) then throw TYPE_UNSUPPORTED exception. Line 257: optionTypeStringVal = ConstantExpressionUtil.getStringArgument(openRecConsExpr, i + 1) 1) Use ConstantExpressionUtil.getStringContstant(optionExprVal) here. 2) Check whether the returned value is null before calling toLowerCase() on it. This would avoid NPE if the actual value is not a string. The null check below at line 259 is too late. Line 277: checkSearchModeOption(optionTypeStringVal, functionName); > BLOCKER SonarQube violation: This would fail with NPE (as reported by SonarQube) if optionExprVal.getExpressionTag() != LogicalExpressionTag.CONSTANT because optionTypeStringVal would be null (and typeError = false) -- To view, visit https://asterix-gerrit.ics.uci.edu/2116 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I240fbe08891d29532c6fcd60638a3b6bbe8da771 Gerrit-PatchSet: 7 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Taewoo Kim <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Dmitry Lychagin <[email protected]> Gerrit-Reviewer: Ian2 Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Taewoo Kim <[email protected]> Gerrit-HasComments: Yes
