Steven Jacobs has posted comments on this change. Change subject: Move channels to a result sharing framework ......................................................................
Patch Set 1: (18 comments) https://asterix-gerrit.ics.uci.edu/#/c/2731/1/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: Line 181: StringBuilder builder = new StringBuilder(); > One side comment. I think these constructed stmts are one per query. if not I think the BAD stuff is okay, I only use "SET as a second statement, which he mentioned will still work" Line 182: builder.append("upsert into " + channelSubscriptionsDataset + "(\n"); > quick Q. This works without use dataverse? The names passed are absolute so no "use" is needed. PS1, Line 183: s > Change this a constant and give it a better name. Done PS1, Line 185: param > also, is this part of certain meta data schema? The ChannelSubscriptions dataset automatically names the fields for the param values as param0,param1,... PS1, Line 185: param > Change it to a constant variable. Done PS1, Line 195: param > same Done PS1, Line 229: param > same Done PS1, Line 249: s > same here Done https://asterix-gerrit.ics.uci.edu/#/c/2731/1/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: PS1, Line 184: partitionFields = new ArrayList<>(); : keyIndicators = new ArrayList<>(); : keyIndicators.add(0); : keyIndicators.add(0); : fieldName = new ArrayList<>(); : List<String> fieldName2 = new ArrayList<>(); : fieldName.add(BADConstants.ChannelSubscriptionId); : fieldName2.add(BADConstants.BrokerSubscriptionId); : partitionFields.add(fieldName); : partitionFields.add(fieldName2); > Do you mind explain this a little bit? also, regular_name+number is very co I'll clean up the names. Essentially the partitioning fields are each a list (for cases of nested keys). We have two fields for the primary key here, the channelSubscriptionId and the brokerSubscriptionId. Since neither is nested each list is of size one. PS1, Line 280: a > I'm ok with constructing query strings. One minor suggestion would be to cr Done https://asterix-gerrit.ics.uci.edu/#/c/2731/1/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 186: //Maybe we need to add a project??? > ? Done Line 245: > empty line Done Line 343: /*This function is used to find specific operators within the plan, either > You should be able to use the function signature to get a better template f Done PS1, Line 352: String param1, String param2 > these two parameters seem not being used except for in recursive call... sa This method went through several rewrites, so it was pretty messy. I cleaned it up now. Take a look at the new version and see what you think. Line 352: private AbstractLogicalOperator findOp(AbstractLogicalOperator op, int searchId, String param1, String param2) { > I think you could use LogicalOperatorTag here directly Done https://asterix-gerrit.ics.uci.edu/#/c/2731/1/asterix-bad/src/test/resources/runtimets/queries/channel/room_occupants/room_occupants.4.query.sqlpp File asterix-bad/src/test/resources/runtimets/queries/channel/room_occupants/room_occupants.4.query.sqlpp: PS1, Line 30: param0 > correlation with the createChannel param0? (probably not...) This would be the first parameter value from the subscribe statement. The ChannelSubscriptions dataset stores the field names as param0,param1,... https://asterix-gerrit.ics.uci.edu/#/c/2731/1/asterix-bad/src/test/resources/runtimets/queries/channel/shared_subscriptions/shared_subscriptions.3.query.sqlpp File asterix-bad/src/test/resources/runtimets/queries/channel/shared_subscriptions/shared_subscriptions.3.query.sqlpp: PS1, Line 21: param0 > same here? same https://asterix-gerrit.ics.uci.edu/#/c/2731/1/asterix-bad/src/test/resources/runtimets/queries/channel/shared_subscriptions/shared_subscriptions.6.pollquery.sqlpp File asterix-bad/src/test/resources/runtimets/queries/channel/shared_subscriptions/shared_subscriptions.6.pollquery.sqlpp: Line 19: // polltimeoutsecs=15 > ws 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: 1 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
