You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org> on 2023/04/20 16:25:15 UTC
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
Zoltan Borok-Nagy has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19776
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
This patch adds support for DELETE statements on unpartitioned Iceberg
tables. Impala uses the 'merge-on-read' mode with position delete files.
The patch reuses the existing IcebergPositionDeleteTable as the target
table of the DELETE statements, because this table already has the same
schema as position delete files, even with correct Iceberg field IDs.
The patch basically rewrites DELETE statements to INSERT statements,
e.g.:
from:
DELETE FROM ice_t WHERE id = 42;
to:
INSERT INTO ice_t-POSITION-DELETE
SELECT INPUT__FILE__NAME, FILE__POSITION
FROM ice_t
WHERE id = 42;
Position delete files need to be ordered by (file_position, pos), so
we add an extra SORT node before the table sink operator.
In the backend the patch adds a new table sink operator, the
IcebergDeleteSink. It writes the incoming rows (file_path, position) to
delete files. It reuses a lot of code from HdfsTableSink, so this patch
moves the common code to the new common base class: TableSinkBase.
The coordinator then collects the written delete files and invokes
UpdateCatalog to finalize the DELETE statement.
The Catalog then uses Iceberg APIs to create a new snapshot with the
created delete files. It also validates that there was no conflicting
data files written since the operation started.
Testing:
* added planer test
* e2e tests
* interop test between Impala and Hive
Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
---
M be/src/exec/CMakeLists.txt
M be/src/exec/data-sink.cc
M be/src/exec/file-metadata-utils.cc
M be/src/exec/hdfs-table-sink.cc
M be/src/exec/hdfs-table-sink.h
M be/src/exec/hdfs-table-writer.cc
M be/src/exec/hdfs-table-writer.h
A be/src/exec/iceberg-delete-sink.cc
A be/src/exec/iceberg-delete-sink.h
M be/src/exec/parquet/hdfs-parquet-table-writer.cc
M be/src/exec/parquet/hdfs-parquet-table-writer.h
A be/src/exec/table-sink-base.cc
A be/src/exec/table-sink-base.h
M be/src/runtime/coordinator.cc
M be/src/runtime/descriptors.cc
M be/src/service/client-request-state.cc
M common/thrift/CatalogObjects.thrift
M common/thrift/CatalogService.thrift
M common/thrift/DataSinks.thrift
M common/thrift/Query.thrift
M common/thrift/Types.thrift
M fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
M fe/src/main/java/org/apache/impala/analysis/IcebergPartitionSpec.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergPositionDeleteTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/VirtualColumn.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/TableSink.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-delete.test
A testdata/workloads/functional-query/queries/QueryTest/iceberg-delete.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M tests/query_test/test_iceberg.py
39 files changed, 1,561 insertions(+), 439 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/76/19776/1
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 10:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9352/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 10
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 25 May 2023 15:27:44 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 2: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/9259/
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 27 Apr 2023 13:42:26 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19776/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19776/1//COMMIT_MSG@19
PS1, Line 19: from:
: DELETE FROM ice_t WHERE id = 42;
:
: to:
: INSERT INTO ice_t-POSITION-DELETE
: SELECT INPUT__FILE__NAME, FILE__POSITION
: FROM ice_t
: WHERE id = 42;
> This is a very important statement, thanks for your work Zoltan.
Thanks Penglin for taking a look.
I have a design doc about DELETE + UPDATE: https://docs.google.com/document/d/1GuRiJ3jjqkwINsSCKYaWwcfXHzbMrsd3WEMDOB11Xqw/edit#heading=h.5bmfhbmb4qdk
So my plan is to reuse as much as I can of this work when I'm implementing the UPDATE.
There will be some differences when I'll implement it for partitioned tables, because partition evolution introduces additional complexities. I'll update the design doc with the final solution once we have it implemented.
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 27 Apr 2023 13:01:50 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 1:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/12828/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Comment-Date: Thu, 20 Apr 2023 16:38:29 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
lipenglin@apache.org has posted comments on this change. ( http://gerrit.cloudera.org:8080/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19776/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19776/1//COMMIT_MSG@19
PS1, Line 19: from:
: DELETE FROM ice_t WHERE id = 42;
:
: to:
: INSERT INTO ice_t-POSITION-DELETE
: SELECT INPUT__FILE__NAME, FILE__POSITION
: FROM ice_t
: WHERE id = 42;
This is a very important statement, thanks for your work Zoltan.
Sorry I only did a review of the algorithm process of the entire implementation and haven't done a detailed code review yet, because I currently have very little working time on Impala, so.
For just implementing the "delete statement", I think this is a very good implementation solution, but I realize that Flink or Spark have already implemented the "update statement". If we also need to implement this feature in the future, my understanding is that "update = delete + insert", so I wonder if the implementation approach of this patch can be reused and helpful for the implementation of "update statement"? If so, it will be easy to continue iterating features in the future!
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Comment-Date: Mon, 24 Apr 2023 13:03:44 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 8:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13102/ : 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/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 8
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 May 2023 11:14:11 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Hello Andrew Sherman, Tamas Mate, Gabor Kaszab, lipenglin@apache.org, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19776
to look at the new patch set (#10).
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
This patch adds support for DELETE statements on unpartitioned Iceberg
tables. Impala uses the 'merge-on-read' mode with position delete files.
The patch reuses the existing IcebergPositionDeleteTable as the target
table of the DELETE statements, because this table already has the same
schema as position delete files, even with correct Iceberg field IDs.
The patch basically rewrites DELETE statements to INSERT statements,
e.g.:
from:
DELETE FROM ice_t WHERE id = 42;
to:
INSERT INTO ice_t-POSITION-DELETE
SELECT INPUT__FILE__NAME, FILE__POSITION
FROM ice_t
WHERE id = 42;
Position delete files need to be ordered by (file_position, pos), so
we add an extra SORT node before the table sink operator.
In the backend the patch adds a new table sink operator, the
IcebergDeleteSink. It writes the incoming rows (file_path, position) to
delete files. It reuses a lot of code from HdfsTableSink, so this patch
moves the common code to the new common base class: TableSinkBase.
The coordinator then collects the written delete files and invokes
UpdateCatalog to finalize the DELETE statement.
The Catalog then uses Iceberg APIs to create a new snapshot with the
created delete files. It also validates that there was no conflicting
data files written since the operation started.
Testing:
* added planer test
* e2e tests
* interop test between Impala and Hive
Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
---
M be/src/exec/CMakeLists.txt
M be/src/exec/data-sink.cc
M be/src/exec/file-metadata-utils.cc
M be/src/exec/hdfs-table-sink.cc
M be/src/exec/hdfs-table-sink.h
M be/src/exec/hdfs-table-writer.cc
M be/src/exec/hdfs-table-writer.h
A be/src/exec/iceberg-delete-sink.cc
A be/src/exec/iceberg-delete-sink.h
M be/src/exec/parquet/hdfs-parquet-table-writer.cc
M be/src/exec/parquet/hdfs-parquet-table-writer.h
A be/src/exec/table-sink-base.cc
A be/src/exec/table-sink-base.h
M be/src/runtime/coordinator.cc
M be/src/runtime/descriptors.cc
M be/src/service/client-request-state.cc
M common/thrift/CatalogObjects.thrift
M common/thrift/CatalogService.thrift
M common/thrift/DataSinks.thrift
M common/thrift/Query.thrift
M common/thrift/Types.thrift
M fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
M fe/src/main/java/org/apache/impala/analysis/IcebergPartitionSpec.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergPositionDeleteTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/VirtualColumn.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
A fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/TableSink.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeModifyStmtsTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-delete.test
A testdata/workloads/functional-query/queries/QueryTest/iceberg-delete-complex.test
A testdata/workloads/functional-query/queries/QueryTest/iceberg-delete.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M tests/query_test/test_iceberg.py
44 files changed, 1,820 insertions(+), 448 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/76/19776/10
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 10
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 5:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9283/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 May 2023 14:49:41 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 6: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/9305/
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 May 2023 20:09:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 2:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/12880/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 27 Apr 2023 13:09:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 6:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9305/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 May 2023 14:57:37 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Hello Andrew Sherman, Tamas Mate, Gabor Kaszab, lipenglin@apache.org, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19776
to look at the new patch set (#12).
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
This patch adds support for DELETE statements on unpartitioned Iceberg
tables. Impala uses the 'merge-on-read' mode with position delete files.
The patch reuses the existing IcebergPositionDeleteTable as the target
table of the DELETE statements, because this table already has the same
schema as position delete files, even with correct Iceberg field IDs.
The patch basically rewrites DELETE statements to INSERT statements,
e.g.:
from:
DELETE FROM ice_t WHERE id = 42;
to:
INSERT INTO ice_t-POSITION-DELETE
SELECT INPUT__FILE__NAME, FILE__POSITION
FROM ice_t
WHERE id = 42;
Position delete files need to be ordered by (file_path, pos), so
we add an extra SORT node before the table sink operator.
In the backend the patch adds a new table sink operator, the
IcebergDeleteSink. It writes the incoming rows (file_path, position) to
delete files. It reuses a lot of code from HdfsTableSink, so this patch
moves the common code to the new common base class: TableSinkBase.
The coordinator then collects the written delete files and invokes
UpdateCatalog to finalize the DELETE statement.
The Catalog then uses Iceberg APIs to create a new snapshot with the
created delete files. It also validates that there was no conflicting
data files written since the operation started.
Testing:
* added planer test
* e2e tests
* interop test between Impala and Hive
Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
---
M be/src/exec/CMakeLists.txt
M be/src/exec/data-sink.cc
M be/src/exec/file-metadata-utils.cc
M be/src/exec/hdfs-table-sink.cc
M be/src/exec/hdfs-table-sink.h
M be/src/exec/hdfs-table-writer.cc
M be/src/exec/hdfs-table-writer.h
A be/src/exec/iceberg-delete-sink.cc
A be/src/exec/iceberg-delete-sink.h
M be/src/exec/parquet/hdfs-parquet-table-writer.cc
M be/src/exec/parquet/hdfs-parquet-table-writer.h
A be/src/exec/table-sink-base.cc
A be/src/exec/table-sink-base.h
M be/src/runtime/coordinator.cc
M be/src/runtime/descriptors.cc
M be/src/service/client-request-state.cc
M common/thrift/CatalogObjects.thrift
M common/thrift/CatalogService.thrift
M common/thrift/DataSinks.thrift
M common/thrift/Query.thrift
M common/thrift/Types.thrift
M fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
M fe/src/main/java/org/apache/impala/analysis/IcebergPartitionSpec.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergPositionDeleteTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/VirtualColumn.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
A fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/TableSink.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeModifyStmtsTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-delete.test
A testdata/workloads/functional-query/queries/QueryTest/iceberg-delete-complex.test
A testdata/workloads/functional-query/queries/QueryTest/iceberg-delete.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M tests/query_test/test_iceberg.py
44 files changed, 1,858 insertions(+), 465 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/76/19776/12
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 12
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 12:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13230/ : 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/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 12
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Jun 2023 08:29:09 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 11:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13227/ : 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/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 11
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Jun 2023 17:46:06 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 12: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 12
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 09 Jun 2023 00:15:22 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 9:
PS9 is a rebase
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 9
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 May 2023 12:23:23 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Hello Tamas Mate, Gabor Kaszab, lipenglin@apache.org, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19776
to look at the new patch set (#7).
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
This patch adds support for DELETE statements on unpartitioned Iceberg
tables. Impala uses the 'merge-on-read' mode with position delete files.
The patch reuses the existing IcebergPositionDeleteTable as the target
table of the DELETE statements, because this table already has the same
schema as position delete files, even with correct Iceberg field IDs.
The patch basically rewrites DELETE statements to INSERT statements,
e.g.:
from:
DELETE FROM ice_t WHERE id = 42;
to:
INSERT INTO ice_t-POSITION-DELETE
SELECT INPUT__FILE__NAME, FILE__POSITION
FROM ice_t
WHERE id = 42;
Position delete files need to be ordered by (file_position, pos), so
we add an extra SORT node before the table sink operator.
In the backend the patch adds a new table sink operator, the
IcebergDeleteSink. It writes the incoming rows (file_path, position) to
delete files. It reuses a lot of code from HdfsTableSink, so this patch
moves the common code to the new common base class: TableSinkBase.
The coordinator then collects the written delete files and invokes
UpdateCatalog to finalize the DELETE statement.
The Catalog then uses Iceberg APIs to create a new snapshot with the
created delete files. It also validates that there was no conflicting
data files written since the operation started.
Testing:
* added planer test
* e2e tests
* interop test between Impala and Hive
Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
---
M be/src/exec/CMakeLists.txt
M be/src/exec/data-sink.cc
M be/src/exec/file-metadata-utils.cc
M be/src/exec/hdfs-table-sink.cc
M be/src/exec/hdfs-table-sink.h
M be/src/exec/hdfs-table-writer.cc
M be/src/exec/hdfs-table-writer.h
A be/src/exec/iceberg-delete-sink.cc
A be/src/exec/iceberg-delete-sink.h
M be/src/exec/parquet/hdfs-parquet-table-writer.cc
M be/src/exec/parquet/hdfs-parquet-table-writer.h
A be/src/exec/table-sink-base.cc
A be/src/exec/table-sink-base.h
M be/src/runtime/coordinator.cc
M be/src/runtime/descriptors.cc
M be/src/service/client-request-state.cc
M common/thrift/CatalogObjects.thrift
M common/thrift/CatalogService.thrift
M common/thrift/DataSinks.thrift
M common/thrift/Query.thrift
M common/thrift/Types.thrift
M fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
M fe/src/main/java/org/apache/impala/analysis/IcebergPartitionSpec.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergPositionDeleteTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/VirtualColumn.java
A fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/TableSink.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeModifyStmtsTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-delete.test
A testdata/workloads/functional-query/queries/QueryTest/iceberg-delete.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M tests/query_test/test_iceberg.py
41 files changed, 1,699 insertions(+), 444 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/76/19776/7
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 7
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 5: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 May 2023 20:03:59 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 4: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/9263/
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 28 Apr 2023 16:45:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 8:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19776/8/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:
http://gerrit.cloudera.org:8080/#/c/19776/8/tests/query_test/test_iceberg.py@1200
PS8, Line 1200:
flake8: W292 no newline at end of file
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 8
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 May 2023 10:54:27 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 12:
Thanks everyone for the review!
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 12
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 13 Jun 2023 11:33:16 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 10:
(10 comments)
Excellent work, Zoli! Sorry, for the long delay with the review.
http://gerrit.cloudera.org:8080/#/c/19776/10/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
File fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java:
http://gerrit.cloudera.org:8080/#/c/19776/10/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java@467
PS10, Line 467: List<String> path = Path.createRawPath(targetTableRef_.getUniqueAlias(), k);
: SlotRef ref = new SlotRef(path);
: ref.analyze(analyzer);
: selectList.add(new SelectListItem(ref, null));
: uniqueSlots.add(ref.getSlotId());
: keySlots.add(ref.getSlotId());
: referencedColumns.add(colIndexMap.get(k));
For me it seems that the body of the for loop is identical between the Iceberg and Kudu implementations just the iterated items are different. Could you move this to a common place between the 2 implementations?
http://gerrit.cloudera.org:8080/#/c/19776/10/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
File fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java:
http://gerrit.cloudera.org:8080/#/c/19776/10/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java@340
PS10, Line 340: get
You can use the getOrDefault(..) method on properties() to simplify this function.
String writeFormat = ... properties().getOrDefault(DEFAULT_FILE_FORMAT, DEFAULT_FILE_FORMAT_DEFAULT);
return getIceergFormat(writeFormat);
http://gerrit.cloudera.org:8080/#/c/19776/10/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java@349
PS10, Line 349: getIcebergApiTable().properties().get(TableProperties.DELETE_DEFAULT_FILE_FORMAT);
You can use getOrDefault() here as well, where the default value is getWriteFileFormat().
http://gerrit.cloudera.org:8080/#/c/19776/10/fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java
File fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java:
http://gerrit.cloudera.org:8080/#/c/19776/10/fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java@28
PS10, Line 28: import org.slf4j.Logger;
Logger and LoggerFactory aren't used.
http://gerrit.cloudera.org:8080/#/c/19776/10/fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java@61
PS10, Line 61: long numBufferedPartitionsPerInstance = 1;
These variables can be final, if I'm not mistaken.
http://gerrit.cloudera.org:8080/#/c/19776/10/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/19776/10/fe/src/main/java/org/apache/impala/service/Frontend.java@2548
PS10, Line 2548: private static void addFinalizationParamsForDelete(
nit: I think the indentation is off in this function.
http://gerrit.cloudera.org:8080/#/c/19776/10/fe/src/main/java/org/apache/impala/service/Frontend.java@2555
PS10, Line 2555: TFinalizeParams finalizeParams = new TFinalizeParams();
: finalizeParams.setTable_name(targetTable.getTableName().getTbl());
: finalizeParams.setTable_id(DescriptorTable.TABLE_SINK_ID);
: String db = targetTable.getTableName().getDb();
: finalizeParams.setTable_db(db == null ? queryCtx.session.database : db);
: FeFsTable hdfsTable = (FeFsTable) targetTable;
: finalizeParams.setHdfs_base_dir(hdfsTable.getHdfsBaseDir());
This part is kind of the same as in addFinalizatonParamsForInsert(). Would it make sense to introduce a common function to populate the non-iceberg part?
http://gerrit.cloudera.org:8080/#/c/19776/10/testdata/workloads/functional-query/queries/QueryTest/iceberg-delete-complex.test
File testdata/workloads/functional-query/queries/QueryTest/iceberg-delete-complex.test:
http://gerrit.cloudera.org:8080/#/c/19776/10/testdata/workloads/functional-query/queries/QueryTest/iceberg-delete-complex.test@3
PS10, Line 3: # Delete from empty table is no-op.
I think this comment is not relevant here.
http://gerrit.cloudera.org:8080/#/c/19776/10/testdata/workloads/functional-query/queries/QueryTest/iceberg-delete-complex.test@90
PS10, Line 90: ====
Could you give it a try to include an UNNEST() fn call into the select list when querying the INPUT__FILE__NAME and also having delete files?
http://gerrit.cloudera.org:8080/#/c/19776/10/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:
http://gerrit.cloudera.org:8080/#/c/19776/10/tests/query_test/test_iceberg.py@1163
PS10, Line 1163: self.run_stmt_in_hive("SELECT * FROM {} ORDER BY i".format(ice_delete))
You run this 2 SELECTs twice. I think this line can be dropped.
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 10
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Jun 2023 07:43:52 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 3: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/9260/
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 27 Apr 2023 21:39:25 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Hello Tamas Mate, Gabor Kaszab, lipenglin@apache.org, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19776
to look at the new patch set (#5).
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
This patch adds support for DELETE statements on unpartitioned Iceberg
tables. Impala uses the 'merge-on-read' mode with position delete files.
The patch reuses the existing IcebergPositionDeleteTable as the target
table of the DELETE statements, because this table already has the same
schema as position delete files, even with correct Iceberg field IDs.
The patch basically rewrites DELETE statements to INSERT statements,
e.g.:
from:
DELETE FROM ice_t WHERE id = 42;
to:
INSERT INTO ice_t-POSITION-DELETE
SELECT INPUT__FILE__NAME, FILE__POSITION
FROM ice_t
WHERE id = 42;
Position delete files need to be ordered by (file_position, pos), so
we add an extra SORT node before the table sink operator.
In the backend the patch adds a new table sink operator, the
IcebergDeleteSink. It writes the incoming rows (file_path, position) to
delete files. It reuses a lot of code from HdfsTableSink, so this patch
moves the common code to the new common base class: TableSinkBase.
The coordinator then collects the written delete files and invokes
UpdateCatalog to finalize the DELETE statement.
The Catalog then uses Iceberg APIs to create a new snapshot with the
created delete files. It also validates that there was no conflicting
data files written since the operation started.
Testing:
* added planer test
* e2e tests
* interop test between Impala and Hive
Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
---
M be/src/exec/CMakeLists.txt
M be/src/exec/data-sink.cc
M be/src/exec/file-metadata-utils.cc
M be/src/exec/hdfs-table-sink.cc
M be/src/exec/hdfs-table-sink.h
M be/src/exec/hdfs-table-writer.cc
M be/src/exec/hdfs-table-writer.h
A be/src/exec/iceberg-delete-sink.cc
A be/src/exec/iceberg-delete-sink.h
M be/src/exec/parquet/hdfs-parquet-table-writer.cc
M be/src/exec/parquet/hdfs-parquet-table-writer.h
A be/src/exec/table-sink-base.cc
A be/src/exec/table-sink-base.h
M be/src/runtime/coordinator.cc
M be/src/runtime/descriptors.cc
M be/src/service/client-request-state.cc
M common/thrift/CatalogObjects.thrift
M common/thrift/CatalogService.thrift
M common/thrift/DataSinks.thrift
M common/thrift/Query.thrift
M common/thrift/Types.thrift
M fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
M fe/src/main/java/org/apache/impala/analysis/IcebergPartitionSpec.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergPositionDeleteTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/VirtualColumn.java
A fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/TableSink.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeModifyStmtsTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-delete.test
A testdata/workloads/functional-query/queries/QueryTest/iceberg-delete.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M tests/query_test/test_iceberg.py
41 files changed, 1,734 insertions(+), 448 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/76/19776/5
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 6:
(4 comments)
Great change Zoltan! Did a first round of review and testing, looks good, but I would like to do another round before giving a +1.
Just a few nits and found a corner case. I was able to target a virtual column with delete statement and received a NPE.
DELETE FROM ice_delete WHERE FILE__POSITION = 0;
http://gerrit.cloudera.org:8080/#/c/19776/6/fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
File fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java:
http://gerrit.cloudera.org:8080/#/c/19776/6/fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java@68
PS6, Line 68:
nit: empty line
http://gerrit.cloudera.org:8080/#/c/19776/6/fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java
File fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java:
http://gerrit.cloudera.org:8080/#/c/19776/6/fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java@36
PS6, Line 36: * Sink for deleting data from Iceberg tables.
nit: Could you extend this comment with a few sentence how iceberg handles deletes. Something like delete files will be inserted and these will be used in a join during scanning.
http://gerrit.cloudera.org:8080/#/c/19776/5/testdata/workloads/functional-query/queries/QueryTest/iceberg-delete.test
File testdata/workloads/functional-query/queries/QueryTest/iceberg-delete.test:
http://gerrit.cloudera.org:8080/#/c/19776/5/testdata/workloads/functional-query/queries/QueryTest/iceberg-delete.test@8
PS5, Line 8: Deelte
nit: Delete
http://gerrit.cloudera.org:8080/#/c/19776/6/testdata/workloads/functional-query/queries/QueryTest/iceberg-delete.test
File testdata/workloads/functional-query/queries/QueryTest/iceberg-delete.test:
http://gerrit.cloudera.org:8080/#/c/19776/6/testdata/workloads/functional-query/queries/QueryTest/iceberg-delete.test@8
PS6, Line 8: Deelte
nit: Delete
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 May 2023 09:24:51 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 7: Code-Review+1
(1 comment)
Thanks Zoltan!
LGTM!
http://gerrit.cloudera.org:8080/#/c/19776/7/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/19776/7/fe/src/main/java/org/apache/impala/service/Frontend.java@2571
PS7, Line 2571:
nit: indentation
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 7
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 May 2023 10:24:25 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
lipenglin@apache.org has posted comments on this change. ( http://gerrit.cloudera.org:8080/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 11:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19776/11//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19776/11//COMMIT_MSG@28
PS11, Line 28: file_position, pos
nit: maybe "file_path, position"?
Why sorting by these two columns:
DeleteData sorted by file_path and position can be found and loaded more easily without having to scan more files, improving query performance and subsequent compaction of datafile and deletefile.
Other than that, what other reason?
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 11
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Jun 2023 03:16:11 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 12: Code-Review+2
Thanks, Zoltan!
I guess this has been open for a while so no need to wait for others to give blessing.
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 12
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 13 Jun 2023 11:17:54 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 9: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/9344/
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 9
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 May 2023 17:46:39 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 11:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19776/11/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/19776/11/fe/src/main/java/org/apache/impala/service/Frontend.java@2597
PS11, Line 2597: private static TFinalizeParams addFinalizationParamsForDml(TQueryCtx queryCtx, FeTable targetTable,
line too long (101 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 11
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Jun 2023 17:24:31 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
lipenglin@apache.org has posted comments on this change. ( http://gerrit.cloudera.org:8080/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 11:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19776/11//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19776/11//COMMIT_MSG@28
PS11, Line 28: file_position, pos
> Thanks, fixed to 'file_path'.
Thanks! I got it.
"
Sorting by file_path allows filter pushdown by file in columnar storage formats.
Sorting by pos allows filtering rows while scanning, to avoid keeping deletes in memory.
"
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 11
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Jun 2023 12:43:14 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/19776/1/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:
http://gerrit.cloudera.org:8080/#/c/19776/1/tests/query_test/test_iceberg.py@1175
PS1, Line 1175: #
flake8: E116 unexpected indentation (comment)
http://gerrit.cloudera.org:8080/#/c/19776/1/tests/query_test/test_iceberg.py@1179
PS1, Line 1179:
flake8: W291 trailing whitespace
http://gerrit.cloudera.org:8080/#/c/19776/1/tests/query_test/test_iceberg.py@1179
PS1, Line 1179: assert hive_output == "_c0\n4799964\n"
line has trailing whitespace
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Apr 2023 16:26:06 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 6:
(11 comments)
Thanks for the comments.
http://gerrit.cloudera.org:8080/#/c/19776/5/common/thrift/CatalogObjects.thrift
File common/thrift/CatalogObjects.thrift:
http://gerrit.cloudera.org:8080/#/c/19776/5/common/thrift/CatalogObjects.thrift@81
PS5, Line 81: }
> I haven't found any usage for these new virtual columns. I'd bet they are f
Done
http://gerrit.cloudera.org:8080/#/c/19776/5/common/thrift/Query.thrift
File common/thrift/Query.thrift:
http://gerrit.cloudera.org:8080/#/c/19776/5/common/thrift/Query.thrift@827
PS5, Line 827: DML op
> It's not just INSERT anymore, right? (same below)
Done
http://gerrit.cloudera.org:8080/#/c/19776/5/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
File fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java:
http://gerrit.cloudera.org:8080/#/c/19776/5/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java@162
PS5, Line 162:
> This comment is off now.
Done
http://gerrit.cloudera.org:8080/#/c/19776/5/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java@172
PS5, Line 172: }
> is 'this.' needed here? Looks bit weird to me.
Yes, 'this' is needed here as ModifyKudu and ModifyIceberg are inner classes.
http://gerrit.cloudera.org:8080/#/c/19776/5/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java@174
PS5, Line 174: modifyImpl_ = this.new ModifyKudu();
> I believe this case is taken care by the check in L163.
Done
http://gerrit.cloudera.org:8080/#/c/19776/5/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java@362
PS5, Line 362: throws AnalysisException;
> This function (and the one below) seems implementation specific and seems o
Moved these methods back to the ModifyStmt.
http://gerrit.cloudera.org:8080/#/c/19776/5/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java@466
PS5, Line 466: deleteCols = new String[] {"INPUT__FILE__NAME", "FILE__POSITION"};
> Could you please create a Jira for this and link it here?
Done
http://gerrit.cloudera.org:8080/#/c/19776/5/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java@482
PS5, Line 482:
> As written in an above comment, having Kudu specific functions in this Iceb
Done
http://gerrit.cloudera.org:8080/#/c/19776/5/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
File fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java:
http://gerrit.cloudera.org:8080/#/c/19776/5/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java@300
PS5, Line 300: default boolean isPartitioned() {
> for my information: This returns true even if the table is not partitioned
Yes, maybe it could have a better name. isOrWasPartitiond()? :)
Could be improved further to detect if there's partitioning in only the available snapshots. But I'm not sure if it worth the effort as it is probably a very rare situation when users make a partitioned table non-partitioned.
http://gerrit.cloudera.org:8080/#/c/19776/5/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java@304
PS5, Line 304: if (partField.getTransformType() != TIcebergPartitionTransformType.VOID) {
> nit: you can negate the condition here and return true instead of 'continue
Thanks, done.
http://gerrit.cloudera.org:8080/#/c/19776/5/fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java
File fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java:
http://gerrit.cloudera.org:8080/#/c/19776/5/fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java@42
PS5, Line 42: it.
> Technically Iceberg is not a file format. Would it break anything to remove
Turns out I can remove this whole set.
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 May 2023 14:57:16 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 10: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 10
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 25 May 2023 20:46:32 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/19776/2/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:
http://gerrit.cloudera.org:8080/#/c/19776/2/tests/query_test/test_iceberg.py@1175
PS2, Line 1175: #
flake8: E116 unexpected indentation (comment)
http://gerrit.cloudera.org:8080/#/c/19776/2/tests/query_test/test_iceberg.py@1179
PS2, Line 1179:
flake8: W291 trailing whitespace
http://gerrit.cloudera.org:8080/#/c/19776/2/tests/query_test/test_iceberg.py@1179
PS2, Line 1179: assert hive_output == "_c0\n4799964\n"
line has trailing whitespace
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Comment-Date: Thu, 27 Apr 2023 12:57:14 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 5:
(11 comments)
Thanks for this change, Zoltan! I spent some time reviewing the FE and thrift part of the change. Will continue with the rest.
http://gerrit.cloudera.org:8080/#/c/19776/5/common/thrift/CatalogObjects.thrift
File common/thrift/CatalogObjects.thrift:
http://gerrit.cloudera.org:8080/#/c/19776/5/common/thrift/CatalogObjects.thrift@81
PS5, Line 81: PARTITION_SPEC_ID,
I haven't found any usage for these new virtual columns. I'd bet they are for the step2/delete from partitioned tables. Can we remove them from this patch then?
http://gerrit.cloudera.org:8080/#/c/19776/5/common/thrift/Query.thrift
File common/thrift/Query.thrift:
http://gerrit.cloudera.org:8080/#/c/19776/5/common/thrift/Query.thrift@827
PS5, Line 827: INSERT
It's not just INSERT anymore, right? (same below)
http://gerrit.cloudera.org:8080/#/c/19776/5/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
File fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java:
http://gerrit.cloudera.org:8080/#/c/19776/5/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java@162
PS5, Line 162: // Only Kudu tables can be updated
This comment is off now.
http://gerrit.cloudera.org:8080/#/c/19776/5/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java@172
PS5, Line 172: modifyImpl_ = this.new ModifyIceberg();
is 'this.' needed here? Looks bit weird to me.
http://gerrit.cloudera.org:8080/#/c/19776/5/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java@174
PS5, Line 174: throw new AnalysisException(
I believe this case is taken care by the check in L163.
http://gerrit.cloudera.org:8080/#/c/19776/5/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java@362
PS5, Line 362: void setKuduTransactionToken(byte[] kuduTxnToken);
This function (and the one below) seems implementation specific and seems odd to have Kudu related function in the parent class. Isn't there a way to remove these from the parent class?
http://gerrit.cloudera.org:8080/#/c/19776/5/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java@466
PS5, Line 466: // TODO: rewrite DELETE FROM t; statements to TRUNCATE TABLE t;
Could you please create a Jira for this and link it here?
http://gerrit.cloudera.org:8080/#/c/19776/5/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java@482
PS5, Line 482: public ByteBuffer getKuduTransactionToken() {
As written in an above comment, having Kudu specific functions in this Iceberg specific implementation seems odd.
http://gerrit.cloudera.org:8080/#/c/19776/5/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
File fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java:
http://gerrit.cloudera.org:8080/#/c/19776/5/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java@300
PS5, Line 300: default boolean isPartitioned() {
for my information: This returns true even if the table is not partitioned now but there was a partition spec in the past where it was partitioned?
http://gerrit.cloudera.org:8080/#/c/19776/5/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java@304
PS5, Line 304: if (partField.getTransformType() == TIcebergPartitionTransformType.VOID) {
nit: you can negate the condition here and return true instead of 'continue'. Might be slightly more readable.
http://gerrit.cloudera.org:8080/#/c/19776/5/fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java
File fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java:
http://gerrit.cloudera.org:8080/#/c/19776/5/fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java@42
PS5, Line 42: HdfsFileFormat.ICEBERG
Technically Iceberg is not a file format. Would it break anything to remove it from this list?
I did have another change the other day where I skipped adding HdfsFileFormat.ICEBERG into the file formats of a table. That might help removing this.
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 10 May 2023 08:46:40 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 7:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9312/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 7
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 May 2023 13:50:56 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Hello Andrew Sherman, Tamas Mate, Gabor Kaszab, lipenglin@apache.org, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19776
to look at the new patch set (#8).
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
This patch adds support for DELETE statements on unpartitioned Iceberg
tables. Impala uses the 'merge-on-read' mode with position delete files.
The patch reuses the existing IcebergPositionDeleteTable as the target
table of the DELETE statements, because this table already has the same
schema as position delete files, even with correct Iceberg field IDs.
The patch basically rewrites DELETE statements to INSERT statements,
e.g.:
from:
DELETE FROM ice_t WHERE id = 42;
to:
INSERT INTO ice_t-POSITION-DELETE
SELECT INPUT__FILE__NAME, FILE__POSITION
FROM ice_t
WHERE id = 42;
Position delete files need to be ordered by (file_position, pos), so
we add an extra SORT node before the table sink operator.
In the backend the patch adds a new table sink operator, the
IcebergDeleteSink. It writes the incoming rows (file_path, position) to
delete files. It reuses a lot of code from HdfsTableSink, so this patch
moves the common code to the new common base class: TableSinkBase.
The coordinator then collects the written delete files and invokes
UpdateCatalog to finalize the DELETE statement.
The Catalog then uses Iceberg APIs to create a new snapshot with the
created delete files. It also validates that there was no conflicting
data files written since the operation started.
Testing:
* added planer test
* e2e tests
* interop test between Impala and Hive
Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
---
M be/src/exec/CMakeLists.txt
M be/src/exec/data-sink.cc
M be/src/exec/file-metadata-utils.cc
M be/src/exec/hdfs-table-sink.cc
M be/src/exec/hdfs-table-sink.h
M be/src/exec/hdfs-table-writer.cc
M be/src/exec/hdfs-table-writer.h
A be/src/exec/iceberg-delete-sink.cc
A be/src/exec/iceberg-delete-sink.h
M be/src/exec/parquet/hdfs-parquet-table-writer.cc
M be/src/exec/parquet/hdfs-parquet-table-writer.h
A be/src/exec/table-sink-base.cc
A be/src/exec/table-sink-base.h
M be/src/runtime/coordinator.cc
M be/src/runtime/descriptors.cc
M be/src/service/client-request-state.cc
M common/thrift/CatalogObjects.thrift
M common/thrift/CatalogService.thrift
M common/thrift/DataSinks.thrift
M common/thrift/Query.thrift
M common/thrift/Types.thrift
M fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
M fe/src/main/java/org/apache/impala/analysis/IcebergPartitionSpec.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergPositionDeleteTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/VirtualColumn.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
A fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/TableSink.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeModifyStmtsTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-delete.test
A testdata/workloads/functional-query/queries/QueryTest/iceberg-delete-complex.test
A testdata/workloads/functional-query/queries/QueryTest/iceberg-delete.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M tests/query_test/test_iceberg.py
43 files changed, 1,814 insertions(+), 448 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/76/19776/8
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 8
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Hello Andrew Sherman, Tamas Mate, Gabor Kaszab, lipenglin@apache.org, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19776
to look at the new patch set (#9).
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
This patch adds support for DELETE statements on unpartitioned Iceberg
tables. Impala uses the 'merge-on-read' mode with position delete files.
The patch reuses the existing IcebergPositionDeleteTable as the target
table of the DELETE statements, because this table already has the same
schema as position delete files, even with correct Iceberg field IDs.
The patch basically rewrites DELETE statements to INSERT statements,
e.g.:
from:
DELETE FROM ice_t WHERE id = 42;
to:
INSERT INTO ice_t-POSITION-DELETE
SELECT INPUT__FILE__NAME, FILE__POSITION
FROM ice_t
WHERE id = 42;
Position delete files need to be ordered by (file_position, pos), so
we add an extra SORT node before the table sink operator.
In the backend the patch adds a new table sink operator, the
IcebergDeleteSink. It writes the incoming rows (file_path, position) to
delete files. It reuses a lot of code from HdfsTableSink, so this patch
moves the common code to the new common base class: TableSinkBase.
The coordinator then collects the written delete files and invokes
UpdateCatalog to finalize the DELETE statement.
The Catalog then uses Iceberg APIs to create a new snapshot with the
created delete files. It also validates that there was no conflicting
data files written since the operation started.
Testing:
* added planer test
* e2e tests
* interop test between Impala and Hive
Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
---
M be/src/exec/CMakeLists.txt
M be/src/exec/data-sink.cc
M be/src/exec/file-metadata-utils.cc
M be/src/exec/hdfs-table-sink.cc
M be/src/exec/hdfs-table-sink.h
M be/src/exec/hdfs-table-writer.cc
M be/src/exec/hdfs-table-writer.h
A be/src/exec/iceberg-delete-sink.cc
A be/src/exec/iceberg-delete-sink.h
M be/src/exec/parquet/hdfs-parquet-table-writer.cc
M be/src/exec/parquet/hdfs-parquet-table-writer.h
A be/src/exec/table-sink-base.cc
A be/src/exec/table-sink-base.h
M be/src/runtime/coordinator.cc
M be/src/runtime/descriptors.cc
M be/src/service/client-request-state.cc
M common/thrift/CatalogObjects.thrift
M common/thrift/CatalogService.thrift
M common/thrift/DataSinks.thrift
M common/thrift/Query.thrift
M common/thrift/Types.thrift
M fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
M fe/src/main/java/org/apache/impala/analysis/IcebergPartitionSpec.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergPositionDeleteTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/VirtualColumn.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
A fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/TableSink.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeModifyStmtsTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-delete.test
A testdata/workloads/functional-query/queries/QueryTest/iceberg-delete-complex.test
A testdata/workloads/functional-query/queries/QueryTest/iceberg-delete.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M tests/query_test/test_iceberg.py
43 files changed, 1,810 insertions(+), 447 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/76/19776/9
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 9
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/12890/ : 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/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 28 Apr 2023 11:49:03 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Hello Tamas Mate, Gabor Kaszab, lipenglin@apache.org, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19776
to look at the new patch set (#3).
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
This patch adds support for DELETE statements on unpartitioned Iceberg
tables. Impala uses the 'merge-on-read' mode with position delete files.
The patch reuses the existing IcebergPositionDeleteTable as the target
table of the DELETE statements, because this table already has the same
schema as position delete files, even with correct Iceberg field IDs.
The patch basically rewrites DELETE statements to INSERT statements,
e.g.:
from:
DELETE FROM ice_t WHERE id = 42;
to:
INSERT INTO ice_t-POSITION-DELETE
SELECT INPUT__FILE__NAME, FILE__POSITION
FROM ice_t
WHERE id = 42;
Position delete files need to be ordered by (file_position, pos), so
we add an extra SORT node before the table sink operator.
In the backend the patch adds a new table sink operator, the
IcebergDeleteSink. It writes the incoming rows (file_path, position) to
delete files. It reuses a lot of code from HdfsTableSink, so this patch
moves the common code to the new common base class: TableSinkBase.
The coordinator then collects the written delete files and invokes
UpdateCatalog to finalize the DELETE statement.
The Catalog then uses Iceberg APIs to create a new snapshot with the
created delete files. It also validates that there was no conflicting
data files written since the operation started.
Testing:
* added planer test
* e2e tests
* interop test between Impala and Hive
Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
---
M be/src/exec/CMakeLists.txt
M be/src/exec/data-sink.cc
M be/src/exec/file-metadata-utils.cc
M be/src/exec/hdfs-table-sink.cc
M be/src/exec/hdfs-table-sink.h
M be/src/exec/hdfs-table-writer.cc
M be/src/exec/hdfs-table-writer.h
A be/src/exec/iceberg-delete-sink.cc
A be/src/exec/iceberg-delete-sink.h
M be/src/exec/parquet/hdfs-parquet-table-writer.cc
M be/src/exec/parquet/hdfs-parquet-table-writer.h
A be/src/exec/table-sink-base.cc
A be/src/exec/table-sink-base.h
M be/src/runtime/coordinator.cc
M be/src/runtime/descriptors.cc
M be/src/service/client-request-state.cc
M common/thrift/CatalogObjects.thrift
M common/thrift/CatalogService.thrift
M common/thrift/DataSinks.thrift
M common/thrift/Query.thrift
M common/thrift/Types.thrift
M fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
M fe/src/main/java/org/apache/impala/analysis/IcebergPartitionSpec.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergPositionDeleteTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/VirtualColumn.java
A fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/TableSink.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-delete.test
A testdata/workloads/functional-query/queries/QueryTest/iceberg-delete.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M tests/query_test/test_iceberg.py
40 files changed, 1,723 insertions(+), 439 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/76/19776/3
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Hello Tamas Mate, Gabor Kaszab, lipenglin@apache.org, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19776
to look at the new patch set (#2).
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
This patch adds support for DELETE statements on unpartitioned Iceberg
tables. Impala uses the 'merge-on-read' mode with position delete files.
The patch reuses the existing IcebergPositionDeleteTable as the target
table of the DELETE statements, because this table already has the same
schema as position delete files, even with correct Iceberg field IDs.
The patch basically rewrites DELETE statements to INSERT statements,
e.g.:
from:
DELETE FROM ice_t WHERE id = 42;
to:
INSERT INTO ice_t-POSITION-DELETE
SELECT INPUT__FILE__NAME, FILE__POSITION
FROM ice_t
WHERE id = 42;
Position delete files need to be ordered by (file_position, pos), so
we add an extra SORT node before the table sink operator.
In the backend the patch adds a new table sink operator, the
IcebergDeleteSink. It writes the incoming rows (file_path, position) to
delete files. It reuses a lot of code from HdfsTableSink, so this patch
moves the common code to the new common base class: TableSinkBase.
The coordinator then collects the written delete files and invokes
UpdateCatalog to finalize the DELETE statement.
The Catalog then uses Iceberg APIs to create a new snapshot with the
created delete files. It also validates that there was no conflicting
data files written since the operation started.
Testing:
* added planer test
* e2e tests
* interop test between Impala and Hive
Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
---
M be/src/exec/CMakeLists.txt
M be/src/exec/data-sink.cc
M be/src/exec/file-metadata-utils.cc
M be/src/exec/hdfs-table-sink.cc
M be/src/exec/hdfs-table-sink.h
M be/src/exec/hdfs-table-writer.cc
M be/src/exec/hdfs-table-writer.h
A be/src/exec/iceberg-delete-sink.cc
A be/src/exec/iceberg-delete-sink.h
M be/src/exec/parquet/hdfs-parquet-table-writer.cc
M be/src/exec/parquet/hdfs-parquet-table-writer.h
A be/src/exec/table-sink-base.cc
A be/src/exec/table-sink-base.h
M be/src/runtime/coordinator.cc
M be/src/runtime/descriptors.cc
M be/src/service/client-request-state.cc
M common/thrift/CatalogObjects.thrift
M common/thrift/CatalogService.thrift
M common/thrift/DataSinks.thrift
M common/thrift/Query.thrift
M common/thrift/Types.thrift
M fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
M fe/src/main/java/org/apache/impala/analysis/IcebergPartitionSpec.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergPositionDeleteTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/VirtualColumn.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/TableSink.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-delete.test
A testdata/workloads/functional-query/queries/QueryTest/iceberg-delete.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M tests/query_test/test_iceberg.py
39 files changed, 1,561 insertions(+), 439 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/76/19776/2
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 11:
(10 comments)
Thanks for the comments!
http://gerrit.cloudera.org:8080/#/c/19776/10/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
File fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java:
http://gerrit.cloudera.org:8080/#/c/19776/10/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java@467
PS10, Line 467: Set<SlotId> uniqueSlots, Set<SlotId> keySlots, Map<String, Integer> colIndexMap)
: throws AnalysisException {
: String[] deleteCols;
: Preconditions.checkState(!icePosDelTable_.isPartitioned());
: deleteCols = new String[] {"INPUT__FILE__NAME", "FILE__POSITION"};
: // Add the key columns as slot refs
: for (String k : deleteCols) {
> For me it seems that the body of the for loop is identical between the Iceb
Done
http://gerrit.cloudera.org:8080/#/c/19776/10/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
File fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java:
http://gerrit.cloudera.org:8080/#/c/19776/10/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java@340
PS10, Line 340: get
> You can use the getOrDefault(..) method on properties() to simplify this fu
Done
http://gerrit.cloudera.org:8080/#/c/19776/10/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java@349
PS10, Line 349: return IcebergUtil.getIcebergFileFormat(deleteFormat);
> You can use getOrDefault() here as well, where the default value is getWrit
getWriteFileFormat() returns TIcebergFileFormat, so we cannot use it in getOrDefault() which expects the two argument with the same type.
http://gerrit.cloudera.org:8080/#/c/19776/10/fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java
File fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java:
http://gerrit.cloudera.org:8080/#/c/19776/10/fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java@28
PS10, Line 28: import org.apache.impala.thrift.TExplainLevel;
> Logger and LoggerFactory aren't used.
Done
http://gerrit.cloudera.org:8080/#/c/19776/10/fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java@61
PS10, Line 61: PlanNode inputNode = fragment_.getPlanRoot();
> These variables can be final, if I'm not mistaken.
Done
http://gerrit.cloudera.org:8080/#/c/19776/10/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/19776/10/fe/src/main/java/org/apache/impala/service/Frontend.java@2548
PS10, Line 2548: private static void addFinalizationParamsForDelete(
> nit: I think the indentation is off in this function.
Done
http://gerrit.cloudera.org:8080/#/c/19776/10/fe/src/main/java/org/apache/impala/service/Frontend.java@2555
PS10, Line 2555: queryCtx, targetTable, false);
: TIcebergDmlFinalizeParams iceFinalizeParams = new TIcebergDmlFinalizeParams();
: iceFinalizeParams.operation = TIcebergOperation.DELETE;
: FeIcebergTable iceTable = (FeIcebergTable)targetTable;
: iceFinalizeParams.setSpec_id(iceTable.getDefaultPartitionSpecId());
: iceFinalizeParams.setInitial_snapshot_id(iceTable.snapshotId());
: finalizeParams.setIceberg_params(iceFinalizeParams);
> This part is kind of the same as in addFinalizatonParamsForInsert(). Would
Done
http://gerrit.cloudera.org:8080/#/c/19776/10/testdata/workloads/functional-query/queries/QueryTest/iceberg-delete-complex.test
File testdata/workloads/functional-query/queries/QueryTest/iceberg-delete-complex.test:
http://gerrit.cloudera.org:8080/#/c/19776/10/testdata/workloads/functional-query/queries/QueryTest/iceberg-delete-complex.test@3
PS10, Line 3: SELECT * FROM ice_complex_delete;
> I think this comment is not relevant here.
Done
http://gerrit.cloudera.org:8080/#/c/19776/10/testdata/workloads/functional-query/queries/QueryTest/iceberg-delete-complex.test@90
PS10, Line 90: ---- QUERY
> Could you give it a try to include an UNNEST() fn call into the select list
I added UNNEST to int_array and int_array_array. I tried to add UNNEST to nested_struct.c.d, but apparently it is not supported by Impala.
http://gerrit.cloudera.org:8080/#/c/19776/10/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:
http://gerrit.cloudera.org:8080/#/c/19776/10/tests/query_test/test_iceberg.py@1163
PS10, Line 1163: hive_output = self.run_stmt_in_hive("SELECT * FROM {} ORDER BY i".format(ice_delete))
> You run this 2 SELECTs twice. I think this line can be dropped.
Done
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 11
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Jun 2023 17:24:13 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 3:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/12883/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 27 Apr 2023 16:40:57 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9260/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 27 Apr 2023 16:19:41 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13020/ : 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/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 7
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 May 2023 14:10:47 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
Posted by "Andrew Sherman (Code Review)" <ge...@cloudera.org>.
Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 7:
(4 comments)
A few nits only
http://gerrit.cloudera.org:8080/#/c/19776/7/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
File fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java:
http://gerrit.cloudera.org:8080/#/c/19776/7/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java@30
PS7, Line 30: import org.apache.commons.lang.NotImplementedException;
unused I think
http://gerrit.cloudera.org:8080/#/c/19776/7/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java@442
PS7, Line 442: "but the given table use a different file format: " +
Nit "table uses a"
http://gerrit.cloudera.org:8080/#/c/19776/7/fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java
File fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java:
http://gerrit.cloudera.org:8080/#/c/19776/7/fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java@26
PS7, Line 26: import org.apache.impala.catalog.HdfsFileFormat;
Nit: HdfsFileFormat, IcebergPositionDeleteTable, ImmutableSet are unused
http://gerrit.cloudera.org:8080/#/c/19776/7/fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java@43
PS7, Line 43: private final static Logger LOG = LoggerFactory.getLogger(IcebergDeleteSink.class);
Nit: unused
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 7
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 May 2023 20:13:56 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/12999/ : 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/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 May 2023 15:18:05 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 7: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 7
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 May 2023 19:05:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Hello Tamas Mate, Gabor Kaszab, lipenglin@apache.org, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19776
to look at the new patch set (#4).
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
This patch adds support for DELETE statements on unpartitioned Iceberg
tables. Impala uses the 'merge-on-read' mode with position delete files.
The patch reuses the existing IcebergPositionDeleteTable as the target
table of the DELETE statements, because this table already has the same
schema as position delete files, even with correct Iceberg field IDs.
The patch basically rewrites DELETE statements to INSERT statements,
e.g.:
from:
DELETE FROM ice_t WHERE id = 42;
to:
INSERT INTO ice_t-POSITION-DELETE
SELECT INPUT__FILE__NAME, FILE__POSITION
FROM ice_t
WHERE id = 42;
Position delete files need to be ordered by (file_position, pos), so
we add an extra SORT node before the table sink operator.
In the backend the patch adds a new table sink operator, the
IcebergDeleteSink. It writes the incoming rows (file_path, position) to
delete files. It reuses a lot of code from HdfsTableSink, so this patch
moves the common code to the new common base class: TableSinkBase.
The coordinator then collects the written delete files and invokes
UpdateCatalog to finalize the DELETE statement.
The Catalog then uses Iceberg APIs to create a new snapshot with the
created delete files. It also validates that there was no conflicting
data files written since the operation started.
Testing:
* added planer test
* e2e tests
* interop test between Impala and Hive
Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
---
M be/src/exec/CMakeLists.txt
M be/src/exec/data-sink.cc
M be/src/exec/file-metadata-utils.cc
M be/src/exec/hdfs-table-sink.cc
M be/src/exec/hdfs-table-sink.h
M be/src/exec/hdfs-table-writer.cc
M be/src/exec/hdfs-table-writer.h
A be/src/exec/iceberg-delete-sink.cc
A be/src/exec/iceberg-delete-sink.h
M be/src/exec/parquet/hdfs-parquet-table-writer.cc
M be/src/exec/parquet/hdfs-parquet-table-writer.h
A be/src/exec/table-sink-base.cc
A be/src/exec/table-sink-base.h
M be/src/runtime/coordinator.cc
M be/src/runtime/descriptors.cc
M be/src/service/client-request-state.cc
M common/thrift/CatalogObjects.thrift
M common/thrift/CatalogService.thrift
M common/thrift/DataSinks.thrift
M common/thrift/Query.thrift
M common/thrift/Types.thrift
M fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
M fe/src/main/java/org/apache/impala/analysis/IcebergPartitionSpec.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergPositionDeleteTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/VirtualColumn.java
A fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/TableSink.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeModifyStmtsTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-delete.test
A testdata/workloads/functional-query/queries/QueryTest/iceberg-delete.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M tests/query_test/test_iceberg.py
41 files changed, 1,739 insertions(+), 448 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/76/19776/4
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
This patch adds support for DELETE statements on unpartitioned Iceberg
tables. Impala uses the 'merge-on-read' mode with position delete files.
The patch reuses the existing IcebergPositionDeleteTable as the target
table of the DELETE statements, because this table already has the same
schema as position delete files, even with correct Iceberg field IDs.
The patch basically rewrites DELETE statements to INSERT statements,
e.g.:
from:
DELETE FROM ice_t WHERE id = 42;
to:
INSERT INTO ice_t-POSITION-DELETE
SELECT INPUT__FILE__NAME, FILE__POSITION
FROM ice_t
WHERE id = 42;
Position delete files need to be ordered by (file_path, pos), so
we add an extra SORT node before the table sink operator.
In the backend the patch adds a new table sink operator, the
IcebergDeleteSink. It writes the incoming rows (file_path, position) to
delete files. It reuses a lot of code from HdfsTableSink, so this patch
moves the common code to the new common base class: TableSinkBase.
The coordinator then collects the written delete files and invokes
UpdateCatalog to finalize the DELETE statement.
The Catalog then uses Iceberg APIs to create a new snapshot with the
created delete files. It also validates that there was no conflicting
data files written since the operation started.
Testing:
* added planer test
* e2e tests
* interop test between Impala and Hive
Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Reviewed-on: http://gerrit.cloudera.org:8080/19776
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Gabor Kaszab <ga...@cloudera.com>
---
M be/src/exec/CMakeLists.txt
M be/src/exec/data-sink.cc
M be/src/exec/file-metadata-utils.cc
M be/src/exec/hdfs-table-sink.cc
M be/src/exec/hdfs-table-sink.h
M be/src/exec/hdfs-table-writer.cc
M be/src/exec/hdfs-table-writer.h
A be/src/exec/iceberg-delete-sink.cc
A be/src/exec/iceberg-delete-sink.h
M be/src/exec/parquet/hdfs-parquet-table-writer.cc
M be/src/exec/parquet/hdfs-parquet-table-writer.h
A be/src/exec/table-sink-base.cc
A be/src/exec/table-sink-base.h
M be/src/runtime/coordinator.cc
M be/src/runtime/descriptors.cc
M be/src/service/client-request-state.cc
M common/thrift/CatalogObjects.thrift
M common/thrift/CatalogService.thrift
M common/thrift/DataSinks.thrift
M common/thrift/Query.thrift
M common/thrift/Types.thrift
M fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
M fe/src/main/java/org/apache/impala/analysis/IcebergPartitionSpec.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergPositionDeleteTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/VirtualColumn.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
A fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/TableSink.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeModifyStmtsTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-delete.test
A testdata/workloads/functional-query/queries/QueryTest/iceberg-delete-complex.test
A testdata/workloads/functional-query/queries/QueryTest/iceberg-delete.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M tests/query_test/test_iceberg.py
44 files changed, 1,858 insertions(+), 465 deletions(-)
Approvals:
Impala Public Jenkins: Verified
Gabor Kaszab: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 13
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Hello Andrew Sherman, Tamas Mate, Gabor Kaszab, lipenglin@apache.org, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19776
to look at the new patch set (#11).
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
This patch adds support for DELETE statements on unpartitioned Iceberg
tables. Impala uses the 'merge-on-read' mode with position delete files.
The patch reuses the existing IcebergPositionDeleteTable as the target
table of the DELETE statements, because this table already has the same
schema as position delete files, even with correct Iceberg field IDs.
The patch basically rewrites DELETE statements to INSERT statements,
e.g.:
from:
DELETE FROM ice_t WHERE id = 42;
to:
INSERT INTO ice_t-POSITION-DELETE
SELECT INPUT__FILE__NAME, FILE__POSITION
FROM ice_t
WHERE id = 42;
Position delete files need to be ordered by (file_position, pos), so
we add an extra SORT node before the table sink operator.
In the backend the patch adds a new table sink operator, the
IcebergDeleteSink. It writes the incoming rows (file_path, position) to
delete files. It reuses a lot of code from HdfsTableSink, so this patch
moves the common code to the new common base class: TableSinkBase.
The coordinator then collects the written delete files and invokes
UpdateCatalog to finalize the DELETE statement.
The Catalog then uses Iceberg APIs to create a new snapshot with the
created delete files. It also validates that there was no conflicting
data files written since the operation started.
Testing:
* added planer test
* e2e tests
* interop test between Impala and Hive
Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
---
M be/src/exec/CMakeLists.txt
M be/src/exec/data-sink.cc
M be/src/exec/file-metadata-utils.cc
M be/src/exec/hdfs-table-sink.cc
M be/src/exec/hdfs-table-sink.h
M be/src/exec/hdfs-table-writer.cc
M be/src/exec/hdfs-table-writer.h
A be/src/exec/iceberg-delete-sink.cc
A be/src/exec/iceberg-delete-sink.h
M be/src/exec/parquet/hdfs-parquet-table-writer.cc
M be/src/exec/parquet/hdfs-parquet-table-writer.h
A be/src/exec/table-sink-base.cc
A be/src/exec/table-sink-base.h
M be/src/runtime/coordinator.cc
M be/src/runtime/descriptors.cc
M be/src/service/client-request-state.cc
M common/thrift/CatalogObjects.thrift
M common/thrift/CatalogService.thrift
M common/thrift/DataSinks.thrift
M common/thrift/Query.thrift
M common/thrift/Types.thrift
M fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
M fe/src/main/java/org/apache/impala/analysis/IcebergPartitionSpec.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergPositionDeleteTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/VirtualColumn.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
A fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/TableSink.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeModifyStmtsTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-delete.test
A testdata/workloads/functional-query/queries/QueryTest/iceberg-delete-complex.test
A testdata/workloads/functional-query/queries/QueryTest/iceberg-delete.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M tests/query_test/test_iceberg.py
44 files changed, 1,858 insertions(+), 465 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/76/19776/11
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 11
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 12:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19776/11//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19776/11//COMMIT_MSG@28
PS11, Line 28: file_path, pos), s
> nit: maybe "file_path, position"?
Thanks, fixed to 'file_path'.
The Iceberg spec states that position delete files must be sorted by (file_path, pos): https://iceberg.apache.org/spec/#position-delete-files
It mentions predicate pushdown and optimized scanning. Btw, we are also planning to optimize the processing of position delete files: https://gerrit.cloudera.org/#/c/19850/
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 12
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Jun 2023 08:14:28 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 12:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9385/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 12
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Jun 2023 18:56:08 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 6:
(4 comments)
Thanks for the comments.
PS6 was flawed, it raised NPE for all DELETE statements.
http://gerrit.cloudera.org:8080/#/c/19776/6/fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
File fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java:
http://gerrit.cloudera.org:8080/#/c/19776/6/fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java@68
PS6, Line 68:
> nit: empty line
Done
http://gerrit.cloudera.org:8080/#/c/19776/6/fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java
File fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java:
http://gerrit.cloudera.org:8080/#/c/19776/6/fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java@36
PS6, Line 36: * Sink for deleting data from Iceberg tables.
> nit: Could you extend this comment with a few sentence how iceberg handles
Extended the comment.
http://gerrit.cloudera.org:8080/#/c/19776/5/testdata/workloads/functional-query/queries/QueryTest/iceberg-delete.test
File testdata/workloads/functional-query/queries/QueryTest/iceberg-delete.test:
http://gerrit.cloudera.org:8080/#/c/19776/5/testdata/workloads/functional-query/queries/QueryTest/iceberg-delete.test@8
PS5, Line 8: Deelte
> nit: Delete
Done
http://gerrit.cloudera.org:8080/#/c/19776/6/testdata/workloads/functional-query/queries/QueryTest/iceberg-delete.test
File testdata/workloads/functional-query/queries/QueryTest/iceberg-delete.test:
http://gerrit.cloudera.org:8080/#/c/19776/6/testdata/workloads/functional-query/queries/QueryTest/iceberg-delete.test@8
PS6, Line 8: Deelte
> nit: Delete
Done
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 May 2023 13:50:37 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 2:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9259/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 27 Apr 2023 13:02:18 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 4:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9263/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 28 Apr 2023 11:28:09 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/12932/ : 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/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 May 2023 15:09:47 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Hello Tamas Mate, Gabor Kaszab, lipenglin@apache.org, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19776
to look at the new patch set (#6).
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
This patch adds support for DELETE statements on unpartitioned Iceberg
tables. Impala uses the 'merge-on-read' mode with position delete files.
The patch reuses the existing IcebergPositionDeleteTable as the target
table of the DELETE statements, because this table already has the same
schema as position delete files, even with correct Iceberg field IDs.
The patch basically rewrites DELETE statements to INSERT statements,
e.g.:
from:
DELETE FROM ice_t WHERE id = 42;
to:
INSERT INTO ice_t-POSITION-DELETE
SELECT INPUT__FILE__NAME, FILE__POSITION
FROM ice_t
WHERE id = 42;
Position delete files need to be ordered by (file_position, pos), so
we add an extra SORT node before the table sink operator.
In the backend the patch adds a new table sink operator, the
IcebergDeleteSink. It writes the incoming rows (file_path, position) to
delete files. It reuses a lot of code from HdfsTableSink, so this patch
moves the common code to the new common base class: TableSinkBase.
The coordinator then collects the written delete files and invokes
UpdateCatalog to finalize the DELETE statement.
The Catalog then uses Iceberg APIs to create a new snapshot with the
created delete files. It also validates that there was no conflicting
data files written since the operation started.
Testing:
* added planer test
* e2e tests
* interop test between Impala and Hive
Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
---
M be/src/exec/CMakeLists.txt
M be/src/exec/data-sink.cc
M be/src/exec/file-metadata-utils.cc
M be/src/exec/hdfs-table-sink.cc
M be/src/exec/hdfs-table-sink.h
M be/src/exec/hdfs-table-writer.cc
M be/src/exec/hdfs-table-writer.h
A be/src/exec/iceberg-delete-sink.cc
A be/src/exec/iceberg-delete-sink.h
M be/src/exec/parquet/hdfs-parquet-table-writer.cc
M be/src/exec/parquet/hdfs-parquet-table-writer.h
A be/src/exec/table-sink-base.cc
A be/src/exec/table-sink-base.h
M be/src/runtime/coordinator.cc
M be/src/runtime/descriptors.cc
M be/src/service/client-request-state.cc
M common/thrift/CatalogObjects.thrift
M common/thrift/CatalogService.thrift
M common/thrift/DataSinks.thrift
M common/thrift/Query.thrift
M common/thrift/Types.thrift
M fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
M fe/src/main/java/org/apache/impala/analysis/IcebergPartitionSpec.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergPositionDeleteTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/VirtualColumn.java
A fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/TableSink.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeModifyStmtsTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-delete.test
A testdata/workloads/functional-query/queries/QueryTest/iceberg-delete.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
M tests/query_test/test_iceberg.py
41 files changed, 1,697 insertions(+), 444 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/76/19776/6
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 8:
(5 comments)
Thanks for the comments!
Added a new test: iceberg-delete-complex.test
It currently failes because of IMPALA-12153 (fix is on review https://gerrit.cloudera.org/#/c/19911/)
http://gerrit.cloudera.org:8080/#/c/19776/7/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
File fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java:
http://gerrit.cloudera.org:8080/#/c/19776/7/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java@30
PS7, Line 30: import org.apache.impala.authorization.Privilege;
> unused I think
Done
http://gerrit.cloudera.org:8080/#/c/19776/7/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java@442
PS7, Line 442: originalTargetTable_.getFullName());
> Nit "table uses a"
Done
http://gerrit.cloudera.org:8080/#/c/19776/7/fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java
File fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java:
http://gerrit.cloudera.org:8080/#/c/19776/7/fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java@26
PS7, Line 26:
> Nit: HdfsFileFormat, IcebergPositionDeleteTable, ImmutableSet are unused
Done
http://gerrit.cloudera.org:8080/#/c/19776/7/fe/src/main/java/org/apache/impala/planner/IcebergDeleteSink.java@43
PS7, Line 43:
> Nit: unused
Done
http://gerrit.cloudera.org:8080/#/c/19776/7/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/19776/7/fe/src/main/java/org/apache/impala/service/Frontend.java@2571
PS7, Line 2571: TF
> nit: indentation
Done
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 8
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 May 2023 10:54:37 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 9:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13108/ : 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/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 9
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 May 2023 12:46:05 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 10:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13128/ : 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/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 10
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 25 May 2023 15:46:17 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED 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/19776 )
Change subject: IMPALA-11877: (part 1) Add support for DELETE statements for UNPARTITIONED Iceberg tables
......................................................................
Patch Set 9:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9344/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/19776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic933b2295abe54b46d2a736961219988ff42915b
Gerrit-Change-Number: 19776
Gerrit-PatchSet: 9
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 May 2023 12:23:43 +0000
Gerrit-HasComments: No