Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/12179 )
Change subject: IMPALA-7640: RENAME on managed Kudu table should rename Kudu table ...................................................................... Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/12179/2/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/12179/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2419 PS2, Line 2419: if (!oldTbl.getKuduTableName().equals(newKuduTableName)) { > nit: we often structure these as "if (condition) return;" to keep the inden Done http://gerrit.cloudera.org:8080/#/c/12179/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2423 PS2, Line 2423: oldMsTbl.getParameters().put(KuduTable.KEY_TABLE_NAME, newKuduTableName); > I noticed that the order changed but couldn't figure out why. Is there a pa You want to update the msTbl with the value of the new Kudu table, before passing the msTbl to validateKuduTblExists. validateKuduTblExists will take the name of the Kudu table from the msTbl using the key KuduTable.KEY_TABLE_NAME and use it to check if that Kudu table exists. However, I decided to just remove the call to validateKuduTblExists, I don't think its adding any value here because kuduClient.renameTable should guarantee the new table exists. So doing another check just adds an unnecessary RPC call to Kudu. The original run of the tests passed without this change because of IMPALA-8117. The fix for IMPALA-8117 requires some invasive changes to add proper unit testing, so I'm planning on doing it in a separate patch. -- To view, visit http://gerrit.cloudera.org:8080/12179 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I77e7583ce93cba8f6e743c4bedd9900ae1fae081 Gerrit-Change-Number: 12179 Gerrit-PatchSet: 2 Gerrit-Owner: Sahil Takiar <stak...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Sahil Takiar <stak...@cloudera.com> Gerrit-Comment-Date: Fri, 25 Jan 2019 17:44:41 +0000 Gerrit-HasComments: Yes
