[Impala-ASF-CR] IMPALA-10519: Allow setting of num reactors for KuduClient
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/17086 ) Change subject: IMPALA-10519: Allow setting of num_reactors for KuduClient .. Patch Set 2: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/17086/2/be/src/exec/kudu-util.cc File be/src/exec/kudu-util.cc: http://gerrit.cloudera.org:8080/#/c/17086/2/be/src/exec/kudu-util.cc@48 PS2, Line 48: 16 nit: maybe use the default 4 to be consistent with current setting, if we are not sure what a good default will be (as it may vary depends on the workload.) -- To view, visit http://gerrit.cloudera.org:8080/17086 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If2ccb9659b9223c9a5de2416b946e6313a3239ff Gerrit-Change-Number: 17086 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 19 Feb 2021 00:21:55 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9092: Add support for creating external Kudu table
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/14750 ) Change subject: IMPALA-9092: Add support for creating external Kudu table .. Patch Set 5: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/14750 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I76f81d41db0cf2269ee1b365857164a43677e14d Gerrit-Change-Number: 14750 Gerrit-PatchSet: 5 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Tue, 26 Nov 2019 22:42:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9092: Add support for creating external Kudu table
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/14750 )
Change subject: IMPALA-9092: Add support for creating external Kudu table
..
Patch Set 4:
(4 comments)
Overall looks good to me. Thanks Vihang for fixing this!
http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java:
http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java@383
PS4, Line 383: if (Boolean.parseBoolean(
:
getTblProperties().get(KuduTable.TBL_PROP_EXTERNAL_TABLE_PURGE))) {
: throw new AnalysisException(String.format("Table property
'%s' cannot be set to " +
: "true with an external Kudu table.",
KuduTable.TBL_PROP_EXTERNAL_TABLE_PURGE));
: }
This can be removed as 'external.table.purge' = true has been checked at L290?
http://gerrit.cloudera.org:8080/#/c/14750/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/14750/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2211
PS4, Line 2211: except we skip Kudu table creation if
:* it already exists
From the code in KuduCatalogOpExecutor, it looks like we skip Kudu table
creation (throw error) for both managed and external purge table?
http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java@92
PS4, Line 92: if table is externalPurgeTable
nit: if table is managed or externalPurgeTable?
http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/main/java/org/apache/impala/service/MetadataOp.java
File fe/src/main/java/org/apache/impala/service/MetadataOp.java:
http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/main/java/org/apache/impala/service/MetadataOp.java@21
PS4, Line 21: import java.util.Collections;
nit: why is the added while no other change in this file?
--
To view, visit http://gerrit.cloudera.org:8080/14750
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I76f81d41db0cf2269ee1b365857164a43677e14d
Gerrit-Change-Number: 14750
Gerrit-PatchSet: 4
Gerrit-Owner: Vihang Karajgaonkar
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Comment-Date: Thu, 21 Nov 2019 22:32:14 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9030: Handle translated external Kudu tables
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/14397 ) Change subject: IMPALA-9030: Handle translated external Kudu tables .. Patch Set 10: > Patch Set 10: Verified-1 > > Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/5123/ The failure test TestEventProcessing.test_insert_events seems to be unrelated flaky test. -- To view, visit http://gerrit.cloudera.org:8080/14397 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I324523361c923b7d291cb4d0f1028b1a5b653b36 Gerrit-Change-Number: 14397 Gerrit-PatchSet: 10 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Mon, 21 Oct 2019 23:08:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9030: Handle translated external Kudu tables
Hao Hao has uploaded a new patch set (#10) to the change originally created by Vihang Karajgaonkar. ( http://gerrit.cloudera.org:8080/14397 ) Change subject: IMPALA-9030: Handle translated external Kudu tables .. IMPALA-9030: Handle translated external Kudu tables In HMS 3.1 there is a default tranformer introduced which checks the client capabilities and transforms a table before creating it. Additionally, it also makes sure that any managed table which is created is transactional. If a user creates a managed table which is not transactional, it automatically converts such table as external and sets certain table properties to mark such transformed tables. This presents a problem for managed Kudu tables in Impala since managed and external tables are handled differently in Kudu. Specifically, if a Kudu table is managed, certain operations like drop table, rename table, alter table are performed on the Kudu side along with updating the catalog. If the Kudu table is external, the Kudu operations are skipped and only catalog side operations are performed. When the user creates a managed Kudu table, user expects that drop table, rename table should be updated by Impala automatically in Kudu as well. But since HMS 3 transforms such managed tables into external, currently Impala does not perform the Kudu side operations breaking the semantics for the user. This patch makes changes to Catalog so that it can detect such transformed external tables and perform Kudu side operations similar to what it was doing for managed Kudu table when talking with previous HMS versions. Note that this change is in preparation of bumping up the CDP build which will be done in a separate change. For the current CDP build number the patch is essentially a no-op. Testing: 1. Bumped up the CDP build number in a private build so that the HMS translation logic is pulled in. Ran all the tests. Without the patch there are many Kudu tests which were failing. After the patch none of the Kudu tests fail. There were additional Ranger tests which failed due to the CDP bump but those were unrelated to this patch and should be fixed as part of a separate change when the CDP build number is bumped up. Change-Id: I324523361c923b7d291cb4d0f1028b1a5b653b36 --- M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java M fe/src/test/java/org/apache/impala/common/FrontendFixture.java 8 files changed, 105 insertions(+), 40 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/97/14397/10 -- To view, visit http://gerrit.cloudera.org:8080/14397 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I324523361c923b7d291cb4d0f1028b1a5b653b36 Gerrit-Change-Number: 14397 Gerrit-PatchSet: 10 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-9030: Handle translated external Kudu tables
Hao Hao has uploaded a new patch set (#9) to the change originally created by Vihang Karajgaonkar. ( http://gerrit.cloudera.org:8080/14397 ) Change subject: IMPALA-9030: Handle translated external Kudu tables .. IMPALA-9030: Handle translated external Kudu tables In HMS 3.1 there is a default tranformer introduced which checks the client capabilities and transforms a table before creating it. Additionally, it also makes sure that any managed table which is created is transactional. If a user creates a managed table which is not transactional, it automatically converts such table as external and sets certain table properties to mark such transformed tables. This presents a problem for managed Kudu tables in Impala since managed and external tables are handled differently in Kudu. Specifically, if a Kudu table is managed, certain operations like drop table, rename table, alter table are performed on the Kudu side along with updating the catalog. If the Kudu table is external, the Kudu operations are skipped and only catalog side operations are performed. When the user creates a managed Kudu table, user expects that drop table, rename table should be updated by Impala automatically in Kudu as well. But since HMS 3 transforms such managed tables into external, currently Impala does not perform the Kudu side operations breaking the semantics for the user. This patch makes changes to Catalog so that it can detect such transformed external tables and perform Kudu side operations similar to what it was doing for managed Kudu table when talking with previous HMS versions. Note that this change is in preparation of bumping up the CDP build which will be done in a separate change. For the current CDP build number the patch is essentially a no-op. Testing: 1. Bumped up the CDP build number in a private build so that the HMS translation logic is pulled in. Ran all the tests. Without the patch there are many Kudu tests which were failing. After the patch none of the Kudu tests fail. There were additional Ranger tests which failed due to the CDP bump but those were unrelated to this patch and should be fixed as part of a separate change when the CDP build number is bumped up. Change-Id: I324523361c923b7d291cb4d0f1028b1a5b653b36 --- M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java M fe/src/test/java/org/apache/impala/common/FrontendFixture.java 8 files changed, 105 insertions(+), 40 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/97/14397/9 -- To view, visit http://gerrit.cloudera.org:8080/14397 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I324523361c923b7d291cb4d0f1028b1a5b653b36 Gerrit-Change-Number: 14397 Gerrit-PatchSet: 9 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-9030: Handle translated external Kudu tables
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/14397 )
Change subject: IMPALA-9030: Handle translated external Kudu tables
..
Patch Set 8:
(6 comments)
> Patch Set 8:
>
> (1 comment)
>
> > Patch Set 8: Verified-1
> >
> > Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/5114/
>
> Some test failures need to fix:
> https://jenkins.impala.io/job/ubuntu-16.04-from-scratch/8384/
> Test Result (3 failures / +3)
> org.apache.impala.analysis.AnalyzeKuduDDLTest.TestCreateExternalKuduTable
> org.apache.impala.planner.PlannerTest.testHbase
> org.apache.impala.planner.PlannerTest.testHbaseNoKeyEstimate
>
> Hao, you can run these java tests locally according to
> https://cwiki.apache.org/confluence/display/IMPALA/How+to+load%2C+run%2C+and+create+new+Impala+tests
> e.g. in $IMPALA_HOME:
> (pushd fe && mvn test -Dtest=AnalyzeKuduDDLTest#TestCreateExternalKuduTable)
> (pushd fe && mvn test -Dtest=PlannerTest#testHbase)
> (pushd fe && mvn test -Dtest=PlannerTest#testHbaseNoKeyEstimate)
>
> Looks like we broke something in HBase Tables. I'll also look into it
> tomorrow.
Thanks Quanlong. I didn't do it as I don't have a suitable environment to set
up Impala (which can takes hours to do), and I think this change is obvious
enough to rely on the Jenkins output.
http://gerrit.cloudera.org:8080/#/c/14397/8/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java:
http://gerrit.cloudera.org:8080/#/c/14397/8/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java@341
PS8, Line 341: // External table cannot have 'external.table.purge'
property set, which is considered
: // equivalent to managed table.
: if (Boolean.parseBoolean(
:
getTblProperties().get(KuduTable.TBL_PROP_EXTERNAL_TABLE_PURGE))) {
: throw new AnalysisException(String.format("Table property
'%s' cannot be set to " +
: "true with an external Kudu table.",
KuduTable.TBL_PROP_EXTERNAL_TABLE_PURGE));
: }
> I think manually changing 'external.table.purge' should not be allowed. We
Good point! Updated.
http://gerrit.cloudera.org:8080/#/c/14397/8/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/14397/8/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@140
PS8, Line 140: public static boolean isSynchronizedTable(
> Sorry, don't need to move these... Only Kudu tables are affected.
Ack
http://gerrit.cloudera.org:8080/#/c/14397/8/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java:
http://gerrit.cloudera.org:8080/#/c/14397/8/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java@505
PS8, Line 505: // Cannot specify the number of replicas for external Kudu
tables
> Comment looks incorrect here. Maybe: External Kudu table is not allowed to
Done
http://gerrit.cloudera.org:8080/#/c/14397/8/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java@506
PS8, Line 506: tab (x int)
> Should not specify columns for creating external Kudu table.
Done
http://gerrit.cloudera.org:8080/#/c/14397/8/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java@507
PS8, Line 507: purges
> typo: purge
Done
http://gerrit.cloudera.org:8080/#/c/14397/8/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java@508
PS8, Line 508: purges
> same here: purge
Done
--
To view, visit http://gerrit.cloudera.org:8080/14397
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I324523361c923b7d291cb4d0f1028b1a5b653b36
Gerrit-Change-Number: 14397
Gerrit-PatchSet: 8
Gerrit-Owner: Vihang Karajgaonkar
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Greg Solovyev
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Comment-Date: Mon, 21 Oct 2019 18:35:54 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9030: Handle translated external Kudu tables
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/14397 ) Change subject: IMPALA-9030: Handle translated external Kudu tables .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/14397/7/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java: http://gerrit.cloudera.org:8080/#/c/14397/7/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java@343 PS7, Line 343: if (Boolean.parseBoolean( > line too long (96 > 90) Done -- To view, visit http://gerrit.cloudera.org:8080/14397 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I324523361c923b7d291cb4d0f1028b1a5b653b36 Gerrit-Change-Number: 14397 Gerrit-PatchSet: 8 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Fri, 18 Oct 2019 23:18:50 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9030: Handle translated external Kudu tables
Hao Hao has uploaded a new patch set (#8) to the change originally created by Vihang Karajgaonkar. ( http://gerrit.cloudera.org:8080/14397 ) Change subject: IMPALA-9030: Handle translated external Kudu tables .. IMPALA-9030: Handle translated external Kudu tables In HMS 3.1 there is a default tranformer introduced which checks the client capabilities and transforms a table before creating it. Additionally, it also makes sure that any managed table which is created is transactional. If a user creates a managed table which is not transactional, it automatically converts such table as external and sets certain table properties to mark such transformed tables. This presents a problem for managed Kudu tables in Impala since managed and external tables are handled differently in Kudu. Specifically, if a Kudu table is managed, certain operations like drop table, rename table, alter table are performed on the Kudu side along with updating the catalog. If the Kudu table is external, the Kudu operations are skipped and only catalog side operations are performed. When the user creates a managed Kudu table, user expects that drop table, rename table should be updated by Impala automatically in Kudu as well. But since HMS 3 transforms such managed tables into external, currently Impala does not perform the Kudu side operations breaking the semantics for the user. This patch makes changes to Catalog so that it can detect such transformed external tables and perform Kudu side operations similar to what it was doing for managed Kudu table when talking with previous HMS versions. Note that this change is in preparation of bumping up the CDP build which will be done in a separate change. For the current CDP build number the patch is essentially a no-op. Testing: 1. Bumped up the CDP build number in a private build so that the HMS translation logic is pulled in. Ran all the tests. Without the patch there are many Kudu tests which were failing. After the patch none of the Kudu tests fail. There were additional Ranger tests which failed due to the CDP bump but those were unrelated to this patch and should be fixed as part of a separate change when the CDP build number is bumped up. Change-Id: I324523361c923b7d291cb4d0f1028b1a5b653b36 --- M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java M fe/src/test/java/org/apache/impala/common/FrontendFixture.java 8 files changed, 94 insertions(+), 40 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/97/14397/8 -- To view, visit http://gerrit.cloudera.org:8080/14397 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I324523361c923b7d291cb4d0f1028b1a5b653b36 Gerrit-Change-Number: 14397 Gerrit-PatchSet: 8 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-9030: Handle translated external Kudu tables
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/14397 ) Change subject: IMPALA-9030: Handle translated external Kudu tables .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/14397/6/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java: http://gerrit.cloudera.org:8080/#/c/14397/6/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java@75 PS6, Line 75: !KuduTable.isExternalTable(msTbl) > That said, if someone more familiar with Impala metadata feels that option I also thinks option 1 is more safe, and as no one so far says it is safe with option 2. I just pushed a patch with option 1. -- To view, visit http://gerrit.cloudera.org:8080/14397 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I324523361c923b7d291cb4d0f1028b1a5b653b36 Gerrit-Change-Number: 14397 Gerrit-PatchSet: 7 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Fri, 18 Oct 2019 23:14:15 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9030: Handle translated external Kudu tables
Hao Hao has uploaded a new patch set (#7) to the change originally created by Vihang Karajgaonkar. ( http://gerrit.cloudera.org:8080/14397 ) Change subject: IMPALA-9030: Handle translated external Kudu tables .. IMPALA-9030: Handle translated external Kudu tables In HMS 3.1 there is a default tranformer introduced which checks the client capabilities and transforms a table before creating it. Additionally, it also makes sure that any managed table which is created is transactional. If a user creates a managed table which is not transactional, it automatically converts such table as external and sets certain table properties to mark such transformed tables. This presents a problem for managed Kudu tables in Impala since managed and external tables are handled differently in Kudu. Specifically, if a Kudu table is managed, certain operations like drop table, rename table, alter table are performed on the Kudu side along with updating the catalog. If the Kudu table is external, the Kudu operations are skipped and only catalog side operations are performed. When the user creates a managed Kudu table, user expects that drop table, rename table should be updated by Impala automatically in Kudu as well. But since HMS 3 transforms such managed tables into external, currently Impala does not perform the Kudu side operations breaking the semantics for the user. This patch makes changes to Catalog so that it can detect such transformed external tables and perform Kudu side operations similar to what it was doing for managed Kudu table when talking with previous HMS versions. Note that this change is in preparation of bumping up the CDP build which will be done in a separate change. For the current CDP build number the patch is essentially a no-op. Testing: 1. Bumped up the CDP build number in a private build so that the HMS translation logic is pulled in. Ran all the tests. Without the patch there are many Kudu tests which were failing. After the patch none of the Kudu tests fail. There were additional Ranger tests which failed due to the CDP bump but those were unrelated to this patch and should be fixed as part of a separate change when the CDP build number is bumped up. Change-Id: I324523361c923b7d291cb4d0f1028b1a5b653b36 --- M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java M fe/src/test/java/org/apache/impala/common/FrontendFixture.java 8 files changed, 93 insertions(+), 40 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/97/14397/7 -- To view, visit http://gerrit.cloudera.org:8080/14397 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I324523361c923b7d291cb4d0f1028b1a5b653b36 Gerrit-Change-Number: 14397 Gerrit-PatchSet: 7 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-9030: Handle translated external Kudu tables
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/14397 )
Change subject: IMPALA-9030: Handle translated external Kudu tables
..
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14397/6/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/14397/6/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java@75
PS6, Line 75: !KuduTable.isExternalTable(msTbl)
> So this is interesting. Note that this method is implemented to create mana
Hmm, it will be translated to
'Preconditions.checkState(KuduTable.isSynchronizedTable(msTbl))' instead of
'Preconditions.checkState(!KuduTable.isSynchronizedTable(msTbl))', right? So
only the following should be allowed:
create table foo (id int);
create external table foo (id int) tblproperties ('external.purge.table' =
'false');
There are actually big differences between treating 'create table foo(id int)
tblproperties ('external.purge.table'='true');` as external or synchronized:
1) creating external table has to have 'kudu.table_name' property specified.
That means user can only do
create external table foo (id int) tblproperties ('external.purge.table' =
'false', 'kudu.table_name' = 'my_kudu_table'); And Kudu table name will be the
one specified by the user.
2) creating a synchronized table means a Kudu table will be created by Impala,
while creating external table will not.
Therefore, after pondering more, I think we should either block
'external.purge.table'='true' for Kudu tables as it conflicts with the
definition of synchronized table. Then we can keep the current logic of
checking for external table here. Or treat external +
'external.purge.table'='true' as managed table for table naming, and in
analyzing create table statement
(https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java#L255).
And update to check for synchronized table here. I am good with either
approach.
--
To view, visit http://gerrit.cloudera.org:8080/14397
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I324523361c923b7d291cb4d0f1028b1a5b653b36
Gerrit-Change-Number: 14397
Gerrit-PatchSet: 6
Gerrit-Owner: Vihang Karajgaonkar
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Greg Solovyev
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Comment-Date: Fri, 18 Oct 2019 00:20:09 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9030: Handle translated external Kudu tables
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/14397 ) Change subject: IMPALA-9030: Handle translated external Kudu tables .. Patch Set 6: (4 comments) http://gerrit.cloudera.org:8080/#/c/14397/6/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/14397/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2185 PS6, Line 2185: isExternalTable It should be updated to check for not (synchronized) tables. http://gerrit.cloudera.org:8080/#/c/14397/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2192 PS6, Line 2192: isExternalTable Same here. http://gerrit.cloudera.org:8080/#/c/14397/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2210 PS6, Line 2210: isExternalTable Same here. http://gerrit.cloudera.org:8080/#/c/14397/6/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java: http://gerrit.cloudera.org:8080/#/c/14397/6/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java@75 PS6, Line 75: !KuduTable.isExternalTable(msTbl) > Should this be checking for whether the table is synchronized? Are impala u +1. I think checks in CatalogOpExecutor::createKuduTable() also need to be updated. -- To view, visit http://gerrit.cloudera.org:8080/14397 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I324523361c923b7d291cb4d0f1028b1a5b653b36 Gerrit-Change-Number: 14397 Gerrit-PatchSet: 6 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Thu, 17 Oct 2019 21:13:19 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9030: Handle translated external Kudu tables
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/14397 ) Change subject: IMPALA-9030: Handle translated external Kudu tables .. Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/14397/2/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java File fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java: http://gerrit.cloudera.org:8080/#/c/14397/2/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java@117 PS2, Line 117: managed Kudu table Update the comment to reflect the change and else where. http://gerrit.cloudera.org:8080/#/c/14397/2/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java@120 PS2, Line 120: isExternallyManagedTable > logically its doing the same right. It is checking if this table is not ext Yeah, I guess I would prefer the naming 'isSynchronizedTable' as it is more clear and less misleading than 'isExternallyManagedTable'. http://gerrit.cloudera.org:8080/#/c/14397/2/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/14397/2/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@131 PS2, Line 131: HMS nit: missing period. http://gerrit.cloudera.org:8080/#/c/14397/2/fe/src/main/java/org/apache/impala/catalog/Table.java File fe/src/main/java/org/apache/impala/catalog/Table.java: http://gerrit.cloudera.org:8080/#/c/14397/2/fe/src/main/java/org/apache/impala/catalog/Table.java@157 PS2, Line 157: external table nit: external table. -- To view, visit http://gerrit.cloudera.org:8080/14397 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I324523361c923b7d291cb4d0f1028b1a5b653b36 Gerrit-Change-Number: 14397 Gerrit-PatchSet: 2 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Thu, 10 Oct 2019 23:35:11 + Gerrit-HasComments: Yes
[Impala-ASF-CR] Remove redundant table name population in Kudu integration
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/14130 ) Change subject: Remove redundant table name population in Kudu integration .. Patch Set 1: > Patch Set 1: Code-Review+2 > > I assume CDH_BUILD_NUMBER doesn't need to be bumped? No, I don't think so. -- To view, visit http://gerrit.cloudera.org:8080/14130 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaa88ae5f0597ef203b60adcc972d06f8f4a418b7 Gerrit-Change-Number: 14130 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 26 Aug 2019 16:37:32 + Gerrit-HasComments: No
[Impala-ASF-CR] Remove redundant table name population in Kudu integration
Hao Hao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14130 Change subject: Remove redundant table name population in Kudu integration .. Remove redundant table name population in Kudu integration This patch removes the hack to populate table name if empty in Kudu integration. Since with the current Kudu version, the storage handler now supports 'kudu.table_name' property and this table name should not be empty. Change-Id: Iaa88ae5f0597ef203b60adcc972d06f8f4a418b7 --- M fe/src/main/java/org/apache/impala/catalog/KuduTable.java 1 file changed, 0 insertions(+), 25 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/30/14130/1 -- To view, visit http://gerrit.cloudera.org:8080/14130 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Iaa88ae5f0597ef203b60adcc972d06f8f4a418b7 Gerrit-Change-Number: 14130 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao
[Impala-ASF-CR] IMPALA-8856: Deflake TestKuduHMSIntegration
Hao Hao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14067 Change subject: IMPALA-8856: Deflake TestKuduHMSIntegration .. IMPALA-8856: Deflake TestKuduHMSIntegration Tests for Kudu's integration with the Hive Metastore can be flaky. Since Kudu depends on the HMS, create/drop table requests can timeout when creation/deletion in the HMS take more than 10s, and the following retry will fail as the table has been already created/deleted by the first request. This patch increases the timeout of individual Kudu client rpcs to avoid flakiness cause by such cases. Change-Id: Ib98f34bb831b9255e35b5ef234abe6ceaf261bfd --- M tests/custom_cluster/test_kudu.py 1 file changed, 19 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/67/14067/1 -- To view, visit http://gerrit.cloudera.org:8080/14067 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ib98f34bb831b9255e35b5ef234abe6ceaf261bfd Gerrit-Change-Number: 14067 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao
[Impala-ASF-CR] IMPALA-8629: (part 2) Adjust new KuduStorageHandler package
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13541 ) Change subject: IMPALA-8629: (part 2) Adjust new KuduStorageHandler package .. Patch Set 8: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/13541 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4d0c505643247498383472704a37d27c9e1ce473 Gerrit-Change-Number: 13541 Gerrit-PatchSet: 8 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Wed, 12 Jun 2019 17:34:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8629: (part 1) Add temp KuduStorageHandler
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13561 ) Change subject: IMPALA-8629: (part 1) Add temp KuduStorageHandler .. Patch Set 1: Code-Review+1 (1 comment) Just one minor nit. http://gerrit.cloudera.org:8080/#/c/13561/1/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/13561/1/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@90 PS1, Line 90: // TODO(ghenke): Remove this after Kudu adjusts its KuduStorageHandler logic. We may want to add the Jira number (IMPALA-8629) here? -- To view, visit http://gerrit.cloudera.org:8080/13561 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic9982466699818390fa28efc5ea1aae75b11c12a Gerrit-Change-Number: 13561 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Fri, 07 Jun 2019 19:54:18 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8635. Use local metastore URIs when checking for Kudu/HMS integration
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13555 ) Change subject: IMPALA-8635. Use local metastore URIs when checking for Kudu/HMS integration .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/13555 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id7a4c2cc0580f7c4dc5cfceed30b91e87c547612 Gerrit-Change-Number: 13555 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Fri, 07 Jun 2019 17:41:15 + Gerrit-HasComments: No
[Impala-ASF-CR] Fix integration of kudu-hive.jar
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13542 ) Change subject: Fix integration of kudu-hive.jar .. Patch Set 4: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/13542 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4ac79c3e9b8625ba54145dba23c69fd5117f35c7 Gerrit-Change-Number: 13542 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Thu, 06 Jun 2019 21:20:08 + Gerrit-HasComments: No
[Impala-ASF-CR] Improve adding kudu-hive.jar to HADOOP CLASSPATH
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13542 ) Change subject: Improve adding kudu-hive.jar to HADOOP_CLASSPATH .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/13542 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4ac79c3e9b8625ba54145dba23c69fd5117f35c7 Gerrit-Change-Number: 13542 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 06 Jun 2019 18:35:59 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS integration
Hello Thomas Marshall, Alexey Serbin, Andrew Wong, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13409 to look at the new patch set (#5). Change subject: IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS integration .. IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS integration This commit intends to support the actual handling of ALTER/RENAME TABLE DDL for managed Kudu tables with Kudu's integration with the Hive Metastore. However, currently Kudu is considered as the source of truth of the table schema, so when ALTER TABLE (ADD/DROP COLUMN/RANGE_PARTITION), Impala always directly alters/loads the Kudu tables. Thus, this commit only updates RENAME TABLE DDL, so that after the table is renamed in the Kudu, relies on Kudu to rename the table in the HMS. Change-Id: If7155e0b385b8ad81eda0a84277bc85171a88269 --- M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test A testdata/workloads/functional-query/queries/QueryTest/kudu_hms_alter.test M tests/custom_cluster/test_kudu.py M tests/query_test/test_kudu.py 5 files changed, 679 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/13409/5 -- To view, visit http://gerrit.cloudera.org:8080/13409 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If7155e0b385b8ad81eda0a84277bc85171a88269 Gerrit-Change-Number: 13409 Gerrit-PatchSet: 5 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS integration
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13409 ) Change subject: IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS integration .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/13409/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/13409/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2590 PS4, Line 2590: boolean altersHMSTable = true; > I guess this works because if isManagedKuduTable=false and we don't actuall Done -- To view, visit http://gerrit.cloudera.org:8080/13409 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If7155e0b385b8ad81eda0a84277bc85171a88269 Gerrit-Change-Number: 13409 Gerrit-PatchSet: 5 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Thu, 06 Jun 2019 03:11:22 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS integration
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13409 ) Change subject: IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS integration .. Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/13409/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/13409/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2588 PS3, Line 2588: isKuduHmsIntegrationEnabled(msTbl) > 'boolean isKuduHmsIntegrationEnabled' so you can reuse it below? Done http://gerrit.cloudera.org:8080/#/c/13409/3/testdata/workloads/functional-query/queries/QueryTest/kudu_hms_alter.test File testdata/workloads/functional-query/queries/QueryTest/kudu_hms_alter.test: http://gerrit.cloudera.org:8080/#/c/13409/3/testdata/workloads/functional-query/queries/QueryTest/kudu_hms_alter.test@1 PS3, Line 1: # Test cases on Kudu tables that only expected to be ran with HMS integration enabled. > Please note how this file differs from kudu_alter.test and that they will b Done. It was removed because Grant pointed out this is kind of irrelevant test. However to ease the effort of parameterizing the two test, I will add it back. http://gerrit.cloudera.org:8080/#/c/13409/3/testdata/workloads/functional-query/queries/QueryTest/kudu_hms_alter.test@1 PS3, Line 1: # Test cases on Kudu tables that only expected to be ran with HMS integration enabled. > Also, please add an equivalent note at the top of kudu_alter.test and say t Done http://gerrit.cloudera.org:8080/#/c/13409/3/tests/custom_cluster/test_kudu.py File tests/custom_cluster/test_kudu.py: http://gerrit.cloudera.org:8080/#/c/13409/3/tests/custom_cluster/test_kudu.py@117 PS3, Line 117: class TestKuduHMSIntegration(CustomClusterTestSuite, KuduTestSuite): > It occurs to me - because this is a subclass of CustomClusterTestSuite, it Added this to the TODO as well. -- To view, visit http://gerrit.cloudera.org:8080/13409 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If7155e0b385b8ad81eda0a84277bc85171a88269 Gerrit-Change-Number: 13409 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Wed, 05 Jun 2019 19:39:21 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS integration
Hello Thomas Marshall, Alexey Serbin, Andrew Wong, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13409 to look at the new patch set (#4). Change subject: IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS integration .. IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS integration This commit intends to support the actual handling of ALTER/RENAME TABLE DDL for managed Kudu tables with Kudu's integration with the Hive Metastore. However, currently Kudu is considered as the source of truth of the table schema, so when ALTER TABLE (ADD/DROP COLUMN/RANGE_PARTITION), Impala always directly alters/loads the Kudu tables. Thus, this commit only updates RENAME TABLE DDL, so that after the table is renamed in the Kudu, relies on Kudu to rename the table in the HMS. Change-Id: If7155e0b385b8ad81eda0a84277bc85171a88269 --- M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test A testdata/workloads/functional-query/queries/QueryTest/kudu_hms_alter.test M tests/custom_cluster/test_kudu.py M tests/query_test/test_kudu.py 5 files changed, 677 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/13409/4 -- To view, visit http://gerrit.cloudera.org:8080/13409 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If7155e0b385b8ad81eda0a84277bc85171a88269 Gerrit-Change-Number: 13409 Gerrit-PatchSet: 4 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13400 ) Change subject: IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration .. Patch Set 9: > When the verify job fails and you rebase and rerun it, its helpful > if you post why it failed, eg. due to known issue IMPALA-8567 Got it, thanks! Will do it next time. -- To view, visit http://gerrit.cloudera.org:8080/13400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc Gerrit-Change-Number: 13400 Gerrit-PatchSet: 9 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Wed, 05 Jun 2019 17:26:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS integration
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13409 ) Change subject: IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS integration .. Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/13409/2/testdata/workloads/functional-query/queries/QueryTest/kudu_hms_alter.test File testdata/workloads/functional-query/queries/QueryTest/kudu_hms_alter.test: http://gerrit.cloudera.org:8080/#/c/13409/2/testdata/workloads/functional-query/queries/QueryTest/kudu_hms_alter.test@10 PS2, Line 10: # Alter master address to a different location > Is this test relevant to HMS alter? Done http://gerrit.cloudera.org:8080/#/c/13409/2/testdata/workloads/functional-query/queries/QueryTest/kudu_hms_alter.test@60 PS2, Line 60: QUERY > I am not sure I understand the relevance of all these range partitioning ch I think it is nice to ensure alter a table to add /drop range partitions is still working with HMS integration enabled. http://gerrit.cloudera.org:8080/#/c/13409/2/testdata/workloads/functional-query/queries/QueryTest/kudu_hms_alter.test@274 PS2, Line 274: # Add a row > The tests altering columns make sense. ACK -- To view, visit http://gerrit.cloudera.org:8080/13409 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If7155e0b385b8ad81eda0a84277bc85171a88269 Gerrit-Change-Number: 13409 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Tue, 04 Jun 2019 18:56:46 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS integration
Hello Thomas Marshall, Alexey Serbin, Andrew Wong, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13409 to look at the new patch set (#3). Change subject: IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS integration .. IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS integration This commit intends to support the actual handling of ALTER/RENAME TABLE DDL for managed Kudu tables with Kudu's integration with the Hive Metastore. However, currently Kudu is considered as the source of truth of the table schema, so when ALTER TABLE (ADD/DROP COLUMN/RANGE_PARTITION), Impala always directly alters/loads the Kudu tables. Thus, this commit only updates RENAME TABLE DDL, so that after the table is renamed in the Kudu, relies on Kudu to rename the table in the HMS. Change-Id: If7155e0b385b8ad81eda0a84277bc85171a88269 --- M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java A testdata/workloads/functional-query/queries/QueryTest/kudu_hms_alter.test M tests/custom_cluster/test_kudu.py M tests/query_test/test_kudu.py 4 files changed, 660 insertions(+), 15 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/13409/3 -- To view, visit http://gerrit.cloudera.org:8080/13409 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If7155e0b385b8ad81eda0a84277bc85171a88269 Gerrit-Change-Number: 13409 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8505: Disallow setting Kudu table id in ALTER TABLE
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13465 ) Change subject: IMPALA-8505: Disallow setting Kudu table id in ALTER TABLE .. Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/13465/2/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java File fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java: http://gerrit.cloudera.org:8080/#/c/13465/2/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java@118 PS2, Line 118: // Checking for 'EXTERNAL' is case-insensitive, see IMPALA-5637. : String keyForExternalProperty = : MetaStoreUtil.findTblPropKeyCaseInsensitive(tblProperties_, "EXTERNAL"); > nit: while you are here, maybe move this under the 'if (authzConfig.isEnabl Done http://gerrit.cloudera.org:8080/#/c/13465/2/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java@132 PS2, Line 132: manually > nit: what does 'manually' means in this context? Is it possible to set it Done http://gerrit.cloudera.org:8080/#/c/13465/2/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java@132 PS2, Line 132: > nit: drop the extra space or drop the space and the period altogether. Done -- To view, visit http://gerrit.cloudera.org:8080/13465 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibc1f3f87054db5e28f72a5b54ea5b3c040c1bf22 Gerrit-Change-Number: 13465 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Tue, 04 Jun 2019 18:19:41 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8505: Disallow setting Kudu table id in ALTER TABLE
Hello Thomas Marshall, Alexey Serbin, Andrew Wong, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13465 to look at the new patch set (#3). Change subject: IMPALA-8505: Disallow setting Kudu table id in ALTER TABLE .. IMPALA-8505: Disallow setting Kudu table id in ALTER TABLE This commit disallows explicitly setting the Kudu table id property for Kudu tables in a ALTER TABLE set statement. The Kudu table id property is generated when the table is created and should not be altered afterwards. This commit also extracts Kudu-related analyzing tests for table alteration, so that they can be run with or without Kudu/HMS integration enabled. Change-Id: Ibc1f3f87054db5e28f72a5b54ea5b3c040c1bf22 --- M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java 3 files changed, 135 insertions(+), 126 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/65/13465/3 -- To view, visit http://gerrit.cloudera.org:8080/13465 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibc1f3f87054db5e28f72a5b54ea5b3c040c1bf22 Gerrit-Change-Number: 13465 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration
Hello Thomas Marshall, Alexey Serbin, Andrew Wong, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13400 to look at the new patch set (#7). Change subject: IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration .. IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration This commit supports the actual handling of DROP TABLE DDL for managed Kudu tables with Kudu's integration with the Hive Metastore. When Kudu/HMS integration is enabled, after the table is dropped in the Kudu, relies on Kudu to drop the table in the HMS. Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc --- M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M tests/custom_cluster/test_kudu.py 2 files changed, 149 insertions(+), 41 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/13400/7 -- To view, visit http://gerrit.cloudera.org:8080/13400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc Gerrit-Change-Number: 13400 Gerrit-PatchSet: 7 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration
Hello Thomas Marshall, Alexey Serbin, Andrew Wong, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13400 to look at the new patch set (#6). Change subject: IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration .. IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration This commit supports the actual handling of DROP TABLE DDL for managed Kudu tables with Kudu's integration with the Hive Metastore. When Kudu/HMS integration is enabled, after the table is dropped in the Kudu, relies on Kudu to drop the table in the HMS. Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc --- M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M tests/custom_cluster/test_kudu.py 2 files changed, 149 insertions(+), 41 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/13400/6 -- To view, visit http://gerrit.cloudera.org:8080/13400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc Gerrit-Change-Number: 13400 Gerrit-PatchSet: 6 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13400 ) Change subject: IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration .. Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/13400/5/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/13400/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1590 PS5, Line 1590: KuduTable.isKuduTable(msTbl) && !Table.isExternalTable(msTbl); > nit: Maybe separate setting isManagedKuduTable from the drop. Done http://gerrit.cloudera.org:8080/#/c/13400/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1943 PS5, Line 1943: // the HMS table for managed tables. > Nit, can you add comment here too. Done -- To view, visit http://gerrit.cloudera.org:8080/13400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc Gerrit-Change-Number: 13400 Gerrit-PatchSet: 6 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Tue, 04 Jun 2019 17:31:30 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration
Hello Thomas Marshall, Alexey Serbin, Andrew Wong, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13375 to look at the new patch set (#9). Change subject: IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration .. IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration This commit supports the actual handling of CREATE TABLE DDL for managed Kudu tables when integration with Hive Metastore is enabled. When Kudu/HMS integration is enabled, for CREATE TABLE statement, Impala can rely on Kudu to create the table in the HMS. Change-Id: Icffe412395f47f5e07d97bad457020770cfa7502 --- M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java M tests/common/custom_cluster_test_suite.py M tests/common/kudu_test_suite.py M tests/common/skip.py M tests/custom_cluster/test_kudu.py M tests/query_test/test_kudu.py 10 files changed, 306 insertions(+), 20 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/13375/9 -- To view, visit http://gerrit.cloudera.org:8080/13375 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icffe412395f47f5e07d97bad457020770cfa7502 Gerrit-Change-Number: 13375 Gerrit-PatchSet: 9 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13400 ) Change subject: IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration .. Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/13400/4/tests/custom_cluster/test_kudu.py File tests/custom_cluster/test_kudu.py: http://gerrit.cloudera.org:8080/#/c/13400/4/tests/custom_cluster/test_kudu.py@117 PS4, Line 117: class TestKuduHMSIntegration(CustomClusterTestSuite, KuduTestSuite): > Could you add a TODO here references the JIRA you filed and mentioning that Done http://gerrit.cloudera.org:8080/#/c/13400/4/tests/custom_cluster/test_kudu.py@284 PS4, Line 284: ) > flake8: W292 no newline at end of file Done -- To view, visit http://gerrit.cloudera.org:8080/13400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc Gerrit-Change-Number: 13400 Gerrit-PatchSet: 5 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Mon, 03 Jun 2019 23:32:02 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS integration
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/13409 )
Change subject: IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS
integration
..
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/13409/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/13409/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2584
PS1, Line 2584: if (isManagedKuduTable) {
: Preconditions.checkState(KuduTable.isKuduTable(m
> nit: mind updating the comment to explain the conditional block, instead of
Done
http://gerrit.cloudera.org:8080/#/c/13409/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2797
PS1, Line 2797: tbl.getMetaStoreTable().deepCopy();
> Could you clarify what we need to do here? How does this code need to be up
I think we should throw exception for alter kudu.table_name property for
managed tables as it is disallowed since IMPALA_5654. Filed a JIRA to track it.
http://gerrit.cloudera.org:8080/#/c/13409/1/testdata/workloads/functional-query/queries/QueryTest/kudu_hms_alter.test
File testdata/workloads/functional-query/queries/QueryTest/kudu_hms_alter.test:
PS1:
> Does the .test format allow for file-level comments? If so, might be worth
kudu_alter.test cannot be used because it contains tests with hardcoded legacy
table naming 'impala::' . I try to refactor it but it looks like not easy to.
--
To view, visit http://gerrit.cloudera.org:8080/13409
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7155e0b385b8ad81eda0a84277bc85171a88269
Gerrit-Change-Number: 13409
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Marshall
Gerrit-Comment-Date: Mon, 03 Jun 2019 23:32:07 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration
Hello Thomas Marshall, Alexey Serbin, Andrew Wong, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13400 to look at the new patch set (#5). Change subject: IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration .. IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration This commit supports the actual handling of DROP TABLE DDL for managed Kudu tables with Kudu's integration with the Hive Metastore. When Kudu/HMS integration is enabled, after the table is dropped in the Kudu, relies on Kudu to drop the table in the HMS. Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc --- M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M tests/custom_cluster/test_kudu.py 2 files changed, 148 insertions(+), 41 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/13400/5 -- To view, visit http://gerrit.cloudera.org:8080/13400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc Gerrit-Change-Number: 13400 Gerrit-PatchSet: 5 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS integration
Hello Thomas Marshall, Alexey Serbin, Andrew Wong, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13409 to look at the new patch set (#2). Change subject: IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS integration .. IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS integration This commit intends to support the actual handling of ALTER/RENAME TABLE DDL for managed Kudu tables with Kudu's integration with the Hive Metastore. However, currently Kudu is considered as the source of truth of the table schema, so when ALTER TABLE (ADD/DROP COLUMN/RANGE_PARTITION), Impala always directly alters/loads the Kudu tables. Thus, this commit only updates RENAME TABLE DDL, so that after the table is renamed in the Kudu, relies on Kudu to rename the table in the HMS. Change-Id: If7155e0b385b8ad81eda0a84277bc85171a88269 --- M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java A testdata/workloads/functional-query/queries/QueryTest/kudu_hms_alter.test M tests/custom_cluster/test_kudu.py M tests/query_test/test_kudu.py 4 files changed, 666 insertions(+), 15 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/13409/2 -- To view, visit http://gerrit.cloudera.org:8080/13409 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If7155e0b385b8ad81eda0a84277bc85171a88269 Gerrit-Change-Number: 13409 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration
Hello Thomas Marshall, Alexey Serbin, Andrew Wong, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13375 to look at the new patch set (#7). Change subject: IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration .. IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration This commit supports the actual handling of CREATE TABLE DDL for managed Kudu tables when integration with Hive Metastore is enabled. When Kudu/HMS integration is enabled, for CREATE TABLE statement, Impala can rely on Kudu to create the table in the HMS. Change-Id: Icffe412395f47f5e07d97bad457020770cfa7502 --- M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java M tests/common/custom_cluster_test_suite.py M tests/common/kudu_test_suite.py M tests/common/skip.py M tests/custom_cluster/test_kudu.py M tests/query_test/test_kudu.py 10 files changed, 306 insertions(+), 20 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/13375/7 -- To view, visit http://gerrit.cloudera.org:8080/13375 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icffe412395f47f5e07d97bad457020770cfa7502 Gerrit-Change-Number: 13375 Gerrit-PatchSet: 7 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] Disable custom cluster/service FE tests on S3
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13500 ) Change subject: Disable custom cluster/service FE tests on S3 .. Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/13500/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13500/2//COMMIT_MSG@7 PS2, Line 7: custom > typo Done http://gerrit.cloudera.org:8080/#/c/13500/2//COMMIT_MSG@9 PS2, Line 9: custom > typo Done -- To view, visit http://gerrit.cloudera.org:8080/13500 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaa95fb1cef6349afafe3fd3677df74f7fa225245 Gerrit-Change-Number: 13500 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 03 Jun 2019 21:44:09 + Gerrit-HasComments: Yes
[Impala-ASF-CR] Disable custom cluster/service FE tests on S3
Hello Thomas Marshall, Lars Volker, Tim Armstrong, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13500 to look at the new patch set (#3). Change subject: Disable custom cluster/service FE tests on S3 .. Disable custom cluster/service FE tests on S3 This patch disables the custom cluster/service FE tests when running against S3, as only S3 FE tests should be ran under such environment. Change-Id: Iaa95fb1cef6349afafe3fd3677df74f7fa225245 --- M bin/run-all-tests.sh 1 file changed, 8 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/13500/3 -- To view, visit http://gerrit.cloudera.org:8080/13500 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iaa95fb1cef6349afafe3fd3677df74f7fa225245 Gerrit-Change-Number: 13500 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration
Hello Thomas Marshall, Alexey Serbin, Andrew Wong, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13375 to look at the new patch set (#6). Change subject: IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration .. IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration This commit supports the actual handling of CREATE TABLE DDL for managed Kudu tables when integration with Hive Metastore is enabled. When Kudu/HMS integration is enabled, for CREATE TABLE statement, Impala can rely on Kudu to create the table in the HMS. Change-Id: Icffe412395f47f5e07d97bad457020770cfa7502 --- M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java M tests/common/custom_cluster_test_suite.py M tests/common/kudu_test_suite.py M tests/common/skip.py M tests/custom_cluster/test_kudu.py M tests/query_test/test_kudu.py 10 files changed, 306 insertions(+), 20 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/13375/6 -- To view, visit http://gerrit.cloudera.org:8080/13375 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icffe412395f47f5e07d97bad457020770cfa7502 Gerrit-Change-Number: 13375 Gerrit-PatchSet: 6 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] Disable cumstom cluster/service FE tests on S3
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/13500 )
Change subject: Disable cumstom cluster/service FE tests on S3
..
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/13500/1/bin/run-all-tests.sh
File bin/run-all-tests.sh:
http://gerrit.cloudera.org:8080/#/c/13500/1/bin/run-all-tests.sh@250
PS1, Line 250: if [[ "$FE_TEST" == true && "${TARGET_FILESYSTEM}" != "s3"
]]; then
> I think it would be better to check if FE_TEST is true. I think we want to
Done
http://gerrit.cloudera.org:8080/#/c/13500/1/bin/run-all-tests.sh@257
PS1, Line 257: fi
> pushd and popd are now mismatched.
Done
--
To view, visit http://gerrit.cloudera.org:8080/13500
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaa95fb1cef6349afafe3fd3677df74f7fa225245
Gerrit-Change-Number: 13500
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Marshall
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Mon, 03 Jun 2019 19:24:13 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] Disable cumstom cluster/service FE tests on S3
Hello Thomas Marshall, Tim Armstrong, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13500 to look at the new patch set (#2). Change subject: Disable cumstom cluster/service FE tests on S3 .. Disable cumstom cluster/service FE tests on S3 This patch disables the cumstom cluster/service FE tests when running against S3, as only S3 FE tests should be ran under such environment. Change-Id: Iaa95fb1cef6349afafe3fd3677df74f7fa225245 --- M bin/run-all-tests.sh 1 file changed, 8 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/13500/2 -- To view, visit http://gerrit.cloudera.org:8080/13500 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iaa95fb1cef6349afafe3fd3677df74f7fa225245 Gerrit-Change-Number: 13500 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13375 ) Change subject: IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/13375/5/tests/query_test/test_kudu.py File tests/query_test/test_kudu.py: http://gerrit.cloudera.org:8080/#/c/13375/5/tests/query_test/test_kudu.py@1130 PS5, Line 1130: @SkipIfKudu.hms_integration_enabled > I still think that a lot of the tests in this file and elsewhere that have The tests that are skipped when HMS integration enabled for mainly two reasons: 1) the table name has "impala::" prefix hardcoded. 2) using external table, which could has name collision with managed table created by Kudu. Yeah, I think it makes sense to parameterize some tests so that they all run both with and without hms integration enabled. (I was trying to do it, however don't find a good way to achieve it yet). I can file a JIRA to track this. Non HMS integrated mode is not something going to be aggressively deprecated. So we are planning to maintain both mode for awhile. -- To view, visit http://gerrit.cloudera.org:8080/13375 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icffe412395f47f5e07d97bad457020770cfa7502 Gerrit-Change-Number: 13375 Gerrit-PatchSet: 5 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Mon, 03 Jun 2019 19:15:47 + Gerrit-HasComments: Yes
[Impala-ASF-CR] Disable cumstom cluster/service FE tests on S3
Hao Hao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13500 Change subject: Disable cumstom cluster/service FE tests on S3 .. Disable cumstom cluster/service FE tests on S3 This patch disables the cumstom cluster/service FE tests when running against S3, as only S3 FE tests should be ran under such environment. Change-Id: Iaa95fb1cef6349afafe3fd3677df74f7fa225245 --- M bin/run-all-tests.sh 1 file changed, 7 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/13500/1 -- To view, visit http://gerrit.cloudera.org:8080/13500 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Iaa95fb1cef6349afafe3fd3677df74f7fa225245 Gerrit-Change-Number: 13500 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao
[Impala-ASF-CR] IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration
Hello Thomas Marshall, Alexey Serbin, Andrew Wong, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13400 to look at the new patch set (#4). Change subject: IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration .. IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration This commit supports the actual handling of DROP TABLE DDL for managed Kudu tables with Kudu's integration with the Hive Metastore. When Kudu/HMS integration is enabled, after the table is dropped in the Kudu, relies on Kudu to drop the table in the HMS. Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc --- M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M tests/custom_cluster/test_kudu.py 2 files changed, 146 insertions(+), 41 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/13400/4 -- To view, visit http://gerrit.cloudera.org:8080/13400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc Gerrit-Change-Number: 13400 Gerrit-PatchSet: 4 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration
Hello Thomas Marshall, Alexey Serbin, Andrew Wong, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13400 to look at the new patch set (#3). Change subject: IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration .. IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration This commit supports the actual handling of DROP TABLE DDL for managed Kudu tables with Kudu's integration with the Hive Metastore. When Kudu/HMS integration is enabled, after the table is dropped in the Kudu, relies on Kudu to drop the table in the HMS. Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc --- M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M tests/custom_cluster/test_kudu.py 2 files changed, 145 insertions(+), 41 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/13400/3 -- To view, visit http://gerrit.cloudera.org:8080/13400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc Gerrit-Change-Number: 13400 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/13400 )
Change subject: IMPALA-8507: Support DROP TABLE statement with Kudu/HMS
integration
..
Patch Set 3:
(12 comments)
http://gerrit.cloudera.org:8080/#/c/13400/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/13400/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1439
PS2, Line 1439: // The Kudu tables in the HMS should have been dropped at
this point
: // with the Hive Metastore integrat
> Shouldn't this be true regardless of HMS integration? Isn't that what dropT
Sorry for the confusion, I mean the Kudu table in the HMS should be deleted at
this pointed.
http://gerrit.cloudera.org:8080/#/c/13400/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1508
PS2, Line 1508: KuduCatalogOpExecutor.dropTable(msTable, /*if exists*/ true);
> Does it wait for the table to be actually dropped from Kudu?
Are you referring to wait for the table to be actually dropped in the HMS when
HMS integration is enabled? I think this is not necessary. The table should be
dropped in the HMS if dropping the table in Kudu returns successfully.
http://gerrit.cloudera.org:8080/#/c/13400/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1512
PS2, Line 1512: private boolean isKuduHmsIntegrationEnab
> The CatalogOpExecutor class isn't Kudu-specific; this should probably be na
Done
http://gerrit.cloudera.org:8080/#/c/13400/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1613
PS2, Line 1613: // When Kudu's HMS integration is enabled, Kudu will drop
the managed table
> Please add a comment explaining the rationale of this condition, eg
Done
http://gerrit.cloudera.org:8080/#/c/13400/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1616
PS2, Line 1616: if (!isManagedKuduTable ||
!isKuduHmsIntegrationEnabled(msTbl)) {
> line too long (91 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/13400/2/tests/custom_cluster/test_kudu.py
File tests/custom_cluster/test_kudu.py:
http://gerrit.cloudera.org:8080/#/c/13400/2/tests/custom_cluster/test_kudu.py@124
PS2, Line 124:
> While adding this attribute, could you justify on the reasoning in the doc
My understanding is that all tests in under custom_cluster dir should run
serially because all of them requires restart certain components with
customized configurations.
http://gerrit.cloudera.org:8080/#/c/13400/2/tests/custom_cluster/test_kudu.py@133
PS2, Line 133: def teardown_class(cls):
> I'm not sure this test scenario requires hybrid clock to be present. Why w
Yeah, I think this is actually not needed.
http://gerrit.cloudera.org:8080/#/c/13400/2/tests/custom_cluster/test_kudu.py@138
PS2, Line 138:
> What if db_name is 'default' ?
I think the unique_cursor ensured the db name is unique.
http://gerrit.cloudera.org:8080/#/c/13400/2/tests/custom_cluster/test_kudu.py@150
PS2, Line 150: """
> Ditto for the reasoning to skip the test if no hybrid clock is around: why
Done
http://gerrit.cloudera.org:8080/#/c/13400/2/tests/custom_cluster/test_kudu.py@153
PS2, Line 153: elf.get_kudu_table_base_name(kudu_table.
> nit: move this into a separate sentence for easier comprehension and maybe
Done
http://gerrit.cloudera.org:8080/#/c/13400/2/tests/custom_cluster/test_kudu.py@155
PS2, Line 155: table_n
> What if db_name is 'default'?
Same as above.
http://gerrit.cloudera.org:8080/#/c/13400/2/tests/custom_cluster/test_kudu.py@168
PS2, Line 168: assert ["", "storage_handler", "org.apache.kudu.hive
> Interesting, in case of Kudu+HMS systests which I ran on recent CDH6.x this
I don't quite understand why you encountered scenario that after dropping the
table in Impala, the kudu table is dropped after some time. Because drop table
in Impala will call Kudu drop table API explicitly which should be a sync call.
Would you mind trying again with the latest bits and with my patches included?
Thanks!
--
To view, visit http://gerrit.cloudera.org:8080/13400
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc
Gerrit-Change-Number: 13400
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Marshall
Gerrit-Comment-Date: Mon, 03 Jun 2019 07:11:05 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration
Hello Thomas Marshall, Alexey Serbin, Andrew Wong, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13375 to look at the new patch set (#5). Change subject: IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration .. IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration This commit supports the actual handling of CREATE TABLE DDL for managed Kudu tables when integration with Hive Metastore is enabled. When Kudu/HMS integration is enabled, for CREATE TABLE statement, Impala can rely on Kudu to create the table in the HMS. Change-Id: Icffe412395f47f5e07d97bad457020770cfa7502 --- M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java M tests/common/custom_cluster_test_suite.py M tests/common/kudu_test_suite.py M tests/common/skip.py M tests/custom_cluster/test_kudu.py M tests/query_test/test_kudu.py 10 files changed, 305 insertions(+), 20 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/13375/5 -- To view, visit http://gerrit.cloudera.org:8080/13375 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icffe412395f47f5e07d97bad457020770cfa7502 Gerrit-Change-Number: 13375 Gerrit-PatchSet: 5 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/13375 )
Change subject: IMPALA-8504 (part 2): Support CREATE TABLE statement with
Kudu/HMS integration
..
Patch Set 4:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/13375/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/13375/4/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@172
PS4, Line 172: org.apache.hadoop.hive.metastore.api.Table msTbl,
: boolean isHMSIntegrationEnabled
> these can fit on the same line
It exceeds the line length limit (91).
http://gerrit.cloudera.org:8080/#/c/13375/4/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@231
PS4, Line 231: if (hmsHosts != null && kuduHmsHosts != null
&(kuduHmsHosts)) {
> nit: separate by a space
Done
http://gerrit.cloudera.org:8080/#/c/13375/4/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@234
PS4, Line 234: throw new ImpalaRuntimeException(
: String.format("Kudu is integrated with a different
Hive Metastore " +
: "than that used by Impala, Kudu is configured to
use the HMS: " +
: "%s, while Impala is configured to use the HMS:
%s",
: kuduHmsUris, hmsUris));
> readability nit: remove 'else' and move this out of the scope if/else scope
Done
http://gerrit.cloudera.org:8080/#/c/13375/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/13375/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1930
PS4, Line 1930: !KuduTable.isHMSIntegrationEnabledAndValidate(masterHosts,
:
hmsUris
> formatting - put everything after the '=' on the next line, indented by 4
Done
http://gerrit.cloudera.org:8080/#/c/13375/4/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
File fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java:
http://gerrit.cloudera.org:8080/#/c/13375/4/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java@87
PS4, Line 87: public static final String HIVE_METASTORE_URIS_KEY =
: "hive.metastore.uris";
> This can fit on 1 line
Done
http://gerrit.cloudera.org:8080/#/c/13375/4/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java@120
PS4, Line 120: /**
:* Return the value of thrift URI for the remote Hive
Metastore.
:*/
: public static String
getHiveMetastoreUrisKeyValue(IMetaStoreClient client)
: throws ConfigValSecurityException, TException {
: return client.getConfigValue(
: HIVE_METASTORE_URIS_KEY, DEFAULT_HIVE_METASTORE_URIS);
: }
> Why to create this shortcut instead of just using getMetastoreConfigValue()
This is to ensure always use the correct key (HIVE_METASTORE_URIS_KEY) when
calling this function.
http://gerrit.cloudera.org:8080/#/c/13375/4/tests/custom_cluster/test_kudu.py
File tests/custom_cluster/test_kudu.py:
http://gerrit.cloudera.org:8080/#/c/13375/4/tests/custom_cluster/test_kudu.py@129
PS4, Line 129: manged
> typo
Done
http://gerrit.cloudera.org:8080/#/c/13375/4/tests/query_test/test_kudu.py
File tests/query_test/test_kudu.py:
http://gerrit.cloudera.org:8080/#/c/13375/4/tests/query_test/test_kudu.py@93
PS4, Line 93: @SkipIfKudu.hms_integration_enabled
> Why would a test like this, which is focused on scanning kudu tables, need
Done
http://gerrit.cloudera.org:8080/#/c/13375/4/tests/query_test/test_kudu.py@175
PS4, Line 175: @SkipIfKudu.hms_integration_enabled
> I assume that for cases like these, where the table is modified outside of
I am not sure it is the case, as far as my understanding of IMPALA-4828 is due
to the table schema is cached in impala which is different from the one in
Kudu. While with HMS integration, Kudu only automatically update the schema in
the HMS. Therefore, I don't see the behavior will change. Or I am missing
something?
--
To view, visit http://gerrit.cloudera.org:8080/13375
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icffe412395f47f5e07d97bad457020770cfa7502
Gerrit-Change-Number: 13375
Gerrit-PatchSet: 4
Gerrit-Owner: Hao Hao
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Marshall
Gerrit-Comment-Date: Sun, 02 Jun 2019 20:57:00 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8503: allow the Hive Metastore to start with kudu-hive plugin
Hello Thomas Marshall, Alexey Serbin, Andrew Wong, Adar Dembo, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13319 to look at the new patch set (#5). Change subject: IMPALA-8503: allow the Hive Metastore to start with kudu-hive plugin .. IMPALA-8503: allow the Hive Metastore to start with kudu-hive plugin This patch allows to start the Hive Metasotre with Kudu plugin which is required for enabling Kudu's integration with the HMS. The Kudu plugin is downloaded and extracted from native-toolchain S3 bucket. Change-Id: I4bd1488ced51840ec986d29ed371e26168abcc76 --- M bin/bootstrap_toolchain.py M bin/impala-config.sh M fe/src/test/resources/hive-site.xml.py M impala-parent/pom.xml M testdata/bin/run-hive-server.sh 5 files changed, 22 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/13319/5 -- To view, visit http://gerrit.cloudera.org:8080/13319 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4bd1488ced51840ec986d29ed371e26168abcc76 Gerrit-Change-Number: 13319 Gerrit-PatchSet: 5 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8503: allow the Hive Metastore to start with kudu-hive plugin
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13319 ) Change subject: IMPALA-8503: allow the Hive Metastore to start with kudu-hive plugin .. Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/13319/3/bin/bootstrap_toolchain.py File bin/bootstrap_toolchain.py: http://gerrit.cloudera.org:8080/#/c/13319/3/bin/bootstrap_toolchain.py@571 PS3, Line 571: # Always download Kudu's jars regardless of USE_CDH_KUDU since they > Could you add a comment here like: Done http://gerrit.cloudera.org:8080/#/c/13319/3/bin/impala-config.sh File bin/impala-config.sh: http://gerrit.cloudera.org:8080/#/c/13319/3/bin/impala-config.sh@670 PS3, Line 670: > I don't think this logic works - you'll get a weird result in the case that Thanks for the suggestion, tested with set USE_CDH_KUDU=false, and the cdh_components/kudu... dir contains expected jars. -- To view, visit http://gerrit.cloudera.org:8080/13319 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4bd1488ced51840ec986d29ed371e26168abcc76 Gerrit-Change-Number: 13319 Gerrit-PatchSet: 4 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Sat, 01 Jun 2019 05:48:47 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8503: allow the Hive Metastore to start with kudu-hive plugin
Hello Thomas Marshall, Alexey Serbin, Andrew Wong, Adar Dembo, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13319 to look at the new patch set (#4). Change subject: IMPALA-8503: allow the Hive Metastore to start with kudu-hive plugin .. IMPALA-8503: allow the Hive Metastore to start with kudu-hive plugin This patch allows to start the Hive Metasotre with Kudu plugin which is required for enabling Kudu's integration with the HMS. The Kudu plugin is downloaded and extracted from native-toolchain S3 bucket. Change-Id: I4bd1488ced51840ec986d29ed371e26168abcc76 --- M bin/bootstrap_toolchain.py M bin/impala-config.sh M fe/src/test/resources/hive-site.xml.py M impala-parent/pom.xml M testdata/bin/run-hive-server.sh 5 files changed, 21 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/13319/4 -- To view, visit http://gerrit.cloudera.org:8080/13319 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4bd1488ced51840ec986d29ed371e26168abcc76 Gerrit-Change-Number: 13319 Gerrit-PatchSet: 4 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8503: allow the Hive Metastore to start with kudu-hive plugin
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13319 ) Change subject: IMPALA-8503: allow the Hive Metastore to start with kudu-hive plugin .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/13319/2/bin/bootstrap_toolchain.py File bin/bootstrap_toolchain.py: http://gerrit.cloudera.org:8080/#/c/13319/2/bin/bootstrap_toolchain.py@424 PS2, Line 424: # Kudu is the only component that's platform dependent. > So as written this isn't going to work for non-cdh supported platforms (eg. I see, thanks for the suggestion! I opt for naming the kudu java package 'kudu-java' to avoid touching much existing code. -- To view, visit http://gerrit.cloudera.org:8080/13319 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4bd1488ced51840ec986d29ed371e26168abcc76 Gerrit-Change-Number: 13319 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Fri, 31 May 2019 22:29:04 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8503: allow the Hive Metastore to start with kudu-hive plugin
Hello Thomas Marshall, Alexey Serbin, Andrew Wong, Adar Dembo, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13319 to look at the new patch set (#3). Change subject: IMPALA-8503: allow the Hive Metastore to start with kudu-hive plugin .. IMPALA-8503: allow the Hive Metastore to start with kudu-hive plugin This patch allows to start the Hive Metasotre with Kudu plugin which is required for enabling Kudu's integration with the HMS. The Kudu plugin is downloaded and extracted from native-toolchain S3 bucket. Change-Id: I4bd1488ced51840ec986d29ed371e26168abcc76 --- M bin/bootstrap_toolchain.py M bin/impala-config.sh M fe/src/test/resources/hive-site.xml.py M testdata/bin/run-hive-server.sh 4 files changed, 19 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/13319/3 -- To view, visit http://gerrit.cloudera.org:8080/13319 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4bd1488ced51840ec986d29ed371e26168abcc76 Gerrit-Change-Number: 13319 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration
Hello Thomas Marshall, Mike Percy, Alexey Serbin, Andrew Wong, Adar Dembo, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13318 to look at the new patch set (#12). Change subject: IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration .. IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration This commit adds support for the syntax of CREATE TABLE (and CTAS) statements for managed Kudu tables with Kudu/HMS integration. A follow up patch will address the actual handling of CREATE TABLE statement with Kudu/HMS integration. For a managed table the syntax remains the same. However, the detailed changes includes: 1) Kudu table will always be created with the new Kudu storage handler 'org.apache.kudu.hive.KuduStorageHandler' even when Kudu/HMS integration is disabled. The legacy storage handler will be eventually deprecated. 2) When Kudu/HMS integration is enabled, the Kudu table underneath the managed HMS table will follow the naming convention 'db_name.table_name' instead of 'impala::db_name.table_name'. 3) Add 'kudu.table_id' table property to be used with Kudu/HMS integration. This commit also extracts Kudu-related DDL parsing and analyzing tests, so that they can be run with or without Kudu/HMS integration enabled. Change-Id: I465673d749221bd5f3772814b1c22c2673a53f5c --- M bin/run-all-tests.sh M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/util/KuduUtil.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java A fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java A fe/src/test/java/org/apache/impala/analysis/AuditingKuduTest.java M fe/src/test/java/org/apache/impala/analysis/AuditingTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java A fe/src/test/java/org/apache/impala/customservice/CustomServiceRunner.java A fe/src/test/java/org/apache/impala/customservice/KuduHMSIntegrationTest.java M testdata/cluster/node_templates/common/etc/init.d/kudu-master M tests/query_test/test_kudu.py 16 files changed, 920 insertions(+), 577 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/13318/12 -- To view, visit http://gerrit.cloudera.org:8080/13318 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I465673d749221bd5f3772814b1c22c2673a53f5c Gerrit-Change-Number: 13318 Gerrit-PatchSet: 12 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8503: allow the Hive Metastore to start with kudu-hive plugin
Hello Thomas Marshall, Alexey Serbin, Andrew Wong, Adar Dembo, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13319 to look at the new patch set (#2). Change subject: IMPALA-8503: allow the Hive Metastore to start with kudu-hive plugin .. IMPALA-8503: allow the Hive Metastore to start with kudu-hive plugin This patch allows to start the Hive Metasotre with Kudu plugin which is required for enabling Kudu's integration with the HMS. The Kudu plugin is downloaded and extracted from native-toolchain S3 bucket. Change-Id: I4bd1488ced51840ec986d29ed371e26168abcc76 --- M bin/bootstrap_toolchain.py M fe/src/test/resources/hive-site.xml.py M testdata/bin/run-hive-server.sh 3 files changed, 18 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/13319/2 -- To view, visit http://gerrit.cloudera.org:8080/13319 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4bd1488ced51840ec986d29ed371e26168abcc76 Gerrit-Change-Number: 13319 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8502: Bump CDH BUILD NUMBER and Kudu version
Hello Thomas Marshall, Alexey Serbin, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13481 to look at the new patch set (#3). Change subject: IMPALA-8502: Bump CDH_BUILD_NUMBER and Kudu version .. IMPALA-8502: Bump CDH_BUILD_NUMBER and Kudu version This bumps CDH_BUILD_NUMBER to 1137441 to bring in kudu-hive.jar which is recently added to impala-minicluster-tarballs for testing Kudu tables with the Hive Metastore integration enabled. It also brings in the latest Kudu side changes which adjust external Kudu table handling. Change-Id: I4c9c7d264f08454d9da8ad2d06ead643214ee23e --- M bin/impala-config.sh 1 file changed, 3 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/13481/3 -- To view, visit http://gerrit.cloudera.org:8080/13481 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4c9c7d264f08454d9da8ad2d06ead643214ee23e Gerrit-Change-Number: 13481 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8502: Bump CDH BUILD NUMBER and Kudu version
Hello Thomas Marshall, Alexey Serbin, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13481 to look at the new patch set (#2). Change subject: IMPALA-8502: Bump CDH_BUILD_NUMBER and Kudu version .. IMPALA-8502: Bump CDH_BUILD_NUMBER and Kudu version This bumps CDH_BUILD_NUMBER to 1137441 to bring in kudu-hive.jar which is recently added to impala-minicluster-tarballs for testing Kudu tables with the Hive Metastore integration enabled. It also brings in the latest Kudu side changes which adjust external Kudu table handling. Change-Id: I4c9c7d264f08454d9da8ad2d06ead643214ee23e --- M bin/impala-config.sh 1 file changed, 3 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/13481/2 -- To view, visit http://gerrit.cloudera.org:8080/13481 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4c9c7d264f08454d9da8ad2d06ead643214ee23e Gerrit-Change-Number: 13481 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8502: Bump CDH BUILD NUMBER and Kudu version
Hao Hao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13481 Change subject: IMPALA-8502: Bump CDH_BUILD_NUMBER and Kudu version .. IMPALA-8502: Bump CDH_BUILD_NUMBER and Kudu version This bumps CDH_BUILD_NUMBER to 1137441 to bring in kudu-hive.jar which is recently added to impala-minicluster-tarballs for testing Kudu tables with the Hive Metastore integration enabled. It also brings in the latest Kudu side changes which adjust external Kudu table handling. Change-Id: I4c9c7d264f08454d9da8ad2d06ead643214ee23e --- M bin/impala-config.sh 1 file changed, 3 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/13481/1 -- To view, visit http://gerrit.cloudera.org:8080/13481 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I4c9c7d264f08454d9da8ad2d06ead643214ee23e Gerrit-Change-Number: 13481 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao
[Impala-ASF-CR] IMPALA-8505: Disallow setting Kudu table id in ALTER TABLE
Hao Hao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13465 Change subject: IMPALA-8505: Disallow setting Kudu table id in ALTER TABLE .. IMPALA-8505: Disallow setting Kudu table id in ALTER TABLE This commit disallows explicitly setting the Kudu table id property for Kudu tables in a ALTER TABLE set statement. The Kudu table id property is generated when the table is created and should not be altered afterwards. This commit also extracts Kudu-related analyzing tests for table alteration, so that they can be run with or without Kudu/HMS integration enabled. Change-Id: Ibc1f3f87054db5e28f72a5b54ea5b3c040c1bf22 --- M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java 3 files changed, 132 insertions(+), 122 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/65/13465/1 -- To view, visit http://gerrit.cloudera.org:8080/13465 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ibc1f3f87054db5e28f72a5b54ea5b3c040c1bf22 Gerrit-Change-Number: 13465 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao
[Impala-ASF-CR] IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration
Hello Thomas Marshall, Mike Percy, Alexey Serbin, Andrew Wong, Adar Dembo, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13318 to look at the new patch set (#10). Change subject: IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration .. IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration This commit adds support for the syntax of CREATE TABLE (and CTAS) statements for managed Kudu tables with Kudu/HMS integration. A follow up patch will address the actual handling of CREATE TABLE statement with Kudu/HMS integration. For a managed table the syntax remains the same. However, the detailed changes includes: 1) Kudu table will always be created with the new Kudu storage handler 'org.apache.kudu.hive.KuduStorageHandler' even when Kudu/HMS integration is disabled. The legacy storage handler will be eventually deprecated. 2) When Kudu/HMS integration is enabled, the Kudu table underneath the managed HMS table will follow the naming convention 'db_name.table_name' instead of 'impala::db_name.table_name'. 3) Add 'kudu.table_id' table property to be used with Kudu/HMS integration. This commit also extracts Kudu-related DDL parsing and analyzing tests, so that they can be run with or without Kudu/HMS integration enabled. Change-Id: I465673d749221bd5f3772814b1c22c2673a53f5c --- M bin/run-all-tests.sh M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/util/KuduUtil.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java A fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java A fe/src/test/java/org/apache/impala/analysis/AuditingKuduTest.java M fe/src/test/java/org/apache/impala/analysis/AuditingTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java A fe/src/test/java/org/apache/impala/customservice/CustomServiceRunner.java A fe/src/test/java/org/apache/impala/customservice/KuduHMSIntegrationTest.java M testdata/cluster/node_templates/common/etc/init.d/kudu-master 15 files changed, 919 insertions(+), 576 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/13318/10 -- To view, visit http://gerrit.cloudera.org:8080/13318 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I465673d749221bd5f3772814b1c22c2673a53f5c Gerrit-Change-Number: 13318 Gerrit-PatchSet: 10 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration
Hello Thomas Marshall, Alexey Serbin, Andrew Wong, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13375 to look at the new patch set (#4). Change subject: IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration .. IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration This commit supports the actual handling of CREATE TABLE DDL for managed Kudu tables when integration with Hive Metastore is enabled. When Kudu/HMS integration is enabled, for CREATE TABLE statement, Impala can rely on Kudu to create the table in the HMS. Change-Id: Icffe412395f47f5e07d97bad457020770cfa7502 --- M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java M fe/src/test/resources/hive-site.xml.py M testdata/bin/run-hive-server.sh M tests/common/custom_cluster_test_suite.py M tests/common/kudu_test_suite.py M tests/common/skip.py M tests/custom_cluster/test_kudu.py M tests/query_test/test_kudu.py 12 files changed, 266 insertions(+), 21 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/13375/4 -- To view, visit http://gerrit.cloudera.org:8080/13375 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icffe412395f47f5e07d97bad457020770cfa7502 Gerrit-Change-Number: 13375 Gerrit-PatchSet: 4 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/13318 )
Change subject: IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS
integration
..
Patch Set 8:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/13318/7/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/13318/7/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@171
PS7, Line 171: Error determining if Kudu's integration wi
> I find this awkwardly worded, maybe 'Error determining if'
Done
http://gerrit.cloudera.org:8080/#/c/13318/7/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@173
PS7, Line 173:
> unnecessary
Done
http://gerrit.cloudera.org:8080/#/c/13318/7/fe/src/test/java/org/apache/impala/analysis/AuditingTest.java
File fe/src/test/java/org/apache/impala/analysis/AuditingTest.java:
http://gerrit.cloudera.org:8080/#/c/13318/7/fe/src/test/java/org/apache/impala/analysis/AuditingTest.java@56
PS7, Line 56: new TAccessEvent("functional.alltypes",
TCatalogObjectType.TABLE, "SELECT")
> Probably better to leave off unrelated formatting changes like this. It mak
Good point, though I have to change the lines below due to they exceeds the
line length limit(90).
http://gerrit.cloudera.org:8080/#/c/13318/7/fe/src/test/java/org/apache/impala/analysis/AuditingTest.java@212
PS7, Line 212: Assert.assertEquals(accessEvents, Sets.newHashSet(
> Why this change?
Good catch, accidentally changed this.
http://gerrit.cloudera.org:8080/#/c/13318/7/fe/src/test/java/org/apache/impala/customcluster/CustomClusterRunner.java
File fe/src/test/java/org/apache/impala/customcluster/CustomClusterRunner.java:
http://gerrit.cloudera.org:8080/#/c/13318/7/fe/src/test/java/org/apache/impala/customcluster/CustomClusterRunner.java@49
PS7, Line 49:
> This should be removed
Done
http://gerrit.cloudera.org:8080/#/c/13318/7/fe/src/test/java/org/apache/impala/customservice/CustomServiceRunner.java
File fe/src/test/java/org/apache/impala/customservice/CustomServiceRunner.java:
http://gerrit.cloudera.org:8080/#/c/13318/7/fe/src/test/java/org/apache/impala/customservice/CustomServiceRunner.java@32
PS7, Line 32: minicluster
> minicluster component
Done
--
To view, visit http://gerrit.cloudera.org:8080/13318
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I465673d749221bd5f3772814b1c22c2673a53f5c
Gerrit-Change-Number: 13318
Gerrit-PatchSet: 8
Gerrit-Owner: Hao Hao
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Thomas Marshall
Gerrit-Comment-Date: Wed, 29 May 2019 21:04:13 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration
Hello Thomas Marshall, Mike Percy, Alexey Serbin, Andrew Wong, Adar Dembo, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13318 to look at the new patch set (#8). Change subject: IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration .. IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration This commit adds support for the syntax of CREATE TABLE (and CTAS) statements for managed Kudu tables with Kudu/HMS integration. A follow up patch will address the actual handling of CREATE TABLE statement with Kudu/HMS integration. For a managed table the syntax remains the same. However, the detailed changes includes: 1) Kudu table will always be created with the new Kudu storage handler 'org.apache.kudu.hive.KuduStorageHandler' even when Kudu/HMS integration is disabled. The legacy storage handler will be eventually deprecated. 2) When Kudu/HMS integration is enabled, the Kudu table underneath the managed HMS table will follow the naming convention 'db_name.table_name' instead of 'impala::db_name.table_name'. 3) Add 'kudu.table_id' table property to be used with Kudu/HMS integration. This commit also extracts Kudu-related DDL parsing and analyzing tests, so that they can be run with or without Kudu/HMS integration enabled. Change-Id: I465673d749221bd5f3772814b1c22c2673a53f5c --- M bin/run-all-tests.sh M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/util/KuduUtil.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java A fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java A fe/src/test/java/org/apache/impala/analysis/AuditingKuduTest.java M fe/src/test/java/org/apache/impala/analysis/AuditingTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java A fe/src/test/java/org/apache/impala/customservice/CustomServiceRunner.java A fe/src/test/java/org/apache/impala/customservice/KuduHMSIntegrationTest.java M testdata/cluster/node_templates/common/etc/init.d/kudu-master 15 files changed, 919 insertions(+), 576 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/13318/8 -- To view, visit http://gerrit.cloudera.org:8080/13318 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I465673d749221bd5f3772814b1c22c2673a53f5c Gerrit-Change-Number: 13318 Gerrit-PatchSet: 8 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/13318 )
Change subject: IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS
integration
..
Patch Set 7:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/13318/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/13318/4/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@192
PS4, Line 192: // TODO: Change this to reflect the number of pk columns and
modify all the
> I see. I'm fine with keeping this in if it's a hassle for you to rebase, bu
LGTM, will pull this change out to the follow up patch where it is used.
http://gerrit.cloudera.org:8080/#/c/13318/4/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@200
PS4, Line 200: tch (KuduException e) {
> SGTM. Will that be in the follow-up patch where this function is used or a
It will be in the follow-up where it is used.
http://gerrit.cloudera.org:8080/#/c/13318/6/fe/src/test/java/org/apache/impala/customservice/KuduHMSIntegrationTest.java
File
fe/src/test/java/org/apache/impala/customservice/KuduHMSIntegrationTest.java:
http://gerrit.cloudera.org:8080/#/c/13318/6/fe/src/test/java/org/apache/impala/customservice/KuduHMSIntegrationTest.java@38
PS6, Line 38: AnalyzeKuduDDLTest.class, AuditingKuduTest.class,
: ParserTest.class, ToSqlTest.class
> Do these also get run without the HMS integration enabled?
Yes, it does.
http://gerrit.cloudera.org:8080/#/c/13318/6/fe/src/test/java/org/apache/impala/customservice/KuduHMSIntegrationTest.java@60
PS6, Line 60: List envp = new ArrayList<>();
> nit: definition could be gated by enableHMSIntegration if this were put at
Done
--
To view, visit http://gerrit.cloudera.org:8080/13318
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I465673d749221bd5f3772814b1c22c2673a53f5c
Gerrit-Change-Number: 13318
Gerrit-PatchSet: 7
Gerrit-Owner: Hao Hao
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Thomas Marshall
Gerrit-Comment-Date: Wed, 29 May 2019 00:07:44 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration
Hello Thomas Marshall, Mike Percy, Alexey Serbin, Andrew Wong, Adar Dembo, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13318 to look at the new patch set (#7). Change subject: IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration .. IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration This commit adds support for the syntax of CREATE TABLE (and CTAS) statements for managed Kudu tables with Kudu/HMS integration. A follow up patch will address the actual handling of CREATE TABLE statement with Kudu/HMS integration. For a managed table the syntax remains the same. However, the detailed changes includes: 1) Kudu table will always be created with the new Kudu storage handler 'org.apache.kudu.hive.KuduStorageHandler' even when Kudu/HMS integration is disabled. The legacy storage handler will be eventually deprecated. 2) When Kudu/HMS integration is enabled, the Kudu table underneath the managed HMS table will follow the naming convention 'db_name.table_name' instead of 'impala::db_name.table_name'. 3) Add 'kudu.table_id' table property to be used with Kudu/HMS integration. This commit also extracts Kudu-related DDL parsing and analyzing tests, so that they can be run with or without Kudu/HMS integration enabled. Change-Id: I465673d749221bd5f3772814b1c22c2673a53f5c --- M bin/run-all-tests.sh M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/util/KuduUtil.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java A fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java A fe/src/test/java/org/apache/impala/analysis/AuditingKuduTest.java M fe/src/test/java/org/apache/impala/analysis/AuditingTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/customcluster/CustomClusterRunner.java A fe/src/test/java/org/apache/impala/customservice/CustomServiceRunner.java A fe/src/test/java/org/apache/impala/customservice/KuduHMSIntegrationTest.java M testdata/cluster/node_templates/common/etc/init.d/kudu-master 16 files changed, 935 insertions(+), 580 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/13318/7 -- To view, visit http://gerrit.cloudera.org:8080/13318 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I465673d749221bd5f3772814b1c22c2673a53f5c Gerrit-Change-Number: 13318 Gerrit-PatchSet: 7 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/13318 )
Change subject: IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS
integration
..
Patch Set 4:
(12 comments)
http://gerrit.cloudera.org:8080/#/c/13318/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/13318/4/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@171
PS4, Line 171: String.format("Error getting the configuration on
whether Kudu's " +
> Are these error scenarios tested?
Added a test case in AnalyzeKuduDDLTest.
http://gerrit.cloudera.org:8080/#/c/13318/4/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@188
PS4, Line 188: Check with Kudu master to see if Kudu/HMS integration is
enabled. As well
:* as validate if Kudu is configured to the given Hive
Metastore that Impala
:* is configured to use.
> nit: reword slightly "Check with the Kudu master to see if its Kudu-HMS int
Done
http://gerrit.cloudera.org:8080/#/c/13318/4/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@192
PS4, Line 192: public static boolean
isHMSIntegrationEnabledAndValidate(String kuduMasters,
> Is this used in this patch?
No, it is used in the follow up patch.
http://gerrit.cloudera.org:8080/#/c/13318/4/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@200
PS4, Line 200: hmsUris.equals(kuduHMSUris)
> Do you think it's important to check the set equality of addresses vs just
Yeah, I think it will be nice to check that, though I would prefer to add a
todo and do it in a follow up patch, to avoid putting too much stuff in this
commit.
http://gerrit.cloudera.org:8080/#/c/13318/4/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@204
PS4, Line 204: When Kudu's integration with the Hive Metastore is enabled
> nit: this reads as a general statement, rather than a statement of what exi
Done
http://gerrit.cloudera.org:8080/#/c/13318/4/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@251
PS4, Line 251: (k
> nit: extra parentheses
Done
http://gerrit.cloudera.org:8080/#/c/13318/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/13318/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2579
PS4, Line 2579: false
> nit: not sure if this is standard practice in the Impala codebase, but migh
Done
http://gerrit.cloudera.org:8080/#/c/13318/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2579
PS4, Line 2579: false
> We do that sometimes, its definitely appropriate.
Done
http://gerrit.cloudera.org:8080/#/c/13318/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java:
PS4:
> What has changed from AnalyzeDDLTest? It's a bit hard to determine since th
This is only to extract Kudu table related test. Nothing has changed except I
added a new test about 'Kudu table is not allowed to set table property
'kudu.table_id' and another negative test for getting HMS integration
configuration as you requested. I extract it to a new file because it doesn't
make sense to run non-kudu table related test twice with/without HMS
integration.
http://gerrit.cloudera.org:8080/#/c/13318/4/fe/src/test/java/org/apache/impala/analysis/AuditingKuduTest.java
File fe/src/test/java/org/apache/impala/analysis/AuditingKuduTest.java:
PS4:
> Same here: what's changed with AuditingTest here that warrants a new file?
This is only about Kudu tables and nothing has changed here. Just to run the
test with/without HMS integration.
http://gerrit.cloudera.org:8080/#/c/13318/4/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
File fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java:
http://gerrit.cloudera.org:8080/#/c/13318/4/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java@333
PS4, Line 333: String kuduMasters = catalog_.getDefaultKuduMasterHosts();
> Just making sure I understand this: the only necessary change here was the
Yes, I also update the Kudu master host address to be a valid one to not break
the test.
http://gerrit.cloudera.org:8080/#/c/13318/5/fe/src/test/java/org/apache/impala/customcluster/CustomClusterRunner.java
File fe/src/test/java/org/apache/impala/customcluster/CustomClusterRunner.java:
http://gerrit.cloudera.org:8080/#/c/13318/5/fe/src/test/java/org/apache/impala/customcluster/CustomClusterRunner.java@29
PS5, Line 29: public
> Leaving off this 'public' was intentional to reduce the chances someone wil
Sure, done.
--
To view, visit http://gerrit.cloudera.org:8080/13318
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
[Impala-ASF-CR] IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration
Hello Thomas Marshall, Mike Percy, Alexey Serbin, Andrew Wong, Adar Dembo, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13318 to look at the new patch set (#6). Change subject: IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration .. IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration This commit adds support for the syntax of CREATE TABLE (and CTAS) statements for managed Kudu tables with Kudu/HMS integration. A follow up patch will address the actual handling of CREATE TABLE statement with Kudu/HMS integration. For a managed table the syntax remains the same. However, the detailed changes includes: 1) Kudu table will always be created with the new Kudu storage handler 'org.apache.kudu.hive.KuduStorageHandler' even when Kudu/HMS integration is disabled. The legacy storage handler will be eventually deprecated. 2) When Kudu/HMS integration is enabled, the Kudu table underneath the managed HMS table will follow the naming convention 'db_name.table_name' instead of 'impala::db_name.table_name'. 3) Add 'kudu.table_id' table property to be used with Kudu/HMS integration. This commit also extracts Kudu-related DDL parsing and analyzing tests, so that they can be run with or without Kudu/HMS integration enabled. Change-Id: I465673d749221bd5f3772814b1c22c2673a53f5c --- M bin/run-all-tests.sh M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/util/KuduUtil.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java A fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java A fe/src/test/java/org/apache/impala/analysis/AuditingKuduTest.java M fe/src/test/java/org/apache/impala/analysis/AuditingTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/customcluster/CustomClusterRunner.java A fe/src/test/java/org/apache/impala/customservice/CustomServiceRunner.java A fe/src/test/java/org/apache/impala/customservice/KuduHMSIntegrationTest.java M testdata/cluster/node_templates/common/etc/init.d/kudu-master 16 files changed, 958 insertions(+), 580 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/13318/6 -- To view, visit http://gerrit.cloudera.org:8080/13318 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I465673d749221bd5f3772814b1c22c2673a53f5c Gerrit-Change-Number: 13318 Gerrit-PatchSet: 6 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-4271: Introduce the new Kudu storage handler
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13358 ) Change subject: IMPALA-4271: Introduce the new Kudu storage handler .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/13358/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13358/5//COMMIT_MSG@7 PS5, Line 7: IMPALA-8504: Introduce the new Kudu storage handler > nit: IMPALA-4271 may be a more appropriate ticket now? Done -- To view, visit http://gerrit.cloudera.org:8080/13358 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I75bcd5246005f4e35251aef9219f4d07eeb87dc6 Gerrit-Change-Number: 13358 Gerrit-PatchSet: 6 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Thu, 23 May 2019 21:08:09 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4271: Introduce the new Kudu storage handler
Hello Thomas Marshall, Alexey Serbin, Andrew Wong, Adar Dembo, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13358 to look at the new patch set (#7). Change subject: IMPALA-4271: Introduce the new Kudu storage handler .. IMPALA-4271: Introduce the new Kudu storage handler This commit introduces the new Kudu storage handler 'org.apache.kudu.hive.KuduStorageHandler' that is used in Kudu/HMS integration. Tables with either the legacy storage handler 'com.cloudera.kudu.hive.KuduStorageHandler' or the new one are now considered as Kudu tables. The actual work of adopting the new storage handler will be addressed in a series of follow-up patches, which rely on it to support Kudu/HMS integration in Impala. Change-Id: I75bcd5246005f4e35251aef9219f4d07eeb87dc6 --- M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java 2 files changed, 15 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/13358/7 -- To view, visit http://gerrit.cloudera.org:8080/13358 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I75bcd5246005f4e35251aef9219f4d07eeb87dc6 Gerrit-Change-Number: 13358 Gerrit-PatchSet: 7 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration
Hello Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13400 to look at the new patch set (#2). Change subject: IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration .. IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration This commit supports the actual handling of DROP TABLE DDL for managed Kudu tables with Kudu's integration with the Hive Metastore. When Kudu/HMS integration is enabled, after the table is dropped in the Kudu, relies on Kudu to drop the table in the HMS. WIP: Note that the tests need the latest Kudu bits to pass. Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc --- M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M tests/custom_cluster/test_kudu.py 2 files changed, 77 insertions(+), 25 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/13400/2 -- To view, visit http://gerrit.cloudera.org:8080/13400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc Gerrit-Change-Number: 13400 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration
Hao Hao has removed Adar Dembo from this change. ( http://gerrit.cloudera.org:8080/13400 ) Change subject: IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration .. Removed reviewer Adar Dembo. -- To view, visit http://gerrit.cloudera.org:8080/13400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc Gerrit-Change-Number: 13400 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration
Hello Thomas Marshall, Mike Percy, Alexey Serbin, Andrew Wong, Adar Dembo, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13318 to look at the new patch set (#5). Change subject: IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration .. IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration This commit adds support for the syntax of CREATE TABLE (and CTAS) statements for managed Kudu tables with Kudu/HMS integration. A follow up patch will address the actual handling of CREATE TABLE statement with Kudu/HMS integration. For a managed table the syntax remains the same. However, the detailed changes includes: 1) Kudu table will always be created with the new Kudu storage handler 'org.apache.kudu.hive.KuduStorageHandler' even when Kudu/HMS integration is disabled. The legacy storage handler will be eventually deprecated. 2) When Kudu/HMS integration is enabled, the Kudu table underneath the managed HMS table will follow the naming convention 'db_name.table_name' instead of 'impala::db_name.table_name'. 3) Add 'kudu.table_id' table property to be used with Kudu/HMS integration. This commit also extracts Kudu-related DDL parsing and analyzing tests, so that they can be run with or without Kudu/HMS integration enabled. Change-Id: I465673d749221bd5f3772814b1c22c2673a53f5c --- M bin/run-all-tests.sh M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/util/KuduUtil.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java A fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java A fe/src/test/java/org/apache/impala/analysis/AuditingKuduTest.java M fe/src/test/java/org/apache/impala/analysis/AuditingTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/customcluster/CustomClusterRunner.java A fe/src/test/java/org/apache/impala/customservice/KuduHMSIntegrationTest.java M testdata/cluster/node_templates/common/etc/init.d/kudu-master 15 files changed, 906 insertions(+), 581 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/13318/5 -- To view, visit http://gerrit.cloudera.org:8080/13318 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I465673d749221bd5f3772814b1c22c2673a53f5c Gerrit-Change-Number: 13318 Gerrit-PatchSet: 5 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8504: Introduce the new Kudu storage handler
Hello Thomas Marshall, Alexey Serbin, Andrew Wong, Adar Dembo, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13358 to look at the new patch set (#6). Change subject: IMPALA-8504: Introduce the new Kudu storage handler .. IMPALA-8504: Introduce the new Kudu storage handler This commit introduces the new Kudu storage handler 'org.apache.kudu.hive.KuduStorageHandler' that is used in Kudu/HMS integration. Tables with either the legacy storage handler 'com.cloudera.kudu.hive.KuduStorageHandler' or the new one are now considered as Kudu tables. The actual work of adopting the new storage handler will be addressed in a series of follow-up patches, which rely on it to support Kudu/HMS integration in Impala. Change-Id: I75bcd5246005f4e35251aef9219f4d07eeb87dc6 --- M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java 2 files changed, 15 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/13358/6 -- To view, visit http://gerrit.cloudera.org:8080/13358 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I75bcd5246005f4e35251aef9219f4d07eeb87dc6 Gerrit-Change-Number: 13358 Gerrit-PatchSet: 6 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS integration
Hao Hao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13409 Change subject: IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS integration .. IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS integration This commit intends to support the actual handling of ALTER/RENAME TABLE DDL for managed Kudu tables with Kudu's integration with the Hive Metastore. However, currently Kudu is considered as the source of truth of the table schema, so when ALTER TABLE (ADD/DROP COLUMN/RANGE_PARTITION), Impala always directly alters/loads the Kudu tables. Thus, this commit only updates RENAME TABLE DDL, so that after the table is renamed in the Kudu, relies on Kudu to rename the table in the HMS. WIP: Note that the tests need the latest Kudu bits to pass. Change-Id: If7155e0b385b8ad81eda0a84277bc85171a88269 --- M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java A testdata/workloads/functional-query/queries/QueryTest/kudu_hms_alter.test M tests/custom_cluster/test_kudu.py 3 files changed, 665 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/13409/1 -- To view, visit http://gerrit.cloudera.org:8080/13409 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: If7155e0b385b8ad81eda0a84277bc85171a88269 Gerrit-Change-Number: 13409 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao
[Impala-ASF-CR] IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration
Hello Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13375 to look at the new patch set (#3). Change subject: IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration .. IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration This commit supports the actual handling of CREATE TABLE DDL for managed Kudu tables with Kudu's integration with the Hive Metastore. When Kudu/HMS integration is enabled, after the table is created in the Kudu, relies on Kudu to create the table in the HMS. Change-Id: Icffe412395f47f5e07d97bad457020770cfa7502 --- M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java M fe/src/test/resources/hive-site.xml.py M testdata/bin/run-hive-server.sh M tests/common/custom_cluster_test_suite.py M tests/common/kudu_test_suite.py M tests/common/skip.py M tests/custom_cluster/test_kudu.py M tests/query_test/test_kudu.py 12 files changed, 183 insertions(+), 15 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/13375/3 -- To view, visit http://gerrit.cloudera.org:8080/13375 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icffe412395f47f5e07d97bad457020770cfa7502 Gerrit-Change-Number: 13375 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] (WIP) IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration
Hao Hao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13400 Change subject: (WIP) IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration .. (WIP) IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration This commit supports the actual handling of DROP TABLE DDL for managed Kudu tables with Kudu's integration with the Hive Metastore. When Kudu/HMS integration is enabled, after the table is dropped in the Kudu, relies on Kudu to drop the table in the HMS. Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc --- M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M tests/custom_cluster/test_kudu.py 2 files changed, 78 insertions(+), 25 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/13400/1 -- To view, visit http://gerrit.cloudera.org:8080/13400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc Gerrit-Change-Number: 13400 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao
[Impala-ASF-CR] IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13375 ) Change subject: IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/13375/1/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java: http://gerrit.cloudera.org:8080/#/c/13375/1/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java@102 PS1, Line 102: String tableId = table.getTableId(); > You could probably always set the table id for managed tables, even if HMS I think we should avoid that, as without HMS integration enabled, it is hard to tell if the table id is still correct. -- To view, visit http://gerrit.cloudera.org:8080/13375 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icffe412395f47f5e07d97bad457020770cfa7502 Gerrit-Change-Number: 13375 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 22 May 2019 02:51:45 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration
Hello Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13375 to look at the new patch set (#2). Change subject: IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration .. IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration This commit supports the actual handling of CREATE TABLE DDL for managed Kudu tables with Kudu's integration with the Hive Metastore. When Kudu/HMS integration is enabled, after the table is created in the Kudu, relies on Kudu to create the table in the HMS. Change-Id: Icffe412395f47f5e07d97bad457020770cfa7502 --- M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java M fe/src/test/resources/hive-site.xml.py M testdata/bin/run-hive-server.sh M tests/common/custom_cluster_test_suite.py M tests/common/kudu_test_suite.py M tests/common/skip.py M tests/custom_cluster/test_kudu.py M tests/query_test/test_kudu.py 11 files changed, 205 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/13375/2 -- To view, visit http://gerrit.cloudera.org:8080/13375 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icffe412395f47f5e07d97bad457020770cfa7502 Gerrit-Change-Number: 13375 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/13318 )
Change subject: IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS
integration
..
Patch Set 4:
(17 comments)
http://gerrit.cloudera.org:8080/#/c/13318/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13318/3//COMMIT_MSG@9
PS3, Line 9: commit
> commit
Done
http://gerrit.cloudera.org:8080/#/c/13318/3//COMMIT_MSG@9
PS3, Line 9: dds sup
> adds support for
Done
http://gerrit.cloudera.org:8080/#/c/13318/3//COMMIT_MSG@11
PS3, Line 11: l addr
> actual
Done
http://gerrit.cloudera.org:8080/#/c/13318/3//COMMIT_MSG@13
PS3, Line 13:
> a managed
Done
http://gerrit.cloudera.org:8080/#/c/13318/3//COMMIT_MSG@19
PS3, Line 19: ion is enabled, the Kudu table underneath the
:managed HMS table will follow
> BTW, what happens with the name of the table created in Kudu when the integ
You mean a Kudu table is created with HMS integration enabled and later the
integration is disabled? Yeah, that should be handled by the tool.
http://gerrit.cloudera.org:8080/#/c/13318/3//COMMIT_MSG@23
PS3, Line 23:
> nit: Kudu-related
Done
http://gerrit.cloudera.org:8080/#/c/13318/3//COMMIT_MSG@23
PS3, Line 23:
> commit
Done
http://gerrit.cloudera.org:8080/#/c/13318/3/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
File fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java:
http://gerrit.cloudera.org:8080/#/c/13318/3/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@329
PS3, Line 329: u t
> nit: drop?
Done
http://gerrit.cloudera.org:8080/#/c/13318/3/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@329
PS3, Line 329: a manag
> nit: a managed Kudu
Done
http://gerrit.cloudera.org:8080/#/c/13318/3/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/13318/3/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@192
PS3, Line 192: public static boolean
isHMSIntegrationEnabledAndValidate(String kuduMasters,
> Where is this used?
This will be used when the DDL actual handling. And I don't see there is a need
to expose HiveMetastoreConfig so far. As its content is only used for
validation HMS Uris so far.
http://gerrit.cloudera.org:8080/#/c/13318/3/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@194
PS3, Line 194: HiveMetastoreConfig hmsConfig =
getHiveMetastoreConfig(kuduMasters);
> hmsConfig could be null here.
Done
http://gerrit.cloudera.org:8080/#/c/13318/3/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java:
http://gerrit.cloudera.org:8080/#/c/13318/3/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java@440
PS3, Line 440: .KuduStorageHa
> Is it possible to create a Kudu table with STORED AS KUDU syntax and simult
It is possible for appropriate storage handler, added such a test. For non
appropriate is covered below.
http://gerrit.cloudera.org:8080/#/c/13318/3/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java@485
PS3, Line 485: g.format("Tab
> nit: a Kudu table (should be fixed in corresponding place)
Done
http://gerrit.cloudera.org:8080/#/c/13318/3/fe/src/test/java/org/apache/impala/analysis/AuditingKuduTest.java
File fe/src/test/java/org/apache/impala/analysis/AuditingKuduTest.java:
http://gerrit.cloudera.org:8080/#/c/13318/3/fe/src/test/java/org/apache/impala/analysis/AuditingKuduTest.java@1
PS3, Line 1: // Licensed to the Apache Software Foundation (ASF) under one
> License header
Done
http://gerrit.cloudera.org:8080/#/c/13318/3/fe/src/test/java/org/apache/impala/analysis/AuditingKuduTest.java@22
PS3, Line 22: il.Set;
> What about ALTER TABLE? If generating auditing events for ALTER is not sup
This is extracted from AuditingTest class which has coverage for ALTER
statements. My understanding is this is for Kudu specific/only DDLs/DMLs.
http://gerrit.cloudera.org:8080/#/c/13318/3/fe/src/test/java/org/apache/impala/analysis/AuditingKuduTest.java@24
PS3, Line 24: import org.apache.impala.authorization.AuthorizationException;
> What about CTAS statement? Do we expect both CREATE and SELECT audit event
Yeah, added a test in AuditingTest.
http://gerrit.cloudera.org:8080/#/c/13318/3/fe/src/test/java/org/apache/impala/analysis/AuditingKuduTest.java@81
PS3, Line 81: Assert.assertEquals(accessEvents, Sets.newHashSet(
: new TAccessEvent("functional_kudu.testtbl",
: TCatalogObjectType.TABLE, "SELECT"),
: new TAccessEvent("functional_kudu.alltypes",
> What about 'DROP TABLE ... IF EXISTS'?
Done
--
To view, visit http://gerrit.cloudera.org:8080/13318
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch:
[Impala-ASF-CR] IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration
Hello Thomas Marshall, Mike Percy, Alexey Serbin, Andrew Wong, Adar Dembo, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13318 to look at the new patch set (#4). Change subject: IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration .. IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration This commit adds support for the syntax of CREATE TABLE (and CTAS) statements for managed Kudu tables with Kudu/HMS integration. A follow up patch will address the actual handling of CREATE TABLE statement with Kudu/HMS integration. For a managed table the syntax remains the same. However, the detailed changes includes: 1) Kudu table will always be created with the new Kudu storage handler 'org.apache.kudu.hive.KuduStorageHandler' even when Kudu/HMS integration is disabled. The legacy storage handler will be eventually deprecated. 2) When Kudu/HMS integration is enabled, the Kudu table underneath the managed HMS table will follow the naming convention 'db_name.table_name' instead of 'impala::db_name.table_name'. 3) Add 'kudu.table_id' table property to be used with Kudu/HMS integration. This commit also extracts Kudu-related DDL parsing and analyzing tests, so that they can be run with or without Kudu/HMS integration enabled. Change-Id: I465673d749221bd5f3772814b1c22c2673a53f5c --- M bin/run-all-tests.sh M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/util/KuduUtil.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java A fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java A fe/src/test/java/org/apache/impala/analysis/AuditingKuduTest.java M fe/src/test/java/org/apache/impala/analysis/AuditingTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/customcluster/CustomClusterRunner.java A fe/src/test/java/org/apache/impala/customservice/KuduHMSIntegrationTest.java M testdata/cluster/node_templates/common/etc/init.d/kudu-master 15 files changed, 906 insertions(+), 581 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/13318/4 -- To view, visit http://gerrit.cloudera.org:8080/13318 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I465673d749221bd5f3772814b1c22c2673a53f5c Gerrit-Change-Number: 13318 Gerrit-PatchSet: 4 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration
Hao Hao has abandoned this change. ( http://gerrit.cloudera.org:8080/13374 ) Change subject: IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration .. Abandoned -- To view, visit http://gerrit.cloudera.org:8080/13374 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: Ib02e46109bec4180b037452fd6a6ab8054e7b13f Gerrit-Change-Number: 13374 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-8503: allow the Hive Metastore to start with kudu-hive plugin
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13319 ) Change subject: IMPALA-8503: allow the Hive Metastore to start with kudu-hive plugin .. Patch Set 1: > While this is of course fine for you to use for local testing of > your other changes for now, I don't think this is something that we > want to review and commit. > > Seems better to just wait until kudu-hive.jar is available through > the normal mechanisms, which shouldn't take very long (unless > there's an issue I don't know about) Sounds good, this is intended to be a transient patch. I will update it once the artifact is available. -- To view, visit http://gerrit.cloudera.org:8080/13319 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4bd1488ced51840ec986d29ed371e26168abcc76 Gerrit-Change-Number: 13319 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Mon, 20 May 2019 06:15:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/13318 )
Change subject: IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS
integration
..
Patch Set 3:
(15 comments)
http://gerrit.cloudera.org:8080/#/c/13318/2/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java:
http://gerrit.cloudera.org:8080/#/c/13318/2/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java@266
PS2, Line 266: throw new AnalysisException("Invalid storage handler
specified for Kudu table: " +
> Use !KuduTable.isKuduStorageHandler(...) here.
Done
http://gerrit.cloudera.org:8080/#/c/13318/2/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/13318/2/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@57
PS2, Line 57: // Alias to the string key that identifies the storage handler
for Kudu tables.
> This isn't used anywhere
Done
http://gerrit.cloudera.org:8080/#/c/13318/2/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@167
PS2, Line 167: try {
> How difficult would this be to implement and what's the failure mode if its
I removed the TODO and updated the actual validation.
http://gerrit.cloudera.org:8080/#/c/13318/2/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@220
PS2, Line 220: KuduClient kuduClient =
KuduUtil.getKuduClient(getKuduMasterHosts());
> I think this can be reverted to be the KEY_TABLE_NAME and the error message
Done
http://gerrit.cloudera.org:8080/#/c/13318/2/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@227
PS2, Line 227: }
> I don't think this comment is relevant anymore, this isn't generating a nam
Done
http://gerrit.cloudera.org:8080/#/c/13318/2/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@229
PS2, Line 229:
> See my comment about about always using kuduTableName_ from KEY_TABLE_NAME.
Done
http://gerrit.cloudera.org:8080/#/c/13318/2/fe/src/main/java/org/apache/impala/catalog/local/LocalKuduTable.java
File fe/src/main/java/org/apache/impala/catalog/local/LocalKuduTable.java:
http://gerrit.cloudera.org:8080/#/c/13318/2/fe/src/main/java/org/apache/impala/catalog/local/LocalKuduTable.java@a193
PS2, Line 193:
> Why did this need to change?
Reverted it back.
http://gerrit.cloudera.org:8080/#/c/13318/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/13318/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2579
PS2, Line 2579: newTableName.getDb(), newTableName.getTbl(), false);
> Don't we need to check if HMS is enabled?
Yes, this will be a follow up change.
http://gerrit.cloudera.org:8080/#/c/13318/2/fe/src/main/java/org/apache/impala/util/KuduUtil.java
File fe/src/main/java/org/apache/impala/util/KuduUtil.java:
http://gerrit.cloudera.org:8080/#/c/13318/2/fe/src/main/java/org/apache/impala/util/KuduUtil.java@387
PS2, Line 387: me.metastoreTableName'. This
:* should only b
> What does this mean?
I think this refers to external table which the Kudu table name is provided by
the users.
http://gerrit.cloudera.org:8080/#/c/13318/2/fe/src/main/java/org/apache/impala/util/KuduUtil.java@390
PS2, Line 390: String metastoreDbName,
: String metastoreTableName, boolean
isHMSIntegrationEnabled) {
: return isHMSIntegrationEnabled ? metastoreDbName + "." +
metastoreTableNam
> Please try to match the formatting in the rest of the file, here and elsewh
Done
http://gerrit.cloudera.org:8080/#/c/13318/2/fe/src/main/java/org/apache/impala/util/KuduUtil.java@397
PS2, Line 397:* Check if the given name is the default Kudu table name for
managed table
> brief comment
Done
http://gerrit.cloudera.org:8080/#/c/13318/2/fe/src/test/java/org/apache/impala/analysis/AuditingTest.java
File fe/src/test/java/org/apache/impala/analysis/AuditingTest.java:
http://gerrit.cloudera.org:8080/#/c/13318/2/fe/src/test/java/org/apache/impala/analysis/AuditingTest.java@65
PS2, Line 65: "SELECT"),
> line too long (93 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/13318/2/fe/src/test/java/org/apache/impala/analysis/KuduHMSIntegrationTest.java
File fe/src/test/java/org/apache/impala/analysis/KuduHMSIntegrationTest.java:
http://gerrit.cloudera.org:8080/#/c/13318/2/fe/src/test/java/org/apache/impala/analysis/KuduHMSIntegrationTest.java@39
PS2, Line 39:
> This makes me nervous. We don't currently have any tests that restart any m
Thanks for the suggestion!
http://gerrit.cloudera.org:8080/#/c/13318/2/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
File fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java:
[Impala-ASF-CR] IMPALA-8503: add option to start Kudu cluster with HMS integration
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13248 ) Change subject: IMPALA-8503: add option to start Kudu cluster with HMS integration .. Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/13248/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13248/3//COMMIT_MSG@7 PS3, Line 7: IMPALA-8503: add option to start Kudu cluster with HMS integration > Like I said before, its difficult to evaluate this patch without seeing how Sure, I can move it to the CREATE TABLE patch. My initial intention is to avoid large patch, and want to separate the patches based on the functionality. http://gerrit.cloudera.org:8080/#/c/13248/3/testdata/cluster/node_templates/common/etc/init.d/kudu-master File testdata/cluster/node_templates/common/etc/init.d/kudu-master: http://gerrit.cloudera.org:8080/#/c/13248/3/testdata/cluster/node_templates/common/etc/init.d/kudu-master@28 PS3, Line 28: if [[ -n "$ENABLE_KUDU_HMS_INTEGRATION" ]]; then > I think its better if this is more generic, eg. call the env variable IMPAL Done -- To view, visit http://gerrit.cloudera.org:8080/13248 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I734d14ede6a03ad52e820e38a1fbcbac0a40ede2 Gerrit-Change-Number: 13248 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Mon, 20 May 2019 06:28:29 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8504: Introduce the new Kudu storage handler
Hello Alexey Serbin, Andrew Wong, Adar Dembo, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13358 to look at the new patch set (#4). Change subject: IMPALA-8504: Introduce the new Kudu storage handler .. IMPALA-8504: Introduce the new Kudu storage handler This commit introduces the new Kudu storage handler 'org.apache.kudu.hive.KuduStorageHandler' that is used in Kudu/HMS integration. Tables with either the legacy storage handler 'com.cloudera.kudu.hive.KuduStorageHandler' or the new one are now considered as Kudu tables. The actual work of adopting the new storage handler will be addressed in a series of follow-up patches, which rely on it to support Kudu/HMS integration in Impala. Change-Id: I75bcd5246005f4e35251aef9219f4d07eeb87dc6 --- M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java 2 files changed, 15 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/13358/4 -- To view, visit http://gerrit.cloudera.org:8080/13358 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I75bcd5246005f4e35251aef9219f4d07eeb87dc6 Gerrit-Change-Number: 13358 Gerrit-PatchSet: 4 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration
Hello Thomas Marshall, Mike Percy, Alexey Serbin, Andrew Wong, Adar Dembo, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13318 to look at the new patch set (#3). Change subject: IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration .. IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration This commits support the syntax of CREATE TABLE (and CTAS) statements for managed Kudu tables with Kudu/HMS integration. A follow up patch will address the actaul handling of CREATE TABLE statement with Kudu/HMS integration. For managed table the syntax remains the same. However, the detailed changes includes: 1) Kudu table will always be created with the new Kudu storage handler 'org.apache.kudu.hive.KuduStorageHandler' even when Kudu/HMS integration is disabled. The legacy storage handler will be eventually deprecated. 2) When Kudu/HMS integration is enabled, the Kudu table underneath the managed HMS table will follow the naming convention 'db_name.table_name' instead of 'impala::db_name.table_name'. 3) Add 'kudu.table_id' table property to be used with Kudu/HMS integration. This commits also extracts Kudu related DDL parsing and analyzing tests, so that they can be run with or without Kudu/HMS integration enabled. Change-Id: I465673d749221bd5f3772814b1c22c2673a53f5c --- M bin/run-all-tests.sh M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/util/KuduUtil.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java A fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java A fe/src/test/java/org/apache/impala/analysis/AuditingKuduTest.java M fe/src/test/java/org/apache/impala/analysis/AuditingTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/customcluster/CustomClusterRunner.java A fe/src/test/java/org/apache/impala/customservice/KuduHMSIntegrationTest.java M testdata/cluster/node_templates/common/etc/init.d/kudu-master 15 files changed, 873 insertions(+), 578 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/13318/3 -- To view, visit http://gerrit.cloudera.org:8080/13318 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I465673d749221bd5f3772814b1c22c2673a53f5c Gerrit-Change-Number: 13318 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration
Hao Hao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13374 Change subject: IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration .. IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration This commits support the syntax of CREATE TABLE (and CTAS) statements for managed Kudu tables with Kudu/HMS integration. A follow up patch will address the actaul handling of CREATE TABLE statement with Kudu/HMS integration. For managed table the syntax remains the same. However, the detailed changes includes: 1) Kudu table will always be created with the new Kudu storage handler 'org.apache.kudu.hive.KuduStorageHandler' even when Kudu/HMS integration is disabled. The legacy storage handler will be eventually deprecated. 2) When Kudu/HMS integration is enabled, the Kudu table underneath the managed HMS table will follow the naming convention 'db_name.table_name' instead of 'impala::db_name.table_name'. 3) Add 'kudu.table_id' table property to be used with Kudu/HMS integration. This commits also extracts Kudu related DDL parsing and analyzing tests, so that they can be run with or without Kudu/HMS integration enabled. Change-Id: Ib02e46109bec4180b037452fd6a6ab8054e7b13f --- M bin/run-all-tests.sh M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/util/KuduUtil.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java A fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java A fe/src/test/java/org/apache/impala/analysis/AuditingKuduTest.java M fe/src/test/java/org/apache/impala/analysis/AuditingTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/customcluster/CustomClusterRunner.java A fe/src/test/java/org/apache/impala/customservice/KuduHMSIntegrationTest.java M testdata/cluster/node_templates/common/etc/init.d/kudu-master 15 files changed, 873 insertions(+), 578 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/13374/1 -- To view, visit http://gerrit.cloudera.org:8080/13374 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ib02e46109bec4180b037452fd6a6ab8054e7b13f Gerrit-Change-Number: 13374 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao
[Impala-ASF-CR] (WIP) IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration
Hao Hao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13375 Change subject: (WIP) IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration .. (WIP) IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration This commit supports the actual handling of CREATE TABLE DDL for managed Kudu tables with Kudu's integration with the Hive Metastore. When Kudu/HMS integration is enabled, after the table is created in the Kudu, relies on Kudu to create the table in the HMS. Change-Id: Icffe412395f47f5e07d97bad457020770cfa7502 --- M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java M fe/src/test/resources/hive-site.xml.py M testdata/bin/run-hive-server.sh A testdata/workloads/functional-query/queries/QueryTest/kudu_hms_create M tests/common/custom_cluster_test_suite.py M tests/common/skip.py M tests/custom_cluster/test_kudu.py M tests/query_test/test_kudu.py 10 files changed, 163 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/13375/1 -- To view, visit http://gerrit.cloudera.org:8080/13375 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Icffe412395f47f5e07d97bad457020770cfa7502 Gerrit-Change-Number: 13375 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao
[Impala-ASF-CR] IMPALA-8504: Introduce the new Kudu storage handler
Hello Alexey Serbin, Andrew Wong, Adar Dembo, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13358 to look at the new patch set (#2). Change subject: IMPALA-8504: Introduce the new Kudu storage handler .. IMPALA-8504: Introduce the new Kudu storage handler This commit introduces the new Kudu storage handler 'org.apache.kudu.hive.KuduStorageHandler' that is used in Kudu/HMS integration. Tables with either the legacy storage handler 'com.cloudera.kudu.hive.KuduStorageHandler' or the new one are now considered as Kudu tables. The actual work of adopting the new storage handler will be addressed in a series of follow-up patches, which rely on it to support Kudu/HMS integration in Impala. Change-Id: I75bcd5246005f4e35251aef9219f4d07eeb87dc6 --- M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java 2 files changed, 15 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/13358/2 -- To view, visit http://gerrit.cloudera.org:8080/13358 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I75bcd5246005f4e35251aef9219f4d07eeb87dc6 Gerrit-Change-Number: 13358 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-8504: Introduce the new Kudu storage handler
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/13358 )
Change subject: IMPALA-8504: Introduce the new Kudu storage handler
..
Patch Set 2:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/13358/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13358/1//COMMIT_MSG@9
PS1, Line 9: commit
> commit
Done
http://gerrit.cloudera.org:8080/#/c/13358/1//COMMIT_MSG@13
PS1, Line 13: st
> drop
Done
http://gerrit.cloudera.org:8080/#/c/13358/1/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java:
http://gerrit.cloudera.org:8080/#/c/13358/1/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java@222
PS1, Line 222: if (KuduTable.KUDU_LEGACY_STORAGE_HANDLER.equals(
> Should this check if it's either format? Or just the new format since the o
This is intentionally to only check the legacy format as this commit is only
introducing the new storage handler. And there are a series of follow-up
patches will actual use this storage handler. The motivation is to avoid giant
patch to review and also hopefully to unblock other's testing work which rely
on use new storage handler to identify Kudu tables.
http://gerrit.cloudera.org:8080/#/c/13358/1/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java@222
PS1, Line 222: if (KuduTable.KUDU_LEGACY_STORAGE_HANDLER.equals(
> I'm also curious why new storage handler is not accounted for in this logic
Yeah, this is intentional, as explained above. I will add more detailed
explanation in the commit msg.
http://gerrit.cloudera.org:8080/#/c/13358/1/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java@265
PS1, Line 265: if (handler != null &&
!handler.equals(KuduTable.KUDU_LEGACY_STORAGE_HANDLER)) {
> Same here.
Same as above comment.
http://gerrit.cloudera.org:8080/#/c/13358/1/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java@270
PS1, Line 270: KuduTable.KUDU_LEGACY_STORAGE_HANDLER);
> Should this be the new format since the old format won't be used to create
Same as above.
http://gerrit.cloudera.org:8080/#/c/13358/1/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/13358/1/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@74
PS1, Line 74: // KEY_STORAGE_HANDLER. This is expected to be deprecated
eventually.
> Is this comment still true? I think we will use KUDU_STORAGE_HANDLER for al
Good catch, you're right.
http://gerrit.cloudera.org:8080/#/c/13358/1/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@79
PS1, Line 79: // KEY_STORAGE_HANDLER.
> We also don't need to mention HMS integration here.
Done
--
To view, visit http://gerrit.cloudera.org:8080/13358
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I75bcd5246005f4e35251aef9219f4d07eeb87dc6
Gerrit-Change-Number: 13358
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Fri, 17 May 2019 23:28:29 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8503: add option to start Kudu cluster with HMS integration
Hello Thomas Marshall, Alexey Serbin, Andrew Wong, Adar Dembo, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13248 to look at the new patch set (#3). Change subject: IMPALA-8503: add option to start Kudu cluster with HMS integration .. IMPALA-8503: add option to start Kudu cluster with HMS integration Currently static template configuration under testdata/cluster/ is used to control Kudu gflags when starting a Kudu cluster. An option to allow custom configuration such as enabling HMS integration is needed to allow tests to run with Kudu clusters with different set of configurations. This commit introduces a new environment variable 'ENABLE_KUDU_HMS_INTEGRATION' to allow starting Kudu master with HMS integration enabled when this variable is set. Change-Id: I734d14ede6a03ad52e820e38a1fbcbac0a40ede2 --- M testdata/cluster/node_templates/common/etc/init.d/kudu-master 1 file changed, 4 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/48/13248/3 -- To view, visit http://gerrit.cloudera.org:8080/13248 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I734d14ede6a03ad52e820e38a1fbcbac0a40ede2 Gerrit-Change-Number: 13248 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/13318 )
Change subject: IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS
integration
..
Patch Set 1:
(56 comments)
http://gerrit.cloudera.org:8080/#/c/13318/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13318/1//COMMIT_MSG@7
PS1, Line 7: IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS
integration
> I was under the impression there would need to be special coordination betw
Yeah, the handling part will be in a follow up patch.
http://gerrit.cloudera.org:8080/#/c/13318/1//COMMIT_MSG@9
PS1, Line 9: CATS
> CTAS - Create Table As Select?
Done
http://gerrit.cloudera.org:8080/#/c/13318/1//COMMIT_MSG@9
PS1, Line 9: statments
> statements
Done
http://gerrit.cloudera.org:8080/#/c/13318/1//COMMIT_MSG@13
PS1, Line 13: CREATE EXTERNAL TABLE t STORED AS KUDU TBLPROPERTIES
> How is this different then it was before? Why does it need to be different?
I updated the patch to be only about managed table, due to the in flight
discussion.
http://gerrit.cloudera.org:8080/#/c/13318/1//COMMIT_MSG@21
PS1, Line 21: 2) When Kudu/HMS integration is enabled, the external table is no
longer
> I think this discussion is still in flight with Kudu devs.
Yes, I updated the patch to be only about managed table, due to the in flight
discussion.
http://gerrit.cloudera.org:8080/#/c/13318/1/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java:
http://gerrit.cloudera.org:8080/#/c/13318/1/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java@255
PS1, Line 255: private void analyzeKuduTableProperties(Analyzer analyzer,
boolean isHMSIntegrationEnabled)
> line too long (93 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/13318/1/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java@270
PS1, Line 270: // TODO(hao): remove ALL privileges on SERVER validation
when HMS Integration is enabled.
> line too long (95 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/13318/1/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java@285
PS1, Line 285: (!isHMSIntegrationEnabled &&
!handler.equals(KuduTable.KUDU_LEGACY_STORAGE_HANDLER {
> line too long (97 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/13318/1/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java@304
PS1, Line 304: String.format("Table property %s should not be specified
when creating an Kudu table.",
> line too long (95 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/13318/1/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java@309
PS1, Line 309:* Populates Kudu master addresses either from table property
or the -kudu_master_hosts flag.
> line too long (96 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/13318/1/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java@322
PS1, Line 322: private void analyzeExternalKuduTableParams(Analyzer analyzer,
boolean isHMSIntegrationEnabled)
> line too long (97 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/13318/1/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java@353
PS1, Line 353: private void analyzeManagedKuduTableParams(Analyzer analyzer,
boolean isHMSIntegrationEnabled)
> line too long (96 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/13318/1/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
File fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java:
http://gerrit.cloudera.org:8080/#/c/13318/1/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@329
PS1, Line 329: // When Kudu/HMS integration is not enabled, remove hidden
table property
> What about legacy tables?
Yeah, I think here is to cover legacy tables with Kudu, to remove TABLE_NAME
property with managed table.
http://gerrit.cloudera.org:8080/#/c/13318/1/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/13318/1/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@130
PS1, Line 130: public String getStorageHandlerClassName() {
> Should this return the handler from the HMS instead of generating one based
Actually I think I will just use KUDU_STORAGE_HANDLER here for two reasons. 1)
Going forward we are deprecating KUDU_LEGACY_STORAGE_HANDLER no matter HMS
integration is enabled. 2) This is only used at ToSqlUtils
https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java#L317,
which is just to set to null if it is a KuduTable
(https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java#L326).
But please correct me if my understanding is wrong.
[Impala-ASF-CR] IMPALA-8503: add option to start Kudu cluster with HMS integration
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/13248 )
Change subject: IMPALA-8503: add option to start Kudu cluster with HMS
integration
..
Patch Set 3:
(4 comments)
> (6 comments)
>
> I don't have enough context for high-level review of this
> changelist, but it seems the comment that Thomas added makes sense.
>
> Also, what is the way to configure the way how services started via
> these scripts? I know that in case of UNIX init.d scripts there
> are configuration files in /etc that are picked up by the scripts.
> Using environment variables makes it look like something similar to
> that approach.
Ok, since both of you think using env is better, I updated the patch
accordingly. Thanks!
http://gerrit.cloudera.org:8080/#/c/13248/1/testdata/cluster/admin
File testdata/cluster/admin:
http://gerrit.cloudera.org:8080/#/c/13248/1/testdata/cluster/admin@425
PS1, Line 425:
> drop
Done
http://gerrit.cloudera.org:8080/#/c/13248/1/testdata/cluster/admin@427
PS1, Line 427:
> drop
Done
http://gerrit.cloudera.org:8080/#/c/13248/1/testdata/cluster/admin@428
PS1, Line 428: M
> drop
Done
http://gerrit.cloudera.org:8080/#/c/13248/1/testdata/cluster/admin@497
PS1, Line 497: function restart {
: if [[ $# -ne 1 ]]; then
: echo restart must be called with a single argument -- the
service to restart. 1>&2
: exit 1
: fi
: local SERVICE=$1
: stop $SERVICE
: start $SERVICE
: }
:
: function delete_data {
: # Delete namenode, datanode and KMS data while preserving
directory structure.
: rm -rf "$NODES_DIR/$NODE_PREFIX"*/data/dfs/{nn,dn}/*
: rm -f "$NODES_DIR/$NODE_PREFIX"*/data/kms.keystore
: delete_kudu_data
: }
> Did you verify this works as expected? I'm a bit confused about check at L
Yeah, I verified. What is the confusing point?
--
To view, visit http://gerrit.cloudera.org:8080/13248
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I734d14ede6a03ad52e820e38a1fbcbac0a40ede2
Gerrit-Change-Number: 13248
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Marshall
Gerrit-Comment-Date: Fri, 17 May 2019 05:57:32 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration
Hello Thomas Marshall, Mike Percy, Alexey Serbin, Andrew Wong, Adar Dembo, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13318 to look at the new patch set (#2). Change subject: IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration .. IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration This commits support the syntax of CREATE TABLE (and CTAS) statements for managed Kudu tables with Kudu/HMS integration. A follow up patch will address the actaul handling of CREATE TABLE statement with Kudu/HMS integration. For managed table the syntax remains the same. However, the detailed changes includes: 1) Kudu table will always be created with the new Kudu storage handler 'org.apache.kudu.hive.KuduStorageHandler' even when Kudu/HMS integration is disabled. The legacy storage handler will be eventually deprecated. 2) When Kudu/HMS integration is enabled, the Kudu table underneath the managed HMS table will follow the naming convention 'db_name.table_name' instead of 'impala::db_name.table_name'. 3) Add 'kudu.table_id' table property to be used with Kudu/HMS integration. This commits also extracts Kudu related DDL parsing and analyzing tests, so that they can be run with or without Kudu/HMS integration enabled. Change-Id: I465673d749221bd5f3772814b1c22c2673a53f5c --- M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/local/LocalKuduTable.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/util/KuduUtil.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java A fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java A fe/src/test/java/org/apache/impala/analysis/AuditingKuduTest.java M fe/src/test/java/org/apache/impala/analysis/AuditingTest.java A fe/src/test/java/org/apache/impala/analysis/KuduHMSIntegrationTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java 13 files changed, 825 insertions(+), 572 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/13318/2 -- To view, visit http://gerrit.cloudera.org:8080/13318 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I465673d749221bd5f3772814b1c22c2673a53f5c Gerrit-Change-Number: 13318 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8504: Introduce the new Kudu storage handler
Hao Hao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13358 Change subject: IMPALA-8504: Introduce the new Kudu storage handler .. IMPALA-8504: Introduce the new Kudu storage handler This commits introduces the new Kudu storage handler 'org.apache.kudu.hive.KuduStorageHandler' that is used in Kudu/HMS integration. Tables with either the legacy storage handler 'com.cloudera.kudu.hive.KuduStorageHandler' or the new one are now considered as Kudu tables. A series of follow-up patches will be rely on the new storage handler to support Kudu/HMS integration in Impala. Change-Id: I75bcd5246005f4e35251aef9219f4d07eeb87dc6 --- M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java 2 files changed, 16 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/13358/1 -- To view, visit http://gerrit.cloudera.org:8080/13358 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I75bcd5246005f4e35251aef9219f4d07eeb87dc6 Gerrit-Change-Number: 13358 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao
[Impala-ASF-CR] IMPALA-8503: add option to start Kudu cluster with HMS integration
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/13248 )
Change subject: IMPALA-8503: add option to start Kudu cluster with HMS
integration
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13248/1/testdata/cluster/node_templates/common/etc/init.d/kudu-master
File testdata/cluster/node_templates/common/etc/init.d/kudu-master:
http://gerrit.cloudera.org:8080/#/c/13248/1/testdata/cluster/node_templates/common/etc/init.d/kudu-master@31
PS1, Line 31:
KUDU_COMMON_ARGS+=("-hive_metastore_uris=thrift://${INTERNAL_LISTEN_HOST}:9083")
> Instead of doing all of the work of piping an argument all the way through
Yeah, I separated the patches to be one for adding the option to start with HMS
integration and the ones actually using it. Because I think it might be clearer
and simpler to review.
I can see adding an env variable seems to be a bit easier in terms of code
changes to the start script, but it requires extra work of setting the variable
in the test cases (and it will affect all consumers of the variable). I don't
have strong opinion to choose one from the other. So I will keep it the way it
is unless you feel otherwise.
--
To view, visit http://gerrit.cloudera.org:8080/13248
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I734d14ede6a03ad52e820e38a1fbcbac0a40ede2
Gerrit-Change-Number: 13248
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Marshall
Gerrit-Comment-Date: Mon, 13 May 2019 06:41:01 +
Gerrit-HasComments: Yes
