Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/20887 )
Change subject: IMPALA-12636: Reload filemetadata for AlterTable event of type truncate ...................................................................... Patch Set 3: (5 comments) http://gerrit.cloudera.org:8080/#/c/20887/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java: http://gerrit.cloudera.org:8080/#/c/20887/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@2481 PS3, Line 2481: // force reload truncated partition events nit: catalogd reloads the partitions, not the partition events http://gerrit.cloudera.org:8080/#/c/20887/3/tests/custom_cluster/test_events_custom_configs.py File tests/custom_cluster/test_events_custom_configs.py: http://gerrit.cloudera.org:8080/#/c/20887/3/tests/custom_cluster/test_events_custom_configs.py@1094 PS3, Line 1094: self.execute_query("create database if not exists {0}".format(unique_database)) This shouldn't be necessary with unique_database http://gerrit.cloudera.org:8080/#/c/20887/3/tests/custom_cluster/test_events_custom_configs.py@1099 PS3, Line 1099: " ".join Using join here looks strange to me - shouldn't we simply use the format string? e.g. partitioned_str = " partitioned by (year int) " if is_partitioned else '' create_query = "create table `{}`.`{}` (i int) {} {}".format(unique_database, tbl_name, partitioned_str , values) http://gerrit.cloudera.org:8080/#/c/20887/3/tests/custom_cluster/test_events_custom_configs.py@1101 PS3, Line 1101: self.__get_transactional_tblproperties(is_transactional)]) nit: +2 indentation on line breaks http://gerrit.cloudera.org:8080/#/c/20887/3/tests/custom_cluster/test_events_custom_configs.py@1105 PS3, Line 1105: self.run_stmt_in_hive(insert_query.format(unique_database, tbl_name)) as the test's goal is to check handling of truncate event, shouldn't we wait here for the event processor to process the insert event? there are 2 cases we should avoid to ensure that the test provides proper coverage: 1. the table is reloaded, but not because of the truncate event, but the insert event - as the reload happens after the truncate already happened, catalogd would see an empty table during reload 2. both events are ignored because the table is an IncompleteTable in the catalogd, and the table load is initiated because of the select query, not the event processor The problem with 1 and 2 is that the test would pass even without the truncate event. So actually I think that the best way is to call a REFRESH in Impala before the truncate to ensure that the state with 3 rows is cached and the truncate event handling is really needed to trigger the reload -- To view, visit http://gerrit.cloudera.org:8080/20887 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I53bb80c294623eec7c79d9f30f410771386c6b75 Gerrit-Change-Number: 20887 Gerrit-PatchSet: 3 Gerrit-Owner: Sai Hemanth Gantasala <saihema...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Sai Hemanth Gantasala <saihema...@cloudera.com> Gerrit-Comment-Date: Mon, 15 Jan 2024 12:31:05 +0000 Gerrit-HasComments: Yes
