Till Westmann has posted comments on this change. ( https://asterix-gerrit.ics.uci.edu/3360 )
Change subject: [WIP] Introduce data generator data source functions ...................................................................... Patch Set 4: (17 comments) Looks good so far - just a few small comments. https://asterix-gerrit.ics.uci.edu/#/c/3360/3/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSReader.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSReader.java: https://asterix-gerrit.ics.uci.edu/#/c/3360/3/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSReader.java@35 PS3, Line 35: should will https://asterix-gerrit.ics.uci.edu/#/c/3360/3/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSReader.java@71 PS3, Line 71: record.reset I think (hope) that we don't need to reset a new CharArrayRecord. https://asterix-gerrit.ics.uci.edu/#/c/3360/3/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSReader.java@103 PS3, Line 103: .filter(table -> tableName.toLowerCase().equals(table.getName().toLowerCase())) > MAJOR SonarQube violation: +1 https://asterix-gerrit.ics.uci.edu/#/c/3360/3/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSReader.java@137 PS3, Line 137: stringRecord.substring(0, stringRecord.length() - 1) We should avoid appending the comma instead of copying the whole record here. https://asterix-gerrit.ics.uci.edu/#/c/3360/3/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSReader.java@141 PS3, Line 141: private FunctionIdentifier getIdentifier() { > MAJOR SonarQube violation: +1 https://asterix-gerrit.ics.uci.edu/#/c/3360/3/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSRewriter.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSRewriter.java: https://asterix-gerrit.ics.uci.edu/#/c/3360/3/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSRewriter.java@38 PS3, Line 38: random data according to the specification of the TPC Benchmark DS https://asterix-gerrit.ics.uci.edu/#/c/3360/3/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSRewriter.java@38 PS3, Line 38: This depends on the solution provided by Teradata which is a : * conversion of the TPC-DS data generator from C to Java, which can be found here: : * https://github.com/Teradata/tpcds remove https://asterix-gerrit.ics.uci.edu/#/c/3360/3/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSRewriter.java@42 PS3, Line 42: generated data data generator https://asterix-gerrit.ics.uci.edu/#/c/3360/3/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSRewriter.java@42 PS3, Line 42: depends on the provided takes https://asterix-gerrit.ics.uci.edu/#/c/3360/3/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSRewriter.java@43 PS3, Line 43: string representing the table name to use as a schema a valid table name https://asterix-gerrit.ics.uci.edu/#/c/3360/3/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSRewriter.java@44 PS3, Line 44: number representing the scaling factor to determine the number of rows to generator the desired scaling factor https://asterix-gerrit.ics.uci.edu/#/c/3360/3/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSRewriter.java@62 PS3, Line 62: ((AsterixConstantValue) tableNameArgument.getValue()) extract variable tableNameArgumentValue https://asterix-gerrit.ics.uci.edu/#/c/3360/3/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSRewriter.java@62 PS3, Line 62: type1 tableNameType? https://asterix-gerrit.ics.uci.edu/#/c/3360/3/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSRewriter.java@63 PS3, Line 63: ((AsterixConstantValue) scalingFactorArgument.getValue()) extract variable scalingFactorArgumentValue https://asterix-gerrit.ics.uci.edu/#/c/3360/3/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSRewriter.java@63 PS3, Line 63: type2 scalingFactorType? https://asterix-gerrit.ics.uci.edu/#/c/3360/3/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSRewriter.java@75 PS3, Line 75: TODO +1 https://asterix-gerrit.ics.uci.edu/#/c/3360/3/asterixdb/asterix-app/src/main/java/org/apache/asterix/util/MetadataBuiltinFunctions.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/util/MetadataBuiltinFunctions.java: https://asterix-gerrit.ics.uci.edu/#/c/3360/3/asterixdb/asterix-app/src/main/java/org/apache/asterix/util/MetadataBuiltinFunctions.java@62 PS3, Line 62: // TODO probably need to move the TPCDS function somewhere else since it's not metadata this is metadata for builtin functions - it belongs here -- To view, visit https://asterix-gerrit.ics.uci.edu/3360 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idb6bd6f697628395c70008e6f730bc5ca403da5e Gerrit-Change-Number: 3360 Gerrit-PatchSet: 4 Gerrit-Owner: Hussain Towaileb <hussai...@gmail.com> Gerrit-Reviewer: Anon. E. Moose (1000171) Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Till Westmann <ti...@apache.org> Gerrit-Comment-Date: Sat, 27 Apr 2019 15:21:14 +0000 Gerrit-HasComments: Yes
