Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/10116 )
Change subject: IMPALA-6131: Track time of last statistics update in metadata ...................................................................... Patch Set 7: (6 comments) http://gerrit.cloudera.org:8080/#/c/10116/4/fe/src/main/java/org/apache/impala/catalog/KuduTable.java File fe/src/main/java/org/apache/impala/catalog/KuduTable.java: http://gerrit.cloudera.org:8080/#/c/10116/4/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@247 PS4, Line 247: msClient.alter_table(msTable_.getDbName(), msTable_.getTableName(), msTable_); > nit: single line? Done http://gerrit.cloudera.org:8080/#/c/10116/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java: http://gerrit.cloudera.org:8080/#/c/10116/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@782 PS4, Line 782: updateTableStats(params, msTbl); : // Set impala.lastComputeStatsTime just before alter_table to ensure that it is as : // accurate as possible. > This could be one check now Done http://gerrit.cloudera.org:8080/#/c/10116/4/tests/metadata/test_last_ddl_time_update.py File tests/metadata/test_last_ddl_time_update.py: http://gerrit.cloudera.org:8080/#/c/10116/4/tests/metadata/test_last_ddl_time_update.py@24 PS4, Line 24: # Checks different statements' effect on last DDL time and last compute stats time. > nit: Checks that... Done http://gerrit.cloudera.org:8080/#/c/10116/4/tests/metadata/test_last_ddl_time_update.py@50 PS4, Line 50: self.db_name = db_name > nit: "to fill the.."? Done http://gerrit.cloudera.org:8080/#/c/10116/4/tests/metadata/test_last_ddl_time_update.py@167 PS4, Line 167: > Can you think of ways to make these calls more readable? We could have to c I have chosen creating a helper class - think that it is much more readable without repeating arguments/substitutions, but it is not a typical solution in Impala test code. Feel free to comment if you prefer to do it another way. http://gerrit.cloudera.org:8080/#/c/10116/6/tests/metadata/test_last_ddl_time_update.py File tests/metadata/test_last_ddl_time_update.py: http://gerrit.cloudera.org:8080/#/c/10116/6/tests/metadata/test_last_ddl_time_update.py@162 PS6, Line 162: h.expect_ddl_time_change("alter table %(TBL)s set tblproperties ('numRows'='1')") This is the only change in patch 5+6 that should affect logic. -- To view, visit http://gerrit.cloudera.org:8080/10116 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I59a671ac29d352bd92ce40d5cb6662bb23f146b5 Gerrit-Change-Number: 10116 Gerrit-PatchSet: 7 Gerrit-Owner: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Wed, 02 May 2018 12:40:17 +0000 Gerrit-HasComments: Yes
