Taewoo Kim 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)

@Dmitry: thanks again for your comments. I have addressed your comments and 
uploaded another patch set. Could you check?

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
Done


Line 245:                 String option = 
ConstantExpressionUtil.getStringArgument(openRecConsExpr, i).toLowerCase();
> you already have optionExpr at this point. Why fetch it again from argument
Done


Line 257:                             optionTypeStringVal = 
ConstantExpressionUtil.getStringArgument(openRecConsExpr, i + 1)
> 1) Use ConstantExpressionUtil.getStringContstant(optionExprVal) here.
Done


Line 277:                         checkSearchModeOption(optionTypeStringVal, 
functionName);
> This would fail with NPE (as reported by SonarQube) if optionExprVal.getExp
Done


-- 
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 <wangs...@gmail.com>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Dmitry Lychagin <dmitry.lycha...@couchbase.com>
Gerrit-Reviewer: Ian2 Maxon <ima...@uci.edu>
Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Taewoo Kim <wangs...@gmail.com>
Gerrit-HasComments: Yes

Reply via email to