Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/17081 )
Change subject: IMPALA-10512: ALTER TABLE ADD PARTITION should bump the write id for ACID tables ...................................................................... Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/17081/3/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/17081/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3145 PS3, Line 3145: addHmsPartitionsInTransaction > Do we need similar changes in the alterTableRecoverPartitions path. What ab alterTableRecoverPartitions is not supported for transactional tables in Impala. Also, if partitions added outside of Impala, e.g. by Hive, then it should've bumped the write id of the transactional table. AFAIK we only create partitions dynamically during INSERTs, and INSERT statements already allocate a new write id. Allocating multiple write id for a single operation would be redundant IMO. I think it's not even supported, i.e. you can only allocate one write id per table in a transaction. (I've just tested it and subsequent allocate_table_write_ids() calls return the same write id if we use the same txn id) -- To view, visit http://gerrit.cloudera.org:8080/17081 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iad247008b7c206db00516326c1447bd00a9b34bd Gerrit-Change-Number: 17081 Gerrit-PatchSet: 3 Gerrit-Owner: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Gabor Kaszab <gaborkas...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Vihang Karajgaonkar <vih...@cloudera.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Tue, 16 Mar 2021 09:54:09 +0000 Gerrit-HasComments: Yes