Dmitry Lychagin has posted comments on this change. ( https://asterix-gerrit.ics.uci.edu/3437 )
Change subject: [ASTERIXDB-2593][FUN] TPC-DS always parallelize + gen all tables ...................................................................... Patch Set 7: (3 comments) https://asterix-gerrit.ics.uci.edu/#/c/3437/7/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSAllTablesDataGeneratorDatasource.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSAllTablesDataGeneratorDatasource.java: https://asterix-gerrit.ics.uci.edu/#/c/3437/7/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSAllTablesDataGeneratorDatasource.java@37 PS7, Line 37: private final String tableName; tableName is always 'null' here? why keep this field? https://asterix-gerrit.ics.uci.edu/#/c/3437/7/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSDataGeneratorReader.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSDataGeneratorReader.java: https://asterix-gerrit.ics.uci.edu/#/c/3437/7/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSDataGeneratorReader.java@192 PS7, Line 192: selectedTables = getTableFromStringTableName(tableName); I'd move these 3 lines (192-195) to the constructor. This method assigns these 3 fields and returns a list of iterators which is also assigned to a field in the constructor. Let's either move all field assignments to the constructor or make this method void and rename to initTableGeneratorIterators(), so it assigns all 4 fields (current 3 + list of iterators) https://asterix-gerrit.ics.uci.edu/#/c/3437/7/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/function/TPCDSDataGeneratorReader.java@217 PS7, Line 217: generateAllTables = true; can we assign this field ('gemerateAllTables') in the constructor? (so this method doesn't have side-effects) -- To view, visit https://asterix-gerrit.ics.uci.edu/3437 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iff199b0c533d22bcae1caf5057788b257ba4e486 Gerrit-Change-Number: 3437 Gerrit-PatchSet: 7 Gerrit-Owner: Hussain Towaileb <[email protected]> Gerrit-Reviewer: Anon. E. Moose (1000171) Gerrit-Reviewer: Dmitry Lychagin <[email protected]> Gerrit-Reviewer: Hussain Towaileb <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Comment-Date: Wed, 19 Jun 2019 23:06:54 +0000 Gerrit-HasComments: Yes
