You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "wangsheng (Code Review)" <ge...@cloudera.org> on 2020/10/16 02:25:36 UTC
[Impala-ASF-CR] IMPALA-10166 ALTER TABLE for Iceberg tables This patch mainly implement ALTER TABLE for Iceberg tables, we currently supported these statements: * ADD COLUMNS * DROP COLUMN * REPLACE COLUMNS * ALTER COLUMN CHANGE COLUMN * RENAME TABLE * SET TB
wangsheng has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16606
Change subject: IMPALA-10166 ALTER TABLE for Iceberg tables This patch mainly implement ALTER TABLE for Iceberg tables, we currently supported these statements: * ADD COLUMNS * DROP COLUMN * REPLACE COLUMNS * ALTER COLUMN CHANGE COLUMN * RENAME TABLE * SET TB
......................................................................
IMPALA-10166 ALTER TABLE for Iceberg tables
This patch mainly implement ALTER TABLE for Iceberg
tables, we currently supported these statements:
* ADD COLUMNS
* DROP COLUMN
* REPLACE COLUMNS
* ALTER COLUMN CHANGE COLUMN
* RENAME TABLE
* SET TBL_PROPERTIES
But here something we need to pay attention:
1.We cannot use complex type for CHANGE COLUMN/REPLACE COLUMNS
2.RENAME TABLE is not supported for HadoopCatalog/HadoopTables
3.We cannot ADD/DROP PARTITION now since Iceberg no related api
Testing:
- Iceberg table alter test in test_iceberg.py
- Iceberg table negative test in test_scanners.py
Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
---
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAlterColStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableRecoverPartitionsStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableReplaceColsStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetFileFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetLocationStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCatalog.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopCatalog.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopTables.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/IcebergUtil.java
A testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M tests/query_test/test_iceberg.py
19 files changed, 569 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/16606/1
--
To view, visit http://gerrit.cloudera.org:8080/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 1
Gerrit-Owner: wangsheng <sk...@163.com>
[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
......................................................................
Patch Set 8: Code-Review+1
(4 comments)
Hey!
I did a quick review and only found nits. I'm fine to +2 once they are addressed.
http://gerrit.cloudera.org:8080/#/c/16606/8//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16606/8//COMMIT_MSG@9
PS8, Line 9: implement
nit: implements
http://gerrit.cloudera.org:8080/#/c/16606/8/fe/src/main/java/org/apache/impala/analysis/AlterTableReplaceColsStmt.java
File fe/src/main/java/org/apache/impala/analysis/AlterTableReplaceColsStmt.java:
http://gerrit.cloudera.org:8080/#/c/16606/8/fe/src/main/java/org/apache/impala/analysis/AlterTableReplaceColsStmt.java@25
PS8, Line 25: import org.apache.impala.catalog.FeIcebergTable;
Is this import needed?
http://gerrit.cloudera.org:8080/#/c/16606/8/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/16606/8/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@118
PS8, Line 118: support
nit: supports
http://gerrit.cloudera.org:8080/#/c/16606/8/testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
File testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test:
http://gerrit.cloudera.org:8080/#/c/16606/8/testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test@117
PS8, Line 117: CREATE TABLE iceberg_test9(
Instead of creating a new table here, could you re-use any of the ones above? It might have a good effect on the runtime of the test. (same goes for the other create table below)
--
To view, visit http://gerrit.cloudera.org:8080/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 8
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Thu, 29 Oct 2020 14:34:53 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
Posted by "wangsheng (Code Review)" <ge...@cloudera.org>.
wangsheng has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
......................................................................
IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
This patch mainly implement ALTER TABLE for Iceberg
tables, we currently supported these statements:
* ADD COLUMNS
* RENAME TABLE
* SET TBL_PROPERTIES
* SET OWNER
We forbidden DROP COLUMN/REPLACE COLUMNS/ALTER COLUMN in this
patch, since these statemens may caused Iceberg tables unreadable.
We may support resolved column by field id in the near future,
after that, we will support COLUMN/REPLACE COLUMNS/ALTER COLUMN
for Iceberg tables.
Here something we still need to pay attention:
1.RENAME TABLE is not supported for HadoopCatalog/HadoopTables,
even if we already implement 'RENAME TABLE' statement.
2.We cannot ADD/DROP PARTITION now since Iceberg no related api,
related work is already in progess in Iceberg.
Testing:
- Iceberg table alter test in test_iceberg.py
- Iceberg table negative test in test_scanners.py
Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
---
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAlterColStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableRecoverPartitionsStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableReplaceColsStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetFileFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetLocationStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCatalog.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopCatalog.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopTables.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/IcebergUtil.java
A testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M tests/query_test/test_iceberg.py
19 files changed, 529 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/16606/4
--
To view, visit http://gerrit.cloudera.org:8080/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 4
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
Posted by "wangsheng (Code Review)" <ge...@cloudera.org>.
wangsheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
......................................................................
Patch Set 4:
Now this patch keeping ADD COLUMN/RENAME TABLE/SET TBL_PROPERTIES/SET OWNER, and Jenkins pre-review-test passed based on patch 4: https://jenkins.impala.io/job/pre-review-test/754/
--
To view, visit http://gerrit.cloudera.org:8080/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 4
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Wed, 21 Oct 2020 11:29:02 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
......................................................................
Patch Set 10: Code-Review+2
Thanks for the changes!
--
To view, visit http://gerrit.cloudera.org:8080/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 10
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Fri, 30 Oct 2020 08:31:54 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
Posted by "wangsheng (Code Review)" <ge...@cloudera.org>.
wangsheng has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
......................................................................
IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
This patch mainly implement ALTER TABLE for Iceberg
tables, we currently support these statements:
* ADD COLUMNS
* RENAME TABLE
* SET TBL_PROPERTIES
* SET OWNER
We forbid DROP COLUMN/REPLACE COLUMNS/ALTER COLUMN in this
patch, since these statemens may make Iceberg tables unreadable.
We may support column resolution by field id in the near future,
after that, we will support COLUMN/REPLACE COLUMNS/ALTER COLUMN
for Iceberg tables.
Here something we still need to pay attention:
1.RENAME TABLE is not supported for HadoopCatalog/HadoopTables,
even if we already implement 'RENAME TABLE' statement, so we
only rename the table in the Hive Metastore for external table.
2.We cannot ADD/DROP PARTITION now since there is no API for that
in Iceberg, but related work is already in progess in Iceberg.
Testing:
- Iceberg table alter test in test_iceberg.py
- Iceberg table negative test in test_scanners.py
Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
---
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAlterColStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableRecoverPartitionsStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableReplaceColsStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetFileFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetLocationStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCatalog.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopCatalog.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopTables.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/IcebergUtil.java
A testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M tests/query_test/test_iceberg.py
19 files changed, 488 insertions(+), 11 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/16606/5
--
To view, visit http://gerrit.cloudera.org:8080/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 5
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
......................................................................
Patch Set 11:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6625/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 11
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Fri, 30 Oct 2020 08:32:27 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
Posted by "wangsheng (Code Review)" <ge...@cloudera.org>.
wangsheng has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
......................................................................
IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
This patch mainly implement ALTER TABLE for Iceberg
tables, we currently support these statements:
* ADD COLUMNS
* RENAME TABLE
* SET TBL_PROPERTIES
* SET OWNER
We forbid DROP COLUMN/REPLACE COLUMNS/ALTER COLUMN in this
patch, since these statemens may make Iceberg tables unreadable.
We may support column resolution by field id in the near future,
after that, we will support COLUMN/REPLACE COLUMNS/ALTER COLUMN
for Iceberg tables.
Here something we still need to pay attention:
1.RENAME TABLE is not supported for HadoopCatalog/HadoopTables,
even if we already implement 'RENAME TABLE' statement, so we
only rename the table in the Hive Metastore for external table.
2.We cannot ADD/DROP PARTITION now since there is no API for that
in Iceberg, but related work is already in progess in Iceberg.
Testing:
- Iceberg table alter test in test_iceberg.py
- Iceberg table negative test in test_scanners.py
Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
---
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAlterColStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableRecoverPartitionsStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableReplaceColsStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetFileFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetLocationStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCatalog.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopCatalog.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopTables.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/IcebergUtil.java
A testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M tests/query_test/test_iceberg.py
19 files changed, 493 insertions(+), 11 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/16606/7
--
To view, visit http://gerrit.cloudera.org:8080/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 7
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
......................................................................
Patch Set 4:
(13 comments)
Did a first round. The change looks good to me overall, but found a couple of nits.
http://gerrit.cloudera.org:8080/#/c/16606/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16606/4//COMMIT_MSG@10
PS4, Line 10: supported
nit: support
http://gerrit.cloudera.org:8080/#/c/16606/4//COMMIT_MSG@15
PS4, Line 15: forbidden
nit: forbid
http://gerrit.cloudera.org:8080/#/c/16606/4//COMMIT_MSG@16
PS4, Line 16: caused
nit: make
http://gerrit.cloudera.org:8080/#/c/16606/4//COMMIT_MSG@17
PS4, Line 17: resolved column by field id
column resolution by field id
http://gerrit.cloudera.org:8080/#/c/16606/4//COMMIT_MSG@23
PS4, Line 23: .
So we only rename the table in the Hive Metastore.
http://gerrit.cloudera.org:8080/#/c/16606/4//COMMIT_MSG@24
PS4, Line 24: since Iceberg no related api
since there is no API for that in Iceberg
http://gerrit.cloudera.org:8080/#/c/16606/4/fe/src/main/java/org/apache/impala/analysis/AlterTableAlterColStmt.java
File fe/src/main/java/org/apache/impala/analysis/AlterTableAlterColStmt.java:
http://gerrit.cloudera.org:8080/#/c/16606/4/fe/src/main/java/org/apache/impala/analysis/AlterTableAlterColStmt.java@185
PS4, Line 185: not supported complex type on Iceberg tables.
is not supported for complex types in Iceberg tables.
http://gerrit.cloudera.org:8080/#/c/16606/4/fe/src/main/java/org/apache/impala/analysis/AlterTableReplaceColsStmt.java
File fe/src/main/java/org/apache/impala/analysis/AlterTableReplaceColsStmt.java:
http://gerrit.cloudera.org:8080/#/c/16606/4/fe/src/main/java/org/apache/impala/analysis/AlterTableReplaceColsStmt.java@93
PS4, Line 93: // We cannot replace column if exists, from primitive type to complex type or
: // from complex type to primitive type
I thought we don't want to support REPLACE COLUMNS in this patch at all.
http://gerrit.cloudera.org:8080/#/c/16606/4/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/16606/4/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java@98
PS4, Line 98: if (getTargetTable() instanceof FeKuduTable) analyzeKuduTable(analyzer);
: else if (getTargetTable() instanceof FeIcebergTable) analyzeIcebergTable(analyzer);
The coding convention is to use braces for multi-line statements, i.e. turn this into:
if (...) {
...
} else {
...
}
http://gerrit.cloudera.org:8080/#/c/16606/4/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopTables.java
File fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopTables.java:
http://gerrit.cloudera.org:8080/#/c/16606/4/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopTables.java@100
PS4, Line 100: Cannot rename Hadoop tables
Cannot rename Iceberg tables that use 'hadoop.tables' as catalog.
http://gerrit.cloudera.org:8080/#/c/16606/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/16606/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@960
PS4, Line 960: //
nit: // slashes should be indented
http://gerrit.cloudera.org:8080/#/c/16606/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3388
PS4, Line 3388: oldMsTbl
It's actually the 'new' metastore table. We have deep copied it to safely modify it, then we'll use it in getHiveClient().alter_table() to update the table metadata in HMS.
Seems like we just call it 'msTbl' at similar places in this file.
http://gerrit.cloudera.org:8080/#/c/16606/4/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/16606/4/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@112
PS4, Line 112: * INTEGER -> LONG
: * FLOAT -> DOUBLE
: * DECIMAL(s1,p1) -> DECIMAL(s1,p2), same scale, p1<=p2
Do we have checks for these? What does Iceberg do in cases when we violate these rules?
We can deal with it in a later patch since alter column will be supported later.
--
To view, visit http://gerrit.cloudera.org:8080/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 4
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Wed, 21 Oct 2020 16:00:51 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/7519/ : 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/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 7
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Thu, 22 Oct 2020 15:31:59 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/7517/ : 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/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 6
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Thu, 22 Oct 2020 14:57:12 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10166 ALTER TABLE for Iceberg tables This patch mainly implement ALTER TABLE for Iceberg tables, we currently supported these statements: * ADD COLUMNS * DROP COLUMN * REPLACE COLUMNS * ALTER COLUMN CHANGE COLUMN * RENAME TABLE * SET TB
Posted by "wangsheng (Code Review)" <ge...@cloudera.org>.
wangsheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 ALTER TABLE for Iceberg tables This patch mainly implement ALTER TABLE for Iceberg tables, we currently supported these statements: * ADD COLUMNS * DROP COLUMN * REPLACE COLUMNS * ALTER COLUMN CHANGE COLUMN * RENAME TABLE * SET TB
......................................................................
Patch Set 1:
Iceberg partition related DDL api is on progress now, but not finished yet, more details please refer: https://github.com/apache/iceberg/pull/922/files. So we forbidden these operation in this patch, maybe implement in another patch.
--
To view, visit http://gerrit.cloudera.org:8080/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 1
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Fri, 16 Oct 2020 02:29:23 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10166 ALTER TABLE for Iceberg tables
Posted by "wangsheng (Code Review)" <ge...@cloudera.org>.
wangsheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 ALTER TABLE for Iceberg tables
......................................................................
Patch Set 3:
> (1 comment)
Sorry I didn't make it clear. I mean 'RENAME TABLE' here, not rename column. According to your advice, I will keep ADD COLUMNS/RENAME TABLE/SET TBL_PROPERTIES in this patch.
--
To view, visit http://gerrit.cloudera.org:8080/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 3
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Tue, 20 Oct 2020 14:26:48 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
Posted by "wangsheng (Code Review)" <ge...@cloudera.org>.
wangsheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
......................................................................
Patch Set 10:
> (1 comment)
Thanks for you suggestion, Gabor. I've already rename tables to related situations in iceberg-negative.test
--
To view, visit http://gerrit.cloudera.org:8080/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 10
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Fri, 30 Oct 2020 08:05:53 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10166 ALTER TABLE for Iceberg tables This patch mainly implement ALTER TABLE for Iceberg tables, we currently supported these statements: * ADD COLUMNS * DROP COLUMN * REPLACE COLUMNS * ALTER COLUMN CHANGE COLUMN * RENAME TABLE * SET TB
Posted by "wangsheng (Code Review)" <ge...@cloudera.org>.
Hello Gabor Kaszab, Zoltan Borok-Nagy, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16606
to look at the new patch set (#2).
Change subject: IMPALA-10166 ALTER TABLE for Iceberg tables This patch mainly implement ALTER TABLE for Iceberg tables, we currently supported these statements: * ADD COLUMNS * DROP COLUMN * REPLACE COLUMNS * ALTER COLUMN CHANGE COLUMN * RENAME TABLE * SET TB
......................................................................
IMPALA-10166 ALTER TABLE for Iceberg tables
This patch mainly implement ALTER TABLE for Iceberg tables, we
currently supported these statements:
* ADD COLUMNS
* DROP COLUMN
* REPLACE COLUMNS
* ALTER COLUMN CHANGE COLUMN
* RENAME TABLE
* SET TBL_PROPERTIES
But here something we need to pay attention:
1.We cannot use complex type for CHANGE COLUMN/REPLACE COLUMNS
2.RENAME TABLE is not supported for HadoopCatalog/HadoopTables
3.We cannot ADD/DROP PARTITION now since Iceberg no related api
Testing:
- Iceberg table alter test in test_iceberg.py
- Iceberg table negative test in test_scanners.py
Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
---
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAlterColStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableRecoverPartitionsStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableReplaceColsStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetFileFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetLocationStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCatalog.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopCatalog.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopTables.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/IcebergUtil.java
A testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M tests/query_test/test_iceberg.py
19 files changed, 569 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/16606/2
--
To view, visit http://gerrit.cloudera.org:8080/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 2
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
......................................................................
Patch Set 9:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/7587/ : 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/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 9
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Fri, 30 Oct 2020 02:58:11 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
......................................................................
Patch Set 5:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/16606/5/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopCatalog.java
File fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopCatalog.java:
http://gerrit.cloudera.org:8080/#/c/16606/5/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopCatalog.java@87
PS5, Line 87: TableIdentifier oldTableId = IcebergUtil.getIcebergTableIdentifier(feTable);
: hadoopCatalog.renameTable(oldTableId, newTableId);
I think we should catch the UnsupportedException being thrown by Iceberg, and throw a similar exception we throw in IcebergHadoopTables.
Therefore if HadoopCatalog will support renaming in the future then this method will succeed. Until that we will have a better error message.
http://gerrit.cloudera.org:8080/#/c/16606/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/16606/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3388
PS4, Line 3388: .ICEBERG
> It's actually the 'new' metastore table. We have deep copied it to safely m
Seems like it's got forgotten.
http://gerrit.cloudera.org:8080/#/c/16606/4/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/16606/4/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@112
PS4, Line 112: * FLOAT -> DOUBLE
: * DECIMAL(s1,p1) -> DECIMAL(s1,p2), same scale, p1<=p2
: */
> For Iceberg, it will throw exception directly like this: Cannot change colu
I don't think we need to re-implement the checks, we just need to provide good error messages to the user.
--
To view, visit http://gerrit.cloudera.org:8080/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 5
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Thu, 22 Oct 2020 14:06:42 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10166 ALTER TABLE for Iceberg tables
Posted by "wangsheng (Code Review)" <ge...@cloudera.org>.
wangsheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 ALTER TABLE for Iceberg tables
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16606/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16606/3//COMMIT_MSG@11
PS3, Line 11: * ADD COLUMNS
: * DROP COLUMN
: * REPLACE COLUMNS
: * ALTER COLUMN CHANGE COLUMN
> Iceberg assigns a unique field id to each fields to support schema evolutio
Thanks Zoltan, it indeed a problem. If you insist supporting for ALTER TABLE statements that don't make the table unreadable to Impala. I will separate this patch to these sub-tasks:
1. Keeping ADD COLUMNS/RENAME/SET TBLPROPERTIES in this patch as part1. Maybe we can also including DROP COLUMN statement.
2. Implement a new option such as ‘ICEBERG_FIELD’ in TParquetFallbackSchemaResolution in another patch as part2, so we can use filed id to resolve columns.
3. After we finished the task2, we can supporting REPLACE/ALTER in a new patch as part3.
Maybe we can also merge task2 and task3 into one task. How do you think?
--
To view, visit http://gerrit.cloudera.org:8080/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 3
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Tue, 20 Oct 2020 12:45:59 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
Posted by "wangsheng (Code Review)" <ge...@cloudera.org>.
wangsheng has uploaded a new patch set (#10). ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
......................................................................
IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
This patch mainly implements ALTER TABLE for Iceberg
tables, we currently support these statements:
* ADD COLUMNS
* RENAME TABLE
* SET TBL_PROPERTIES
* SET OWNER
We forbid DROP COLUMN/REPLACE COLUMNS/ALTER COLUMN in this
patch, since these statemens may make Iceberg tables unreadable.
We may support column resolution by field id in the near future,
after that, we will support COLUMN/REPLACE COLUMNS/ALTER COLUMN
for Iceberg tables.
Here something we still need to pay attention:
1.RENAME TABLE is not supported for HadoopCatalog/HadoopTables,
even if we already implement 'RENAME TABLE' statement, so we
only rename the table in the Hive Metastore for external table.
2.We cannot ADD/DROP PARTITION now since there is no API for that
in Iceberg, but related work is already in progess in Iceberg.
Testing:
- Iceberg table alter test in test_iceberg.py
- Iceberg table negative test in test_scanners.py
- Rename tables in iceberg-negative.test
Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
---
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAlterColStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableRecoverPartitionsStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetFileFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetLocationStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCatalog.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopCatalog.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopTables.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/IcebergUtil.java
A testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M tests/query_test/test_iceberg.py
18 files changed, 497 insertions(+), 22 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/16606/10
--
To view, visit http://gerrit.cloudera.org:8080/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 10
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
......................................................................
Patch Set 11: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 11
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Fri, 30 Oct 2020 14:03:28 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
Posted by "wangsheng (Code Review)" <ge...@cloudera.org>.
wangsheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
......................................................................
Patch Set 6:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/16606/5/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopCatalog.java
File fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopCatalog.java:
http://gerrit.cloudera.org:8080/#/c/16606/5/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopCatalog.java@87
PS5, Line 87: TableIdentifier oldTableId = IcebergUtil.getIcebergTableIdentifier(feTable);
: try {
> I think we should catch the UnsupportedException being thrown by Iceberg, a
Done
http://gerrit.cloudera.org:8080/#/c/16606/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/16606/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3388
PS4, Line 3388: .ICEBERG
> It's actually the 'new' metastore table. We have deep copied it to safely m
In fact, I don't quite understand where the problem is. I write this method according to renameManagedKuduTable. It seem that both these two methods only update 'oldMsTbl' properties.
Do you mean we cannot transfom new 'oldMsTbl' to HMS by getHiveClient().alter_table()?
Besides, we cannot rename managed table of HadoopTables and HadoopCatalog, so I cannot test this situation yet.
http://gerrit.cloudera.org:8080/#/c/16606/4/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/16606/4/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@112
PS4, Line 112: * FLOAT -> DOUBLE
: * DECIMAL(s1,p1) -> DECIMAL(s1,p2), same scale, p1<=p2
: */
> I don't think we need to re-implement the checks, we just need to provide g
Ok, I would consider this when supporting alter column in a later patch!
--
To view, visit http://gerrit.cloudera.org:8080/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 6
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Thu, 22 Oct 2020 14:42:10 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
Posted by "wangsheng (Code Review)" <ge...@cloudera.org>.
wangsheng has uploaded a new patch set (#9). ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
......................................................................
IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
This patch mainly implements ALTER TABLE for Iceberg
tables, we currently support these statements:
* ADD COLUMNS
* RENAME TABLE
* SET TBL_PROPERTIES
* SET OWNER
We forbid DROP COLUMN/REPLACE COLUMNS/ALTER COLUMN in this
patch, since these statemens may make Iceberg tables unreadable.
We may support column resolution by field id in the near future,
after that, we will support COLUMN/REPLACE COLUMNS/ALTER COLUMN
for Iceberg tables.
Here something we still need to pay attention:
1.RENAME TABLE is not supported for HadoopCatalog/HadoopTables,
even if we already implement 'RENAME TABLE' statement, so we
only rename the table in the Hive Metastore for external table.
2.We cannot ADD/DROP PARTITION now since there is no API for that
in Iceberg, but related work is already in progess in Iceberg.
Testing:
- Iceberg table alter test in test_iceberg.py
- Iceberg table negative test in test_scanners.py
Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
---
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAlterColStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableRecoverPartitionsStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetFileFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetLocationStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCatalog.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopCatalog.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopTables.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/IcebergUtil.java
A testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M tests/query_test/test_iceberg.py
18 files changed, 486 insertions(+), 11 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/16606/9
--
To view, visit http://gerrit.cloudera.org:8080/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 9
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
......................................................................
Patch Set 9:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16606/8/testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
File testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test:
http://gerrit.cloudera.org:8080/#/c/16606/8/testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test@117
PS8, Line 117: ALTER TABLE iceberg_test3 RENAME TO iceberg_test3_new;
> I can use 'iceberg_test3' to replace 'iceberg_test9', since 'iceberg_test3'
Well, not tightly related to this patch but the naming of the tables in this test is not ideal in my opinion. Instead of having a counter in the name they should express something from their characteristics and purpose. e.g. iceberg_table_hadoop_catalog, iceberg_table_partitioned e.g.
It's hard to figure out for a later reader what the purpose of e.g. iceberg_test3 was.
It's up to you if you decide to rename the ones that are touched by this test or we can create a ticket for refactoring later.
--
To view, visit http://gerrit.cloudera.org:8080/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 9
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Fri, 30 Oct 2020 07:49:08 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
......................................................................
Patch Set 10:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/7589/ : 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/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 10
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Fri, 30 Oct 2020 08:25:18 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10166 ALTER TABLE for Iceberg tables
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 ALTER TABLE for Iceberg tables
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16606/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16606/3//COMMIT_MSG@11
PS3, Line 11: * ADD COLUMNS
: * DROP COLUMN
: * REPLACE COLUMNS
: * ALTER COLUMN CHANGE COLUMN
> Thanks Zoltan, it indeed a problem. If you insist supporting for ALTER TABL
Yeah, it makes sense to me.
I think we cannot support both RENAME and DROP because if those are combined, we cannot find the affected columns by name, neither by position.
DROP also interacts weirdly with ADD COLUMN. E.g. DROP + ADD column with the same name is a troublesome edge case.
So I think we should add support for DROP after step 2.
--
To view, visit http://gerrit.cloudera.org:8080/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 3
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Tue, 20 Oct 2020 13:17:02 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
Posted by "wangsheng (Code Review)" <ge...@cloudera.org>.
wangsheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
......................................................................
Patch Set 9:
(4 comments)
Thanks for review!
http://gerrit.cloudera.org:8080/#/c/16606/8//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16606/8//COMMIT_MSG@9
PS8, Line 9: implement
> nit: implements
Done
http://gerrit.cloudera.org:8080/#/c/16606/8/fe/src/main/java/org/apache/impala/analysis/AlterTableReplaceColsStmt.java
File fe/src/main/java/org/apache/impala/analysis/AlterTableReplaceColsStmt.java:
http://gerrit.cloudera.org:8080/#/c/16606/8/fe/src/main/java/org/apache/impala/analysis/AlterTableReplaceColsStmt.java@25
PS8, Line 25: import org.apache.impala.catalog.FeKuduTable;
> Is this import needed?
Done
http://gerrit.cloudera.org:8080/#/c/16606/8/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/16606/8/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@118
PS8, Line 118: support
> nit: supports
Done
http://gerrit.cloudera.org:8080/#/c/16606/8/testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
File testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test:
http://gerrit.cloudera.org:8080/#/c/16606/8/testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test@117
PS8, Line 117: ALTER TABLE iceberg_test3 RENAME TO iceberg_test3_new;
> Instead of creating a new table here, could you re-use any of the ones abov
I can use 'iceberg_test3' to replace 'iceberg_test9', since 'iceberg_test3' is a normal 'hadoop.tables' table. But iceberg_test10 is necessary here, since we don't create a normal 'hadoop.catalog' table above.
So I replace 'iceberg_test9' by 'iceberg_test3', and rename 'iceberg_test10' to 'iceberg_test9'.
Shall we can reuse these table name, such as 'iceberg_test1' to replace 'iceberg_test10'?
--
To view, visit http://gerrit.cloudera.org:8080/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 9
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Fri, 30 Oct 2020 03:00:02 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10166 ALTER TABLE for Iceberg tables
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 ALTER TABLE for Iceberg tables
......................................................................
Patch Set 3:
(1 comment)
Will take a deeper look tomorrow, but I had a comment about column-level modifications.
http://gerrit.cloudera.org:8080/#/c/16606/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16606/3//COMMIT_MSG@11
PS3, Line 11: * ADD COLUMNS
: * DROP COLUMN
: * REPLACE COLUMNS
: * ALTER COLUMN CHANGE COLUMN
Iceberg assigns a unique field id to each fields to support schema evolution. E.g. if we rename a column it'll have the same field id, and we should lookup the column with the help of the field id.
The problem is that Impala doesn't use the field id to resolve columns, so it might become unable to read the table after schema update.
Currently Impala resolves columns by either name:
https://github.com/apache/impala/blob/c7f118a860af6b811e2e2c4c5e3693f43429def8/be/src/exec/parquet/parquet-metadata-utils.cc#L715
Or by position:
https://github.com/apache/impala/blob/c7f118a860af6b811e2e2c4c5e3693f43429def8/be/src/exec/parquet/parquet-metadata-utils.cc#L747
So I think if we want to support altering the columns, we need to support schema resolution by field id as well. In my INSERT patch I introduced IcebergColumn in the Frontend which has a field id, but in that patch it is only for top-level columns, i.e. we don't store the field id for the nested fields.
I'm also OK if we only add support for ALTER TABLE statements that don't make the table unreadable to Impala. E.g. ADD COLUMNS should be fine.
--
To view, visit http://gerrit.cloudera.org:8080/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 3
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Mon, 19 Oct 2020 17:57:29 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/7515/ : 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/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 5
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Thu, 22 Oct 2020 13:26:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
......................................................................
Patch Set 8:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/7560/ : 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/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 8
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Tue, 27 Oct 2020 02:18:13 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
Posted by "wangsheng (Code Review)" <ge...@cloudera.org>.
wangsheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
......................................................................
Patch Set 5:
(12 comments)
Hi Zoltan, thanks for review. I've already removed 'REPLACE COLUMNS' related code in this patch.
http://gerrit.cloudera.org:8080/#/c/16606/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16606/4//COMMIT_MSG@10
PS4, Line 10: support t
> nit: support
Done
http://gerrit.cloudera.org:8080/#/c/16606/4//COMMIT_MSG@15
PS4, Line 15: forbid DR
> nit: forbid
Done
http://gerrit.cloudera.org:8080/#/c/16606/4//COMMIT_MSG@16
PS4, Line 16: make I
> nit: make
Done
http://gerrit.cloudera.org:8080/#/c/16606/4//COMMIT_MSG@17
PS4, Line 17: column resolution by field
> column resolution by field id
Done
http://gerrit.cloudera.org:8080/#/c/16606/4//COMMIT_MSG@23
PS4, Line 23: ,
> So we only rename the table in the Hive Metastore.
Done
http://gerrit.cloudera.org:8080/#/c/16606/4//COMMIT_MSG@24
PS4, Line 24: etastore for external table.
> since there is no API for that in Iceberg
Done
http://gerrit.cloudera.org:8080/#/c/16606/4/fe/src/main/java/org/apache/impala/analysis/AlterTableAlterColStmt.java
File fe/src/main/java/org/apache/impala/analysis/AlterTableAlterColStmt.java:
http://gerrit.cloudera.org:8080/#/c/16606/4/fe/src/main/java/org/apache/impala/analysis/AlterTableAlterColStmt.java@185
PS4, Line 185: is not supported for complex types in Iceberg
> is not supported for complex types in Iceberg tables.
Done
http://gerrit.cloudera.org:8080/#/c/16606/4/fe/src/main/java/org/apache/impala/analysis/AlterTableReplaceColsStmt.java
File fe/src/main/java/org/apache/impala/analysis/AlterTableReplaceColsStmt.java:
http://gerrit.cloudera.org:8080/#/c/16606/4/fe/src/main/java/org/apache/impala/analysis/AlterTableReplaceColsStmt.java@93
PS4, Line 93: params.setAlter_type(TAlterTableType.REPLACE_COLUMNS);
: TAlterTableReplaceColsParams colParams = new
> I thought we don't want to support REPLACE COLUMNS in this patch at all.
Done
http://gerrit.cloudera.org:8080/#/c/16606/4/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/16606/4/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java@98
PS4, Line 98: if (getTargetTable() instanceof FeKuduTable) {
: analyzeKuduTable(analyzer);
> The coding convention is to use braces for multi-line statements, i.e. turn
Done
http://gerrit.cloudera.org:8080/#/c/16606/4/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopTables.java
File fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopTables.java:
http://gerrit.cloudera.org:8080/#/c/16606/4/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopTables.java@100
PS4, Line 100:
> Cannot rename Iceberg tables that use 'hadoop.tables' as catalog.
Done
http://gerrit.cloudera.org:8080/#/c/16606/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/16606/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@960
PS4, Line 960:
> nit: // slashes should be indented
Done
http://gerrit.cloudera.org:8080/#/c/16606/4/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/16606/4/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@112
PS4, Line 112: * FLOAT -> DOUBLE
: * DECIMAL(s1,p1) -> DECIMAL(s1,p2), same scale, p1<=p2
: */
> Do we have checks for these? What does Iceberg do in cases when we violate
For Iceberg, it will throw exception directly like this: Cannot change column type: level: string -> int.
So I didn't add check here, we can catch this exception by current code. If you insist, I will try to add some checks in a later patch.
--
To view, visit http://gerrit.cloudera.org:8080/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 5
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Thu, 22 Oct 2020 13:07:14 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
......................................................................
Patch Set 6: Code-Review+1
(1 comment)
Thanks for your response, LGTM! I'm only giving it +1 to let others take a look as well.
http://gerrit.cloudera.org:8080/#/c/16606/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/16606/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3388
PS4, Line 3388: .ICEBERG
> In fact, I don't quite understand where the problem is. I write this method
It was just a nitpick comment, but forgot to add 'nit'.
Nothing is wrong here, only the name doesn't really fit. And I think it also doesn't really fit for renameManagedKuduTable() (but no need to change it there).
My suggestion was only to rename the variable to 'msTbl'. I'm also OK with keeping the original name, I just noticed that there was no reply to this comment.
--
To view, visit http://gerrit.cloudera.org:8080/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 6
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Thu, 22 Oct 2020 15:00:12 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
Posted by "wangsheng (Code Review)" <ge...@cloudera.org>.
wangsheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
......................................................................
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16606/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/16606/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3388
PS4, Line 3388: EBERG_TA
> It was just a nitpick comment, but forgot to add 'nit'.
Done, rename 'oldMsTbl' to 'msTbl'
--
To view, visit http://gerrit.cloudera.org:8080/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 7
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Thu, 22 Oct 2020 15:16:25 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
Posted by "wangsheng (Code Review)" <ge...@cloudera.org>.
wangsheng has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
......................................................................
IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
This patch mainly implement ALTER TABLE for Iceberg
tables, we currently support these statements:
* ADD COLUMNS
* RENAME TABLE
* SET TBL_PROPERTIES
* SET OWNER
We forbid DROP COLUMN/REPLACE COLUMNS/ALTER COLUMN in this
patch, since these statemens may make Iceberg tables unreadable.
We may support column resolution by field id in the near future,
after that, we will support COLUMN/REPLACE COLUMNS/ALTER COLUMN
for Iceberg tables.
Here something we still need to pay attention:
1.RENAME TABLE is not supported for HadoopCatalog/HadoopTables,
even if we already implement 'RENAME TABLE' statement, so we
only rename the table in the Hive Metastore for external table.
2.We cannot ADD/DROP PARTITION now since there is no API for that
in Iceberg, but related work is already in progess in Iceberg.
Testing:
- Iceberg table alter test in test_iceberg.py
- Iceberg table negative test in test_scanners.py
Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
---
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAlterColStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableRecoverPartitionsStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableReplaceColsStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetFileFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetLocationStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCatalog.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopCatalog.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopTables.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/IcebergUtil.java
A testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M tests/query_test/test_iceberg.py
19 files changed, 493 insertions(+), 11 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/16606/6
--
To view, visit http://gerrit.cloudera.org:8080/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 6
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
[Impala-ASF-CR] IMPALA-10166 ALTER TABLE for Iceberg tables This patch mainly implement ALTER TABLE for Iceberg tables, we currently supported these statements: * ADD COLUMNS * DROP COLUMN * REPLACE COLUMNS * ALTER COLUMN CHANGE COLUMN * RENAME TABLE * SET TB
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 ALTER TABLE for Iceberg tables This patch mainly implement ALTER TABLE for Iceberg tables, we currently supported these statements: * ADD COLUMNS * DROP COLUMN * REPLACE COLUMNS * ALTER COLUMN CHANGE COLUMN * RENAME TABLE * SET TB
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/7468/ : 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/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 1
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Fri, 16 Oct 2020 02:44:18 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10166 ALTER TABLE for Iceberg tables
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 ALTER TABLE for Iceberg tables
......................................................................
Patch Set 3:
Thanks for the clarification. Seems good to me!
--
To view, visit http://gerrit.cloudera.org:8080/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 3
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Tue, 20 Oct 2020 14:41:33 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
......................................................................
Patch Set 11: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 11
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Fri, 30 Oct 2020 08:32:26 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
......................................................................
IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
This patch mainly implements ALTER TABLE for Iceberg
tables, we currently support these statements:
* ADD COLUMNS
* RENAME TABLE
* SET TBL_PROPERTIES
* SET OWNER
We forbid DROP COLUMN/REPLACE COLUMNS/ALTER COLUMN in this
patch, since these statemens may make Iceberg tables unreadable.
We may support column resolution by field id in the near future,
after that, we will support COLUMN/REPLACE COLUMNS/ALTER COLUMN
for Iceberg tables.
Here something we still need to pay attention:
1.RENAME TABLE is not supported for HadoopCatalog/HadoopTables,
even if we already implement 'RENAME TABLE' statement, so we
only rename the table in the Hive Metastore for external table.
2.We cannot ADD/DROP PARTITION now since there is no API for that
in Iceberg, but related work is already in progess in Iceberg.
Testing:
- Iceberg table alter test in test_iceberg.py
- Iceberg table negative test in test_scanners.py
- Rename tables in iceberg-negative.test
Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Reviewed-on: http://gerrit.cloudera.org:8080/16606
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAlterColStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableRecoverPartitionsStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetFileFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetLocationStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCatalog.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopCatalog.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopTables.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/IcebergUtil.java
A testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M tests/query_test/test_iceberg.py
18 files changed, 497 insertions(+), 22 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 12
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/7502/ : 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/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 4
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Wed, 21 Oct 2020 03:54:45 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
Posted by "wangsheng (Code Review)" <ge...@cloudera.org>.
wangsheng has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
......................................................................
IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
This patch mainly implement ALTER TABLE for Iceberg
tables, we currently support these statements:
* ADD COLUMNS
* RENAME TABLE
* SET TBL_PROPERTIES
* SET OWNER
We forbid DROP COLUMN/REPLACE COLUMNS/ALTER COLUMN in this
patch, since these statemens may make Iceberg tables unreadable.
We may support column resolution by field id in the near future,
after that, we will support COLUMN/REPLACE COLUMNS/ALTER COLUMN
for Iceberg tables.
Here something we still need to pay attention:
1.RENAME TABLE is not supported for HadoopCatalog/HadoopTables,
even if we already implement 'RENAME TABLE' statement, so we
only rename the table in the Hive Metastore for external table.
2.We cannot ADD/DROP PARTITION now since there is no API for that
in Iceberg, but related work is already in progess in Iceberg.
Testing:
- Iceberg table alter test in test_iceberg.py
- Iceberg table negative test in test_scanners.py
Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
---
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAlterColStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableRecoverPartitionsStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableReplaceColsStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetFileFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetLocationStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCatalog.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopCatalog.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopTables.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/IcebergUtil.java
A testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M tests/query_test/test_iceberg.py
19 files changed, 492 insertions(+), 11 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/16606/8
--
To view, visit http://gerrit.cloudera.org:8080/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 8
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
[Impala-ASF-CR] IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16606 )
Change subject: IMPALA-10166 (part 1): ALTER TABLE for Iceberg tables
......................................................................
Patch Set 7: Code-Review+1
Thanks!
--
To view, visit http://gerrit.cloudera.org:8080/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 7
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Thu, 22 Oct 2020 15:17:44 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10166 ALTER TABLE for Iceberg tables
Posted by "wangsheng (Code Review)" <ge...@cloudera.org>.
Hello Gabor Kaszab, Zoltan Borok-Nagy, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16606
to look at the new patch set (#3).
Change subject: IMPALA-10166 ALTER TABLE for Iceberg tables
......................................................................
IMPALA-10166 ALTER TABLE for Iceberg tables
This patch mainly implement ALTER TABLE for Iceberg tables, we
currently supported these statements:
* ADD COLUMNS
* DROP COLUMN
* REPLACE COLUMNS
* ALTER COLUMN CHANGE COLUMN
* RENAME TABLE
* SET TBL_PROPERTIES
But here something we need to pay attention:
1.We cannot use complex type for CHANGE COLUMN/REPLACE COLUMNS
2.RENAME TABLE is not supported for HadoopCatalog/HadoopTables
3.We cannot ADD/DROP PARTITION now since Iceberg no related api
Testing:
- Iceberg table alter test in test_iceberg.py
- Iceberg table negative test in test_scanners.py
Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
---
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAlterColStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableRecoverPartitionsStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableReplaceColsStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetFileFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetLocationStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCatalog.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopCatalog.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopTables.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/IcebergUtil.java
A testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M tests/query_test/test_iceberg.py
19 files changed, 569 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/16606/3
--
To view, visit http://gerrit.cloudera.org:8080/16606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5104cc47c7b42dacdb52983f503cd263135d6bfc
Gerrit-Change-Number: 16606
Gerrit-PatchSet: 3
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>