asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Taewoo Kim (Code Review)" <>
Subject Change in asterixdb[master]: AQLPlus Refactoring 1: add a new rule controller
Date Wed, 18 Jan 2017 05:18:39 GMT
Taewoo Kim has posted comments on this change.

Change subject: AQLPlus Refactoring 1: add a new rule controller

Patch Set 5:

File hyracks-fullstack/algebricks/algebricks-common/src/main/java/org/apache/hyracks/algebricks/common/exceptions/

Line 45:     public static AlgebricksException create(int code, Serializable... params) {
> code -> errorCode
File hyracks-fullstack/algebricks/algebricks-compiler/src/main/java/org/apache/hyracks/algebricks/compiler/rewriter/rulecontrollers/

Line 56:         boolean anyRuleFired;
> move the declaration to line 69, combing that with the rest of the line.

Line 57:         boolean anyChange;
> move the declaration to line 79.

Line 68:         // Checks whether the first rule returns true.
> Make sure that empty input collection can still work gracefully?

Line 70:         if (!anyRuleFired) {
> Adding a "if" check after line 83 should be able to get rid of line 68 to l

Line 75:         for (int i = 1; i < rules.size(); i++) {
> Why do you need that for-loop iteration?
I have added this not to execute the first rule twice in the first iteration. But, I think
addressing your above comment will solve this, too.
File hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/exceptions/

Line 29:  */
> describe the error code boundary here.

Line 38:     public static final int RUNTIME_FULLTEXT_PHRASE_FOUND = 5;
> There is no corresponding message in the properties file.

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I2742e891339e5aba37a00f77d7f18cb3c09bcfe2
Gerrit-PatchSet: 5
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Taewoo Kim <>
Gerrit-Reviewer: Jenkins <>
Gerrit-Reviewer: Taewoo Kim <>
Gerrit-Reviewer: Yingyi Bu <>
Gerrit-HasComments: Yes

View raw message