[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 ) Change subject: IMPALA-8593: Prohibit write operations for bucketed tables .. Patch Set 6: Code-Review+1 (3 comments) http://gerrit.cloudera.org:8080/#/c/13558/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13558/6//COMMIT_MSG@7 PS6, Line 7: Prohibit write operations for bucketed tables I think that bucketed tables are now a secondary feature compared to capability handling. Maybe something like "Support table capabilities handling with Hive 3" would be more descriptive. http://gerrit.cloudera.org:8080/#/c/13558/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/13558/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2210 PS6, Line 2210: The table cannot be found after created. This means that HMS doesn't return Kudu tables where OBJCAPABILITIES is set? If there is a jira for this, can you mention it in the comment? http://gerrit.cloudera.org:8080/#/c/13558/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2211 PS6, Line 2211: addiing typo: adding -- To view, visit http://gerrit.cloudera.org:8080/13558 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec Gerrit-Change-Number: 13558 Gerrit-PatchSet: 6 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 11 Jul 2019 12:05:24 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 ) Change subject: IMPALA-8593: Prohibit write operations for bucketed tables .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3861/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/13558 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec Gerrit-Change-Number: 13558 Gerrit-PatchSet: 6 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 11 Jul 2019 04:28:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Yongzhi Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 ) Change subject: IMPALA-8593: Prohibit write operations for bucketed tables .. Patch Set 6: (2 comments) Patch set 6 addressed the review issues and was rebased to the latest hive bump. Run cdp all core tests, the failures are the same as before the change to upstream master. Both have 12 failures: My private cdp build: https://master-02.jenkins.cloudera.com/job/impala-private-parameterized/5393/testReport/ upstream without the patch: https://jenkins.impala.io/job/ubuntu-16.04-from-scratch-cdp-hive/23/testReport/ So no regressions. http://gerrit.cloudera.org:8080/#/c/13558/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/13558/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2208 PS5, Line 2208: } e > What should happen if it is a full acid tables? If it shouldn't be possible Done http://gerrit.cloudera.org:8080/#/c/13558/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2210 PS5, Line 2210: 1. The property is not stor > A comment could be added about the lack of capabilities for Kudu tables. Done -- To view, visit http://gerrit.cloudera.org:8080/13558 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec Gerrit-Change-Number: 13558 Gerrit-PatchSet: 6 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 11 Jul 2019 04:00:00 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 ) Change subject: IMPALA-8593: Prohibit write operations for bucketed tables .. Patch Set 6: (4 comments) http://gerrit.cloudera.org:8080/#/c/13558/6/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java: http://gerrit.cloudera.org:8080/#/c/13558/6/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@20 PS6, Line 20: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_NONE; line too long (91 > 90) http://gerrit.cloudera.org:8080/#/c/13558/6/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@21 PS6, Line 21: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READONLY; line too long (95 > 90) http://gerrit.cloudera.org:8080/#/c/13558/6/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@22 PS6, Line 22: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READWRITE; line too long (96 > 90) http://gerrit.cloudera.org:8080/#/c/13558/6/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@23 PS6, Line 23: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_WRITEONLY; line too long (96 > 90) -- To view, visit http://gerrit.cloudera.org:8080/13558 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec Gerrit-Change-Number: 13558 Gerrit-PatchSet: 6 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 11 Jul 2019 03:49:28 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Hello Vihang Karajgaonkar, Sudhanshu Arora, Zoltan Borok-Nagy, Sahil Takiar, Todd Lipcon, Joe McDonnell, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13558 to look at the new patch set (#6). Change subject: IMPALA-8593: Prohibit write operations for bucketed tables .. IMPALA-8593: Prohibit write operations for bucketed tables This patch adds a method to check if a table bucketed. For Hive 3, integrates with HMS translation layer for capabilities checks. Implements methods ensureTableWriteSupported and ensureTableReadSupported. Set default capabilities for tables. Tests: Added unit tests to ParserTest and AnalyzerTest. Added bucketed tables which are required by IMPALA-8439. Ran core tests(Hive 2 and Hive 3) ToDo: Integrate checking bucketed tables capabilities and creating error messages with HMS translation after Hive provides the required functions. Enable capabilities checking for Kudu tables. Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec --- M fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java M fe/src/main/java/org/apache/impala/analysis/TruncateStmt.java M fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M testdata/datasets/functional/functional_schema_template.sql M testdata/datasets/functional/schema_constraints.csv M tests/metadata/test_ddl.py M tests/metadata/test_show_create_table.py 19 files changed, 392 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/13558/6 -- To view, visit http://gerrit.cloudera.org:8080/13558 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec Gerrit-Change-Number: 13558 Gerrit-PatchSet: 6 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 ) Change subject: IMPALA-8593: Prohibit write operations for bucketed tables .. Patch Set 5: Code-Review+1 (2 comments) http://gerrit.cloudera.org:8080/#/c/13558/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/13558/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2208 PS5, Line 2208: } What should happen if it is a full acid tables? If it shouldn't be possible, that a precondition could be used instead of the "if". http://gerrit.cloudera.org:8080/#/c/13558/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2210 PS5, Line 2210: !KuduTable.isKuduTable(tbl) A comment could be added about the lack of capabilities for Kudu tables. -- To view, visit http://gerrit.cloudera.org:8080/13558 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec Gerrit-Change-Number: 13558 Gerrit-PatchSet: 5 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 10 Jul 2019 18:47:32 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 ) Change subject: IMPALA-8593: Prohibit write operations for bucketed tables .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3836/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/13558 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec Gerrit-Change-Number: 13558 Gerrit-PatchSet: 5 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 08 Jul 2019 14:14:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Yongzhi Chen has posted comments on this change. (
http://gerrit.cloudera.org:8080/13558 )
Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
..
Patch Set 5:
Ran Hive 2 and Hive 3 core tests. For hive 3 tests, there are 4 failures and
are the same failures as upstream master :
Test Result (4 failures / -7)
custom_cluster.test_hive_parquet_codec_interop.TestParquetInterop.test_hive_impala_interop[protocol:
beeswax | exec_option: {'sync_ddl': 1, 'batch_size': 0, 'num_nodes': 0,
'disable_codegen_rows_threshold': 0, 'disable_codegen': False,
'abort_on_error': 1, 'exec_single_node_rows_threshold': 0} | table_format:
parquet/none]
generate_junitxml.buildall.run-custom-cluster-tests
org.apache.impala.catalog.events.MetastoreEventsProcessorTest.testInsertEventOnRemovedTable
org.apache.impala.catalog.events.MetastoreEventsProcessorTest.testInsertEvents
https://master-02.jenkins.cloudera.com/job/impala-private-parameterized/5367/#showFailuresLink
https://jenkins.impala.io/job/ubuntu-16.04-from-scratch-cdp-hive/20/#showFailuresLink
So there is no regression.
Because of recent cdp bumping from 107 to 195, Impala has to add capabilities
when creating tables. Most impala created table have OBJCAPABILITIES except for
kudu tables (seems that kudu managed table does not store extra table
properties). Therefore, kudu table will not check capabilities for now.
--
To view, visit http://gerrit.cloudera.org:8080/13558
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
Gerrit-Change-Number: 13558
Gerrit-PatchSet: 5
Gerrit-Owner: Yongzhi Chen
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Sahil Takiar
Gerrit-Reviewer: Sudhanshu Arora
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Reviewer: Yongzhi Chen
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Mon, 08 Jul 2019 13:48:11 +
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 ) Change subject: IMPALA-8593: Prohibit write operations for bucketed tables .. Patch Set 5: (4 comments) http://gerrit.cloudera.org:8080/#/c/13558/5/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java: http://gerrit.cloudera.org:8080/#/c/13558/5/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@20 PS5, Line 20: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_NONE; line too long (91 > 90) http://gerrit.cloudera.org:8080/#/c/13558/5/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@21 PS5, Line 21: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READONLY; line too long (95 > 90) http://gerrit.cloudera.org:8080/#/c/13558/5/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@22 PS5, Line 22: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READWRITE; line too long (96 > 90) http://gerrit.cloudera.org:8080/#/c/13558/5/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@23 PS5, Line 23: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_WRITEONLY; line too long (96 > 90) -- To view, visit http://gerrit.cloudera.org:8080/13558 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec Gerrit-Change-Number: 13558 Gerrit-PatchSet: 5 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 08 Jul 2019 13:34:27 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Hello Vihang Karajgaonkar, Sudhanshu Arora, Zoltan Borok-Nagy, Sahil Takiar, Todd Lipcon, Joe McDonnell, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13558 to look at the new patch set (#5). Change subject: IMPALA-8593: Prohibit write operations for bucketed tables .. IMPALA-8593: Prohibit write operations for bucketed tables This patch adds a method to check if a table bucketed. For Hive 3, integrates with HMS translation layer for capabilities checks. Implements methods ensureTableWriteSupported and ensureTableReadSupported. Set default capabilities for tables. Tests: Added unit tests to ParserTest and AnalyzerTest. Added bucketed tables which are required by IMPALA-8439. Ran core tests(Hive 2 and Hive 3) ToDo: Integrate checking bucketed tables capabilities and creating error messages with HMS translation after Hive provides the required functions. Enable capabilities checking for Kudu tables. Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec --- M fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java M fe/src/main/java/org/apache/impala/analysis/TruncateStmt.java M fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M testdata/datasets/functional/functional_schema_template.sql M testdata/datasets/functional/schema_constraints.csv M tests/metadata/test_ddl.py M tests/metadata/test_show_create_table.py 19 files changed, 390 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/13558/5 -- To view, visit http://gerrit.cloudera.org:8080/13558 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec Gerrit-Change-Number: 13558 Gerrit-PatchSet: 5 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 ) Change subject: IMPALA-8593: Prohibit write operations for bucketed tables .. Patch Set 4: Code-Review+2 I give +2, but I would prefer to run https://jenkins.impala.io/job/ubuntu-16.04-from-scratch-cdp-hive/ with the change before merging it. The cdp build is still red, but there are only a few ( 3 ) errors left on master. -- To view, visit http://gerrit.cloudera.org:8080/13558 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec Gerrit-Change-Number: 13558 Gerrit-PatchSet: 4 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 03 Jul 2019 12:56:57 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Yongzhi Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 ) Change subject: IMPALA-8593: Prohibit write operations for bucketed tables .. Patch Set 4: Hive will treat all acid table as managed and all managed are needed to be transactional. Impala created tables will be external tables. The changing and discussion is still going on. This patch is just first step, we can make more fixes later. Following are related docs for HMS translation layer: Hive: https://docs.google.com/document/d/1ZmtMlKvnSz-EFGuCuYfPVA-2kfgHOWenL8sW47vkhnw/edit# Impala: https://docs.google.com/document/d/1lgcD_ZqIFVydLdRPQhkYBdFu2C0mH1A8VnXpcSs9psA/edit#heading=h.l9b51051hhma Three engines: https://docs.google.com/document/d/1fPIn1WW-tRRSmM-aGEVl0DI-TByIswo5HrLdgSPcw9s/edit?ts=5d1baa75 -- To view, visit http://gerrit.cloudera.org:8080/13558 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec Gerrit-Change-Number: 13558 Gerrit-PatchSet: 4 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 03 Jul 2019 12:14:58 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 ) Change subject: IMPALA-8593: Prohibit write operations for bucketed tables .. Patch Set 4: Code-Review+1 (1 comment) The code looks good to me, but I am still trying to grasp the details Hive capabilities concept. For example I saw a line in HMS that suggests that managed tables without ACID will be treated as read only: https://github.com/apache/hive/blob/ec3779978797051fdb345172536aafcd50f1b4ae/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetastoreDefaultTransformer.java#L130 Can't this break some Impala tests? http://gerrit.cloudera.org:8080/#/c/13558/4/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java File fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java: http://gerrit.cloudera.org:8080/#/c/13558/4/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java@599 PS4, Line 599: catlaog typo: catalog -- To view, visit http://gerrit.cloudera.org:8080/13558 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec Gerrit-Change-Number: 13558 Gerrit-PatchSet: 4 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 01 Jul 2019 16:06:29 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 ) Change subject: IMPALA-8593: Prohibit write operations for bucketed tables .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3790/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/13558 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec Gerrit-Change-Number: 13558 Gerrit-PatchSet: 4 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Sat, 29 Jun 2019 12:12:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Yongzhi Chen has posted comments on this change. (
http://gerrit.cloudera.org:8080/13558 )
Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
..
Patch Set 4:
(14 comments)
Patch set 4 addresses the issue and passed core tests for Hive2
http://gerrit.cloudera.org:8080/#/c/13558/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13558/3//COMMIT_MSG@9
PS3, Line 9:
> nit: extra space
Done
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java:
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@60
PS3, Line 60: import org.apache.impala.common.Imp
> I didn't find where we use this.
removed
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@64
PS3, Line 64: import org.apache.impala.service.MetadataOp;
> This is only mentioned in comment, but is not used.
removed
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@438
PS3, Line 438:* - insert-only Acid table read
:* - virtual view read
:* - materialized view read
:*/
> Can you make a bullet list from this?
Done
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@445
PS3, Line 445: ame = I
> Unused variable.
Done
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@451
PS3, Line 451: // Use MAJOR_VERSION for now
: // TODO: Change to impala build version when the functions
are fixed.
: String impalaId = St
> I do not understand the intention here.
Explain why I am using major version not impala version for impala id: The
function available in Impala does not work for catalog front end and causes for
troubles. For the issue does not affect the feature(any name will be fine),
comment here, we can change the name after the related functions fixed.
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@454
PS3, Line 454: String[] capabilities = new String[] {
: EXTWRITE, // External tab
> nit: one line
Done
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@466
PS3, Line 466: };
> nit: missing "is"
I think the "is" can be omitted, but I will add it anyway.
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@300
PS3, Line 300: Check if the table type is supported
> nit: "Check if the table type is supported" would be better.
Done
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@311
PS3, Line 311: // the operations are not supported, we will generate
error messages
> nit: missing "are"
This can be omitted too.
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@319
PS3, Line 319: ensureTableNotTransactional(ta
> Does this make sense in the MetastoreShim.getMajorVersion() == 2 case?
It is just a direct translation of the original code where
ensureTableNotFullAcid(table) used, for version 2, need check if it is
transactional. I will change to ensureTableNotTransactional(table)
And this kind of check is needed for transactional table use table properties,
it can go into version 2 DB.
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
File fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java:
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java@365
PS3, Line 365: analyzer.ensureTableNotTransactional(table_);
> I also mentioned in AnalyzerTest.java that prohibiting COMPUTE STATS is str
Done, change back to the original implementation. I thought we cannot compute
accurate stats for the tables that we do not know how to write.
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java:
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java@897
PS3, Line 897: "is a bucketed table. Only read operations are supported
on such tables
> It is strange for me that DROP STATS is allowed, while COMPUTE STATS is not
Fixed.
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 ) Change subject: IMPALA-8593: Prohibit write operations for bucketed tables .. Patch Set 4: (4 comments) http://gerrit.cloudera.org:8080/#/c/13558/4/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java: http://gerrit.cloudera.org:8080/#/c/13558/4/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@20 PS4, Line 20: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_NONE; line too long (91 > 90) http://gerrit.cloudera.org:8080/#/c/13558/4/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@21 PS4, Line 21: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READONLY; line too long (95 > 90) http://gerrit.cloudera.org:8080/#/c/13558/4/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@22 PS4, Line 22: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READWRITE; line too long (96 > 90) http://gerrit.cloudera.org:8080/#/c/13558/4/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@23 PS4, Line 23: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_WRITEONLY; line too long (96 > 90) -- To view, visit http://gerrit.cloudera.org:8080/13558 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec Gerrit-Change-Number: 13558 Gerrit-PatchSet: 4 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Sat, 29 Jun 2019 11:33:21 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Hello Vihang Karajgaonkar, Sudhanshu Arora, Zoltan Borok-Nagy, Sahil Takiar, Todd Lipcon, Joe McDonnell, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13558 to look at the new patch set (#4). Change subject: IMPALA-8593: Prohibit write operations for bucketed tables .. IMPALA-8593: Prohibit write operations for bucketed tables This patch adds a method to check if a table bucketed. For Hive 3, integrates with HMS translation layer for capabilities checks. Implements methods ensureTableWriteSupported and ensureTableReadSupported. Tests: Added unit tests to ParserTest and AnalyzerTest. Added bucketed tables which are required by IMPALA-8439. Ran core tests. ToDo: Integrate checking bucketed tables capabilities and creating error messages with HMS translation after Hive provides the required functions. Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec --- M fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java M fe/src/main/java/org/apache/impala/analysis/TruncateStmt.java M fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M testdata/datasets/functional/functional_schema_template.sql M testdata/datasets/functional/schema_constraints.csv 17 files changed, 346 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/13558/4 -- To view, visit http://gerrit.cloudera.org:8080/13558 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec Gerrit-Change-Number: 13558 Gerrit-PatchSet: 4 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Csaba Ringhofer has posted comments on this change. (
http://gerrit.cloudera.org:8080/13558 )
Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
..
Patch Set 3:
(14 comments)
http://gerrit.cloudera.org:8080/#/c/13558/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13558/3//COMMIT_MSG@9
PS3, Line 9:
nit: extra space
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java:
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@60
PS3, Line 60: import org.apache.impala.common.Id;
I didn't find where we use this.
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@64
PS3, Line 64: import org.apache.impala.service.BackendConfig;
This is only mentioned in comment, but is not used.
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@438
PS3, Line 438:* Impala supports external table read/write
:* insert-only Acid table read
:* virtual view read
:* materialized view read
Can you make a bullet list from this?
e.g:
* Impala supports:
* - external table read/write
* - insert-only Acid table read
* - virtual view read
* - materialized view read
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@445
PS3, Line 445: version
Unused variable.
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@451
PS3, Line 451: // BackendConfig.INSTANCE.getImpalaBuildVersion() is NULL or
: // BackendConfig.INSTANCE is NULL when the method is called.
: // Use MAJOR_VERSION
I do not understand the intention here.
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@454
PS3, Line 454: String impalaId = String.format("Impala%s@%s",
: MAJOR_VERSION, hostName);
nit: one line
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@466
PS3, Line 466: // number -1. It makes clients unable to
know it a bucketed table.
nit: missing "is"
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@300
PS3, Line 300: Check if support the table type or not
nit: "Check if the table type is supported" would be better.
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@311
PS3, Line 311: // the operations not supported, we will generate error
messages
nit: missing "are"
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@319
PS3, Line 319: ensureTableNotFullAcid(table);
Does this make sense in the MetastoreShim.getMajorVersion() == 2 case?
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
File fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java:
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java@365
PS3, Line 365: analyzer.checkTableCapability(table_,
Analyzer.OperationType.WRITE);
I also mentioned in AnalyzerTest.java that prohibiting COMPUTE STATS is strange
for me. Impala can generally do COMPUTE STATS for tables it cannot write, e.g.
Avro tables.
I think that Transactional tables were excluded because Hive uses the stats for
them to answer some queries, and we are afraid that differences in stats by
Impala can lead to incorrect results in Hive.
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java:
http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java@897
PS3, Line 897: AnalysisError("compute stats functional.bucketed_table",
errorMsgBucketed);
It is strange for me that DROP STATS is allowed, while COMPUTE STATS is not for
bucketed tables. Is this intentional?
http://gerrit.cloudera.org:8080/#/c/13558/3/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:
http://gerrit.cloudera.org:8080/#/c/13558/3/testdata/datasets/functional/functional_schema_template.sql@2162
PS3, Line 2162: STORED AS ORC
Does this
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 ) Change subject: IMPALA-8593: Prohibit write operations for bucketed tables .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3581/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/13558 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec Gerrit-Change-Number: 13558 Gerrit-PatchSet: 3 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 12 Jun 2019 18:57:57 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 ) Change subject: IMPALA-8593: Prohibit write operations for bucketed tables .. Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java: http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@20 PS3, Line 20: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_NONE; line too long (91 > 90) http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@21 PS3, Line 21: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READONLY; line too long (95 > 90) http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@22 PS3, Line 22: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READWRITE; line too long (96 > 90) http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@23 PS3, Line 23: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_WRITEONLY; line too long (96 > 90) -- To view, visit http://gerrit.cloudera.org:8080/13558 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec Gerrit-Change-Number: 13558 Gerrit-PatchSet: 3 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 12 Jun 2019 18:16:26 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Hello Vihang Karajgaonkar, Sudhanshu Arora, Zoltan Borok-Nagy, Todd Lipcon, Joe McDonnell, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13558 to look at the new patch set (#3). Change subject: IMPALA-8593: Prohibit write operations for bucketed tables .. IMPALA-8593: Prohibit write operations for bucketed tables This patch adds a method to check if a table bucketed. For Hive 3, integrates with HMS translation layer for capabilities checks. Implements methods ensureTableWriteSupported and ensureTableReadSupported. Tests: Added unit tests to ParserTest and AnalyzerTest. Added bucketed tables which are required by IMPALA-8439. Ran core tests. ToDo: Integrate checking bucketed tables capabilities and creating error messages with HMS translation after Hive provides the required functions. Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec --- M bin/impala-config.sh M fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java M fe/src/main/java/org/apache/impala/analysis/TruncateStmt.java M fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M testdata/datasets/functional/functional_schema_template.sql M testdata/datasets/functional/schema_constraints.csv 19 files changed, 354 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/13558/3 -- To view, visit http://gerrit.cloudera.org:8080/13558 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec Gerrit-Change-Number: 13558 Gerrit-PatchSet: 3 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Yongzhi Chen has posted comments on this change. (
http://gerrit.cloudera.org:8080/13558 )
Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
..
Patch Set 2:
(12 comments)
Patch 3 will address the issues.
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java:
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@78
PS1, Line 78: private static final String CONNECTORREAD =
"CONNECTORREAD".intern();
> Just to be clear I was saying to do the following
These strings are only used to be packed into a string array and sent to hive
as capabilities. I will use enum in the future if we need enum operations.
I will remove the intern (just copied from hive code)
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@92
PS1, Line 92: private static final long MAJOR_VERSION = 3;
> nit: HIVE_MAJOR_VERSION?
It is Impala shim's major version.
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@437
PS1, Line 437:* Set impala capabilities to hive client
> nit: could you elaborate a little? E.g. why is it needed, what are these ca
Done
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java:
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java@93
PS1, Line 93: analyzer.ensureTableWriteSupported(table_);
> 1) If Hive does not define Metadata_modification and we want to fold this o
Bitwise can combine two checks into one (for example check write along with
read/write). The major purpose is to integrate with hive's translation layer,
hive access type is in byte, bitwise operations work fine with it and may have
better performance and simpler (save a couple of switch statements)
This function is frequently called, checking annotation value for operation
type may be roundabout. I will accept your first option to use enum .
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@129
PS1, Line 129: public static final byte REQUIRE_READ = (byte)2;
> Why not import hive_metastoreConstants access types?
It is auto-generated code and Hive 2 does not have these constant. We do not
want to break hive 2 impala.
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@150
PS1, Line 150: "Table %s not supported. Bucketed tables are only supported " +
: "for read."
> nit: How about "%s is a bucketed table. Only read operations are supported
Done
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java:
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java@876
PS1, Line 876: if (MetastoreShim.getMajorVersion() > 2) {
> Maybe you could add tests for the other statements as well, e.g. COMPUTE ST
Done
http://gerrit.cloudera.org:8080/#/c/13558/1/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:
http://gerrit.cloudera.org:8080/#/c/13558/1/testdata/datasets/functional/functional_schema_template.sql@2183
PS1, Line 2183: LOAD
: INSERT OVERWRITE TABLE {db_name}{db_suffix}.{table_name}
: SELECT id, int_col from functional.alltypes;
> This will only run for the non-text versions of the tables. If you want the
Done
http://gerrit.cloudera.org:8080/#/c/13558/2/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:
http://gerrit.cloudera.org:8080/#/c/13558/2/testdata/datasets/functional/functional_schema_template.sql@2172
PS2, Line 2172: CLUSTERED BY (col1) INTO 5 BUCKETS
> Without STORED AS {file_format} all tables will be text files.
Done
http://gerrit.cloudera.org:8080/#/c/13558/2/testdata/datasets/functional/functional_schema_template.sql@2173
PS2, Line 2173: LOCATION '/test-warehouse/{table_name}';
> Adding {db_name}{db_suffix} would ensure that tables in different databases
Done
http://gerrit.cloudera.org:8080/#/c/13558/2/testdata/datasets/functional/functional_schema_template.sql@2182
PS2, Line 2182: CLUSTERED BY (col1) INTO 5 BUCKETS;
> Same as line 2172.
Done
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Csaba Ringhofer has posted comments on this change. (
http://gerrit.cloudera.org:8080/13558 )
Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
..
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/13558/2/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:
http://gerrit.cloudera.org:8080/#/c/13558/2/testdata/datasets/functional/functional_schema_template.sql@2172
PS2, Line 2172: CLUSTERED BY (col1) INTO 5 BUCKETS
Without STORED AS {file_format} all tables will be text files.
http://gerrit.cloudera.org:8080/#/c/13558/2/testdata/datasets/functional/functional_schema_template.sql@2173
PS2, Line 2173: LOCATION '/test-warehouse/{table_name}';
Adding {db_name}{db_suffix} would ensure that tables in different databases use
different locations. This probably doesn't matter if the table is empty, but it
is still weird if e.g. a Parquet and a text table use the same directory.
http://gerrit.cloudera.org:8080/#/c/13558/2/testdata/datasets/functional/functional_schema_template.sql@2182
PS2, Line 2182: CLUSTERED BY (col1) INTO 5 BUCKETS;
Same as line 2172.
--
To view, visit http://gerrit.cloudera.org:8080/13558
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
Gerrit-Change-Number: 13558
Gerrit-PatchSet: 2
Gerrit-Owner: Yongzhi Chen
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Sudhanshu Arora
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Reviewer: Yongzhi Chen
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Wed, 12 Jun 2019 15:40:21 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Zoltan Borok-Nagy has posted comments on this change. (
http://gerrit.cloudera.org:8080/13558 )
Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13558/2/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:
http://gerrit.cloudera.org:8080/#/c/13558/2/testdata/datasets/functional/functional_schema_template.sql@2183
PS2, Line 2183: LOAD
: INSERT OVERWRITE TABLE {db_name}{db_suffix}.{table_name}
: SELECT id, int_col from functional.alltypes;
This only loads the data to the text table, the other formats won't contain any
data.
You'll need a DEPENDENT_LOAD_HIVE section with an 'INSERT OVERWRITE TABLE
{db_name}{db_suffix}.{table_name} SELECT * FROM functional.{table_name};'
statement. This copies data to the other formats from the text table. See
'complextypes_fileformat' that Joe mentioned.
Of course, you could also use the same statement in DEPENDENT_LOAD_HIVE that
you use in LOAD, but that way you'd need to modify the INSERT statement at both
places whenever you want to make changes.
--
To view, visit http://gerrit.cloudera.org:8080/13558
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
Gerrit-Change-Number: 13558
Gerrit-PatchSet: 2
Gerrit-Owner: Yongzhi Chen
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Sudhanshu Arora
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Reviewer: Yongzhi Chen
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Wed, 12 Jun 2019 09:39:13 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Joe McDonnell has posted comments on this change. (
http://gerrit.cloudera.org:8080/13558 )
Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
..
Patch Set 1:
(1 comment)
Drive-by comment. Hope this helps!
http://gerrit.cloudera.org:8080/#/c/13558/1/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:
http://gerrit.cloudera.org:8080/#/c/13558/1/testdata/datasets/functional/functional_schema_template.sql@2183
PS1, Line 2183: DEPENDENT_LOAD_HIVE
: INSERT INTO TABLE {db_name}{db_suffix}.{table_name}
: SELECT id, int_col from functional.alltypes;
This will only run for the non-text versions of the tables. If you want the
same thing for text, you would want a LOAD section (which always runs in Hive).
See, for example, complextypes_fileformat for a table that has an insert in the
LOAD section.
--
To view, visit http://gerrit.cloudera.org:8080/13558
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
Gerrit-Change-Number: 13558
Gerrit-PatchSet: 1
Gerrit-Owner: Yongzhi Chen
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Sudhanshu Arora
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Reviewer: Yongzhi Chen
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Tue, 11 Jun 2019 19:49:32 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Sudhanshu Arora has posted comments on this change. (
http://gerrit.cloudera.org:8080/13558 )
Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
..
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java:
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@78
PS1, Line 78: private static final String CONNECTORREAD =
"CONNECTORREAD".intern();
> These are hive defined strings for capabilities, they only recognize these
Just to be clear I was saying to do the following
public enum Capability {
CONNECTOREAD("CONNECTTOREAD")
private final String capabilityName;
private final getName() {
return capabilityName;
}
}
And we can use the getName to get the String. If you still feel using String
constants is better, please go ahead with that.
Also what's the benefit of calling intern on the string constants?
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java:
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java@93
PS1, Line 93: analyzer.ensureTableWriteSupported(table_);
> Hive defines access types as byte, we combine the access type check in Met
1) If Hive does not define Metadata_modification and we want to fold this
operation under write. We can do that, i.e. we could still define
@OperationType(WRITE)
2) What is the advantage of using bitwise operators?
I do think annotations are introducing slightly more work. And before we make
the change we should agree that it offers more clarity than the current changes.
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
File fe/src/test/java/org/apache/impala/analysis/ParserTest.java:
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@4021
PS1, Line 4021: ParserError("Create table bucketed_tbl(order_id int,
order_name string)"
> Parsing happens before analysis. Impala does not recognize clustered by, so
My bad. I don't know what I was thinking :).
--
To view, visit http://gerrit.cloudera.org:8080/13558
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
Gerrit-Change-Number: 13558
Gerrit-PatchSet: 1
Gerrit-Owner: Yongzhi Chen
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sudhanshu Arora
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Reviewer: Yongzhi Chen
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Tue, 11 Jun 2019 16:04:26 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Zoltan Borok-Nagy has posted comments on this change. (
http://gerrit.cloudera.org:8080/13558 )
Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
..
Patch Set 1:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java:
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@92
PS1, Line 92: private static final long MAJOR_VERSION = 3;
nit: HIVE_MAJOR_VERSION?
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@437
PS1, Line 437:* Set impala capabilities to hive client
nit: could you elaborate a little? E.g. why is it needed, what are these
capabilities?
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@129
PS1, Line 129: public static final byte REQUIRE_READ = (byte)2;
> They are consistent with hive thrift defined constant for access type, and
Why not import hive_metastoreConstants access types?
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@150
PS1, Line 150: "Table %s not supported. Bucketed tables are only supported " +
: "for read."
nit: How about "%s is a bucketed table. Only read operations are supported on
such tables."
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java:
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java@876
PS1, Line 876: if (MetastoreShim.getMajorVersion() > 2) {
Maybe you could add tests for the other statements as well, e.g. COMPUTE STATS,
CREATE TABLE LIKE, DROP, etc.
--
To view, visit http://gerrit.cloudera.org:8080/13558
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
Gerrit-Change-Number: 13558
Gerrit-PatchSet: 1
Gerrit-Owner: Yongzhi Chen
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sudhanshu Arora
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Reviewer: Yongzhi Chen
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Tue, 11 Jun 2019 15:12:18 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Yongzhi Chen has posted comments on this change. (
http://gerrit.cloudera.org:8080/13558 )
Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
..
Patch Set 1:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java:
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@78
PS1, Line 78: private static final String CONNECTORREAD =
"CONNECTORREAD".intern();
> How about using enum for this?
These are hive defined strings for capabilities, they only recognize these
strings.
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java:
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java@93
PS1, Line 93: analyzer.ensureTableWriteSupported(table_);
> Nit: Another way to do this is to introduce another enum say OperationType.
Hive defines access types as byte, we combine the access type check in
MetastoreShim and try not to spread the definition to so many places and take
advantage of bitwise operations. There is no METADATA_MODIFICATION in Hive,
only read, write, read/write: We need the write operation(write or read/write)
for it, this method includes both.
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@129
PS1, Line 129: public static final byte REQUIRE_READ = (byte)2;
> If we create an enum then these values can go into enum itself
They are consistent with hive thrift defined constant for access type, and byte
type has one more advantage: it can run bitwise operations like | (bitwise
logical or)
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
File fe/src/test/java/org/apache/impala/analysis/ParserTest.java:
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@4021
PS1, Line 4021: ParserError("Create table bucketed_tbl(order_id int,
order_name string)"
> Wouldn't this fail during analysis?
Parsing happens before analysis. Impala does not recognize clustered by, so it
failed with the Syntax error
--
To view, visit http://gerrit.cloudera.org:8080/13558
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
Gerrit-Change-Number: 13558
Gerrit-PatchSet: 1
Gerrit-Owner: Yongzhi Chen
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sudhanshu Arora
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Reviewer: Yongzhi Chen
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Tue, 11 Jun 2019 12:02:32 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Sudhanshu Arora has posted comments on this change. (
http://gerrit.cloudera.org:8080/13558 )
Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
..
Patch Set 1:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java:
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@78
PS1, Line 78: private static final String CONNECTORREAD =
"CONNECTORREAD".intern();
How about using enum for this?
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java:
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java@93
PS1, Line 93: analyzer.ensureTableWriteSupported(table_);
Nit: Another way to do this is to introduce another enum say OperationType.
public enum OperationType {
READ,
WRITE,
METADATA_MODIFICATION
}
And then mark Alter Table as METADATA_MODIFICATION.
Then we can have a function in Analyzer that takes the operationType and tells
whether the operation is allowed on the table or not.
That will make this cleaner I think
Or we can introduce annotations like
@SupportedTableTypes
@OperationType
and we can say that every stmt node must be annotated with these. That will
make it clear.
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@129
PS1, Line 129: public static final byte REQUIRE_READ = (byte)2;
If we create an enum then these values can go into enum itself
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
File fe/src/test/java/org/apache/impala/analysis/ParserTest.java:
http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@4021
PS1, Line 4021: ParserError("Create table bucketed_tbl(order_id int,
order_name string)"
Wouldn't this fail during analysis?
--
To view, visit http://gerrit.cloudera.org:8080/13558
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
Gerrit-Change-Number: 13558
Gerrit-PatchSet: 1
Gerrit-Owner: Yongzhi Chen
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sudhanshu Arora
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Reviewer: Yongzhi Chen
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Mon, 10 Jun 2019 20:54:20 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Yongzhi Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 ) Change subject: IMPALA-8593: Prohibit write operations for bucketed tables .. Patch Set 1: Please help me review the patch. Thanks -- To view, visit http://gerrit.cloudera.org:8080/13558 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec Gerrit-Change-Number: 13558 Gerrit-PatchSet: 1 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 10 Jun 2019 10:11:44 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 ) Change subject: IMPALA-8593: Prohibit write operations for bucketed tables .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3534/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/13558 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec Gerrit-Change-Number: 13558 Gerrit-PatchSet: 1 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Yongzhi Chen Gerrit-Comment-Date: Fri, 07 Jun 2019 14:22:18 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Yongzhi Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 ) Change subject: IMPALA-8593: Prohibit write operations for bucketed tables .. Patch Set 1: > Patch Set 1: > > (4 comments) Do we really want to put import into two lines? I will keep them as is. -- To view, visit http://gerrit.cloudera.org:8080/13558 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec Gerrit-Change-Number: 13558 Gerrit-PatchSet: 1 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Yongzhi Chen Gerrit-Comment-Date: Fri, 07 Jun 2019 13:46:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 ) Change subject: IMPALA-8593: Prohibit write operations for bucketed tables .. Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java: http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@20 PS1, Line 20: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_NONE; line too long (91 > 90) http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@21 PS1, Line 21: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READONLY; line too long (95 > 90) http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@22 PS1, Line 22: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READWRITE; line too long (96 > 90) http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@23 PS1, Line 23: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_WRITEONLY; line too long (96 > 90) -- To view, visit http://gerrit.cloudera.org:8080/13558 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec Gerrit-Change-Number: 13558 Gerrit-PatchSet: 1 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 07 Jun 2019 13:41:15 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables
Yongzhi Chen has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13558 Change subject: IMPALA-8593: Prohibit write operations for bucketed tables .. IMPALA-8593: Prohibit write operations for bucketed tables This patch adds a method to check if a table bucketed. For Hive 3, integrates with HMS translation layer for capabilities checks. Implements methods ensureTableWriteSupported and ensureTableReadSupported. Tests: Added unit tests to ParserTest and AnalyzerTest. Added bucketed tables which are required by IMPALA-8439. Ran core tests. ToDo: Integrate checking bucketed tables capabilities and creating error messages with HMS translation after Hive provides the required functions. Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec --- M bin/impala-config.sh M fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java M fe/src/main/java/org/apache/impala/analysis/TruncateStmt.java M fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M testdata/datasets/functional/functional_schema_template.sql M testdata/datasets/functional/schema_constraints.csv 19 files changed, 311 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/13558/1 -- To view, visit http://gerrit.cloudera.org:8080/13558 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec Gerrit-Change-Number: 13558 Gerrit-PatchSet: 1 Gerrit-Owner: Yongzhi Chen
