Xikui Wang has posted comments on this change. Change subject: Move channels to a result sharing framework ......................................................................
Patch Set 2: (6 comments) added some more comments. Congratulations to the new dad! :) 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 this one: org.apache.hyracks.storage.am.common.datagen.DocumentStringFieldValueGenerator Please do the same for the others. 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 so, replace these with Collections.singletonList(0) and put it into the parameter lists directly. PS2, Line 207: new ArrayList<>(); : channelSubKey.add(BADConstants.ResultId); : partitionFields.add(ch Replaceable with singletonList? 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 it at the top. 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: /** * * @param partitioningExpr * @param keySourceIndicators * @param autogenerated * @param filterField */ (You cant get this nice tempalte by type /** infront of a method and hit new line in Intellij automatically.) 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 recursive call branch at the top would be easier for others to understand the logic. -- 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 <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Steven Jacobs <[email protected]> Gerrit-Reviewer: Xikui Wang <[email protected]> Gerrit-HasComments: Yes
