Steven Jacobs has posted comments on this change.

Change subject: Move channels to a result sharing framework
......................................................................


Patch Set 2:

(6 comments)

Addressed Comments

https://asterix-gerrit.ics.uci.edu/#/c/2731/2/asterix-bad/src/main/java/org/apache/asterix/bad/lang/statement/ChannelSubscribeStatement.java
File 
asterix-bad/src/main/java/org/apache/asterix/bad/lang/statement/ChannelSubscribeStatement.java:

PS2, Line 181: ng channelSubVar = "channelSub";
             :         String param = 
> Move these to the top of the class and make it public static final. Like th
Done


https://asterix-gerrit.ics.uci.edu/#/c/2731/2/asterix-bad/src/main/java/org/apache/asterix/bad/lang/statement/CreateChannelStatement.java
File 
asterix-bad/src/main/java/org/apache/asterix/bad/lang/statement/CreateChannelStatement.java:

PS2, Line 185: keyIndicators = new ArrayList<>();
             :         keyIndicators.add(0);
             :         keyIndicators.add(0);
> it seems the key indicator is not modified but only used as a parameter. If
Done


PS2, Line 207:  new ArrayList<>();
             :             channelSubKey.add(BADConstants.ResultId);
             :             partitionFields.add(ch
> Replaceable with singletonList?
Done


PS2, Line 278: nsertedRecordVar = "a";
             :         String channelSubscriptionRecordVar = "sub";
             :         String channelParamPrefix = channelSubscriptionRecordVar 
+ ".param";
             :         String functionResultVar = "result";
             :         String brokerRecordVar = "b";
             :         String brokerSu
> same as the previous one. Change it to private static final String and put 
Done


https://asterix-gerrit.ics.uci.edu/#/c/2731/2/asterix-bad/src/main/java/org/apache/asterix/bad/rules/InsertBrokerNotifierForChannelRule.java
File 
asterix-bad/src/main/java/org/apache/asterix/bad/rules/InsertBrokerNotifierForChannelRule.java:

Line 343:     //Find Specific Operators within the plan
> Please restore the comments and format it like this:
Done


PS2, Line 374: tractLogicalOperator nestedOp = findOp((AbstractLogicalOperator) 
subOp.getValue(), searchTag,
             :                         subscriptionsName, subscriptionType);
             :                 if (nestedOp != null) {
             :                     return nestedOp;
             :                 }
> Minor code-style suggestion. If it's a recursive function, put the recursiv
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbcdf264bcd21caa0d28a9ac392b36577ca60dad
Gerrit-PatchSet: 2
Gerrit-Project: asterixdb-bad
Gerrit-Branch: master
Gerrit-Owner: Steven Jacobs <sjaco...@ucr.edu>
Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Steven Jacobs <sjaco...@ucr.edu>
Gerrit-Reviewer: Xikui Wang <xkk...@gmail.com>
Gerrit-HasComments: Yes

Reply via email to