[Impala-ASF-CR] IMPALA-7640: RENAME on managed Kudu table should rename Kudu table
Impala Public Jenkins 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 5: Verified+1 -- 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: 5 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Fri, 05 Apr 2019 21:57:15 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7640: RENAME on managed Kudu table should rename Kudu table
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12179 ) Change subject: IMPALA-7640: RENAME on managed Kudu table should rename Kudu table .. IMPALA-7640: RENAME on managed Kudu table should rename Kudu table `ALTER TABLE managed_kudu_tbl RENAME TO new_managed_kudu_tbl` will now rename the underlying Kudu table from `managed_kudu_tbl` to `new_managed_kudu_tbl`. This is only triggered for managed tables, for external tables renames do not modify the underlying Kudu table. Testing: * Ran core tests * Updated kudu_alter.test Change-Id: I77e7583ce93cba8f6e743c4bedd9900ae1fae081 Reviewed-on: http://gerrit.cloudera.org:8080/12179 Reviewed-by: Thomas Marshall Tested-by: Impala Public Jenkins --- M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test 2 files changed, 52 insertions(+), 1 deletion(-) Approvals: Thomas Marshall: Looks good to me, approved Impala Public Jenkins: Verified -- 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: merged Gerrit-Change-Id: I77e7583ce93cba8f6e743c4bedd9900ae1fae081 Gerrit-Change-Number: 12179 Gerrit-PatchSet: 6 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-7640: RENAME on managed Kudu table should rename Kudu table
Impala Public Jenkins 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 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3988/ DRY_RUN=false -- 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: 5 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Fri, 05 Apr 2019 16:51:20 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7640: RENAME on managed Kudu table should rename Kudu table
Thomas Marshall 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 5: Code-Review+2 -- 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: 5 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Fri, 05 Apr 2019 16:47:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7640: RENAME on managed Kudu table should rename Kudu table
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 5: Filed IMPALA-8391 for the docs updated. Rebased 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: 5 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Fri, 05 Apr 2019 15:45:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7640: RENAME on managed Kudu table should rename Kudu table
Thomas Marshall 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 4: Code-Review+2 (1 comment) > (1 comment) > > > (1 comment) > > > > Please be sure to get the docs updated to reflect this change, > eg: > > http://impala.apache.org/docs/build/html/topics/impala_kudu.html > > http://impala.apache.org/docs/build/html/topics/impala_tables.html > > Will do, I can just file a doc JIRA for this right? Yep http://gerrit.cloudera.org:8080/#/c/12179/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/12179/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2378 PS4, Line 2378: oldTbl instanceof KuduTable && !Table.isExternalTable(msTbl) > The whole logic seems a little confusing to me, but from what I can tell al Oh yeah, I see now that altersKuduTable() doesn't even get called on the RENAME_TABLE path as we exit out of alterTable() early after calling alterTableOrViewRename(). This is all a crazy mess, but fortunately it should be getting a lot better very soon when Impala starts taking advantage of the new Kudu/HMS integration, so I'm fine with this the way it is. -- 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: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Thu, 04 Apr 2019 20:31:25 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7640: RENAME on managed Kudu table should rename Kudu table
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 4: (1 comment) > (1 comment) > > Please be sure to get the docs updated to reflect this change, eg: > http://impala.apache.org/docs/build/html/topics/impala_kudu.html > http://impala.apache.org/docs/build/html/topics/impala_tables.html Will do, I can just file a doc JIRA for this right? http://gerrit.cloudera.org:8080/#/c/12179/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/12179/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2378 PS4, Line 2378: oldTbl instanceof KuduTable && !Table.isExternalTable(msTbl) > Is it possible to do this check as part of altersKuduTable() (eg. by adding The whole logic seems a little confusing to me, but from what I can tell altersKuduTable and alterKuduTable is only meant for ADD_COLUMNS, REPLACE_COLUMNS, DROP_COLUMN, ALTER_COLUMN, and ADD_DROP_RANGE_PARTITION. Whereas alterTableOrViewRename is specifically meant to handle RENAME_VIEW and RENAME_TABLE. So I think it would only make sense to move this to altersKuduTable / alterKuduTable if we move this whole method as well. The reason I put this change here is because the alterTableOrViewRename method seems to handle all the logic for RENAME_TABLE and RENAME_VIEW, so I want to keep all the handling for these TAlterTableTypes in one place. It also looks like alterTableOrViewRename is called while the catalog writeLock is acquired, whereas alterKuduTable is not. -- 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: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Thu, 04 Apr 2019 20:15:49 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7640: RENAME on managed Kudu table should rename Kudu table
Thomas Marshall 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 4: (1 comment) Please be sure to get the docs updated to reflect this change, eg: http://impala.apache.org/docs/build/html/topics/impala_kudu.html http://impala.apache.org/docs/build/html/topics/impala_tables.html http://gerrit.cloudera.org:8080/#/c/12179/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/12179/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2378 PS4, Line 2378: oldTbl instanceof KuduTable && !Table.isExternalTable(msTbl) Is it possible to do this check as part of altersKuduTable() (eg. by adding 'msTbl' as a parameter) and then call your new renameKuduTable() in alterKuduTable() so that this follows the same code path as other 'alter' operations that need to be reflected both in HMS and in Kudu? -- 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: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Thu, 04 Apr 2019 18:51:40 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7640: RENAME on managed Kudu table should rename Kudu table
Lars Volker 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 4: Thomas, do you have time to look at this change? -- 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: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Thu, 04 Apr 2019 01:07:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7640: RENAME on managed Kudu table should rename Kudu table
Lars Volker 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 4: Code-Review+1 -- 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: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Sat, 02 Feb 2019 07:21:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7640: RENAME on managed Kudu table should rename Kudu table
Impala Public Jenkins 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 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1888/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Fri, 25 Jan 2019 18:21:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7640: RENAME on managed Kudu table should rename Kudu table
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
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Sahil Takiar
Gerrit-Comment-Date: Fri, 25 Jan 2019 17:44:41 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7640: RENAME on managed Kudu table should rename Kudu table
Hello Lars Volker, Mike Percy, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12179 to look at the new patch set (#4). Change subject: IMPALA-7640: RENAME on managed Kudu table should rename Kudu table .. IMPALA-7640: RENAME on managed Kudu table should rename Kudu table `ALTER TABLE managed_kudu_tbl RENAME TO new_managed_kudu_tbl` will now rename the underlying Kudu table from `managed_kudu_tbl` to `new_managed_kudu_tbl`. This is only triggered for managed tables, for external tables renames do not modify the underlying Kudu table. Testing: * Ran core tests * Updated kudu_alter.test Change-Id: I77e7583ce93cba8f6e743c4bedd9900ae1fae081 --- M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test 2 files changed, 52 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/79/12179/4 -- 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: newpatchset Gerrit-Change-Id: I77e7583ce93cba8f6e743c4bedd9900ae1fae081 Gerrit-Change-Number: 12179 Gerrit-PatchSet: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-7640: RENAME on managed Kudu table should rename Kudu table
Impala Public Jenkins 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 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1887/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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: 3 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Fri, 25 Jan 2019 15:48:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7640: RENAME on managed Kudu table should rename Kudu table
Hello Lars Volker, Mike Percy, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12179 to look at the new patch set (#3). Change subject: IMPALA-7640: RENAME on managed Kudu table should rename Kudu table .. IMPALA-7640: RENAME on managed Kudu table should rename Kudu table `ALTER TABLE managed_kudu_tbl RENAME TO new_managed_kudu_tbl` will now rename the underlying Kudu table from `managed_kudu_tbl` to `new_managed_kudu_tbl`. This is only triggered for managed tables, for external tables renames do not modify the underlying Kudu table. Testing: * Ran core tests * Updated kudu_alter.test Change-Id: I77e7583ce93cba8f6e743c4bedd9900ae1fae081 --- M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test 2 files changed, 55 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/79/12179/3 -- 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: newpatchset Gerrit-Change-Id: I77e7583ce93cba8f6e743c4bedd9900ae1fae081 Gerrit-Change-Number: 12179 Gerrit-PatchSet: 3 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-7640: RENAME on managed Kudu table should rename Kudu table
Lars Volker 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
indentation lower.
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
particular reason?
--
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
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Sahil Takiar
Gerrit-Comment-Date: Fri, 25 Jan 2019 01:03:34 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7640: RENAME on managed Kudu table should rename Kudu table
Impala Public Jenkins 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: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1870/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 23 Jan 2019 23:24:18 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7640: RENAME on managed Kudu table should rename Kudu table
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 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/12179/1/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/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2378
PS1, Line 2378: if (oldTbl instanceof KuduTable &&
!Table.isExternalTable(msTbl)) {
> Can you add a precondition check for (KuduTable.isKuduTable(msTbl))?
Done
http://gerrit.cloudera.org:8080/#/c/12179/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2586
PS1, Line 2586: if (KuduTable.isKuduTable(msTbl)) {
> Can this code now use renameKuduTable()?
I considered that, but doing so would essentially make renameKuduTable() a
wrapper around KuduCatalogOpExecutor.renameTable()
The way KuduCatalogOpExecutor.renameTable is executed here vs. in
renameKuduTable() is different enough that making them use a common method
doesn't help much. The differences are that (1) we use
properties.get(KuduTable.KEY_TABLE_NAME) to get the table name vs
KuduUtil.getDefaultCreateKuduTableName, (2) all the altered properties have to
be added to the msTbl before KuduCatalogOpExecutor.validateKuduTblExists is
invoked - it seems that the call to validateKuduTblExists validates the table
is accessible *and* that the new properties are valid.
http://gerrit.cloudera.org:8080/#/c/12179/1/testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test
File testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test:
http://gerrit.cloudera.org:8080/#/c/12179/1/testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test@370
PS1, Line 370: alter table tbl_to_alter rename to kudu_tbl_to_alter
> Can you add a test that makes sure that the underlying table got renamed su
This query + the "create external table external_tbl stored as kudu..." one
below already cover this. Together they validate the the table was renamed. I
added another test to make sure that the original table does not exist.
--
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: 1
Gerrit-Owner: Sahil Takiar
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Sahil Takiar
Gerrit-Comment-Date: Wed, 23 Jan 2019 23:07:23 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7640: RENAME on managed Kudu table should rename Kudu table
Hello Lars Volker, Mike Percy, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12179 to look at the new patch set (#2). Change subject: IMPALA-7640: RENAME on managed Kudu table should rename Kudu table .. IMPALA-7640: RENAME on managed Kudu table should rename Kudu table `ALTER TABLE managed_kudu_tbl RENAME TO new_managed_kudu_tbl` will now rename the underlying Kudu table from `managed_kudu_tbl` to `new_managed_kudu_tbl`. This is only triggered for managed tables, for external tables renames do not modify the underlying Kudu table. Testing: * Ran core tests * Updated kudu_alter.test Change-Id: I77e7583ce93cba8f6e743c4bedd9900ae1fae081 --- M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test 2 files changed, 53 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/79/12179/2 -- 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: newpatchset Gerrit-Change-Id: I77e7583ce93cba8f6e743c4bedd9900ae1fae081 Gerrit-Change-Number: 12179 Gerrit-PatchSet: 2 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Mike Percy
[Impala-ASF-CR] IMPALA-7640: RENAME on managed Kudu table should rename Kudu table
Lars Volker 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 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/12179/1/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/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2378
PS1, Line 2378: if (oldTbl instanceof KuduTable &&
!Table.isExternalTable(msTbl)) {
Can you add a precondition check for (KuduTable.isKuduTable(msTbl))?
What happens if the old and new table names are the same? I think we'll throw
an error further down below, but will it be an issue here?
http://gerrit.cloudera.org:8080/#/c/12179/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2586
PS1, Line 2586: if (KuduTable.isKuduTable(msTbl)) {
Can this code now use renameKuduTable()?
http://gerrit.cloudera.org:8080/#/c/12179/1/testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test
File testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test:
http://gerrit.cloudera.org:8080/#/c/12179/1/testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test@370
PS1, Line 370: alter table tbl_to_alter rename to kudu_tbl_to_alter
Can you add a test that makes sure that the underlying table got renamed
successfully?
--
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: 1
Gerrit-Owner: Sahil Takiar
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Mike Percy
Gerrit-Comment-Date: Thu, 17 Jan 2019 18:22:50 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7640: RENAME on managed Kudu table should rename Kudu table
Impala Public Jenkins 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 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1737/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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: 1 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 08 Jan 2019 14:49:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7640: RENAME on managed Kudu table should rename Kudu table
Sahil Takiar has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12179 Change subject: IMPALA-7640: RENAME on managed Kudu table should rename Kudu table .. IMPALA-7640: RENAME on managed Kudu table should rename Kudu table `ALTER TABLE managed_kudu_tbl RENAME TO new_managed_kudu_tbl` will now rename the underlying Kudu table from `managed_kudu_tbl` to `new_managed_kudu_tbl`. This is only triggered for managed tables, for external tables renames do not modify the underlying Kudu table. Testing: * Ran core tests * Updated kudu_alter.test Change-Id: I77e7583ce93cba8f6e743c4bedd9900ae1fae081 --- M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test 2 files changed, 42 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/79/12179/1 -- 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: newchange Gerrit-Change-Id: I77e7583ce93cba8f6e743c4bedd9900ae1fae081 Gerrit-Change-Number: 12179 Gerrit-PatchSet: 1 Gerrit-Owner: Sahil Takiar
