Steven Jacobs has posted comments on this change.

Change subject: Compile a plan that matches user query in the case of nonpure 
functions
......................................................................


Patch Set 3:

(3 comments)

I addressed a few comments. I'll take a stab at the Sonar stuff and add a new 
patch set.

https://asterix-gerrit.ics.uci.edu/#/c/1057/3/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/util/OperatorManipulationUtil.java
File 
hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/util/OperatorManipulationUtil.java:

Line 99:             if (assignStatus == -1) {
> Can assignStatus have more values than -1 or 1?
It can also have value '0'. See comments on checkForNonpureCalls to see the 
description for each


Line 102:             if (assignStatus == 1) {
> if else?
I can make this an "else if" for clarity


https://asterix-gerrit.ics.uci.edu/#/c/1057/3/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/SetExecutionModeRule.java
File 
hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/SetExecutionModeRule.java:

Line 50:             // case DISTINCT:
> Can these just be removed?
I'm not sure why they were left. If you agree with me than I will go ahead and 
delete them


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2dec322b30835625430c06acd7626d902bada137
Gerrit-PatchSet: 3
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Steven Jacobs <sjaco...@ucr.edu>
Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mb...@apache.org>
Gerrit-Reviewer: Preston Carman <prest...@apache.org>
Gerrit-Reviewer: Steven Jacobs <sjaco...@ucr.edu>
Gerrit-Reviewer: Yingyi Bu <buyin...@gmail.com>
Gerrit-HasComments: Yes

Reply via email to