You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Tamas Mate (Code Review)" <ge...@cloudera.org> on 2022/06/21 18:22:21 UTC

[Impala-ASF-CR] IMPALA-11377: Handle concurrent Iceberg INSERT OVERWRITEs

Tamas Mate has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18648


Change subject: IMPALA-11377: Handle concurrent Iceberg INSERT OVERWRITEs
......................................................................

IMPALA-11377: Handle concurrent Iceberg INSERT OVERWRITEs

Iceberg now supports validations in the ReplacePartitions API, an initial
snapshot id can be configured for the operation and Iceberg will validate
at commit if there were any conflicting change while the ReplacePartitions
was running.

This commit utilizes this new functionality, the Coordinator sends the initial
snapshot id to the Catalog, so the Catalog can validate the operation.

Testing:
 - Added e2e tests.

Change-Id: I812d19736c8e563541b038970786de7710b59f31
---
M be/src/service/client-request-state.cc
M common/thrift/CatalogService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M tests/query_test/test_iceberg.py
6 files changed, 55 insertions(+), 3 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/48/18648/1
-- 
To view, visit http://gerrit.cloudera.org:8080/18648
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I812d19736c8e563541b038970786de7710b59f31
Gerrit-Change-Number: 18648
Gerrit-PatchSet: 1
Gerrit-Owner: Tamas Mate <tm...@apache.org>

[Impala-ASF-CR] IMPALA-11377: Handle concurrent Iceberg INSERT OVERWRITEs

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18648 )

Change subject: IMPALA-11377: Handle concurrent Iceberg INSERT OVERWRITEs
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10823/ : 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/18648
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I812d19736c8e563541b038970786de7710b59f31
Gerrit-Change-Number: 18648
Gerrit-PatchSet: 1
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 21 Jun 2022 18:42:13 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11377: Handle concurrent Iceberg INSERT OVERWRITEs

Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/18648 )

Change subject: IMPALA-11377: Handle concurrent Iceberg INSERT OVERWRITEs
......................................................................

IMPALA-11377: Handle concurrent Iceberg INSERT OVERWRITEs

Iceberg now supports validations in the ReplacePartitions API, an
initial snapshot id can be configured for the operation and Iceberg
will validate at commit if there were any conflicting change while the
ReplacePartitions was running.

This commit utilizes this new functionality, the Coordinator sends the
initial snapshot id to the Catalog, so the Catalog can validate the
operation.

Testing:
 - Added e2e tests.

Change-Id: I812d19736c8e563541b038970786de7710b59f31
---
M be/src/service/client-request-state.cc
M common/thrift/CatalogService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M tests/query_test/test_iceberg.py
6 files changed, 61 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/48/18648/4
-- 
To view, visit http://gerrit.cloudera.org:8080/18648
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I812d19736c8e563541b038970786de7710b59f31
Gerrit-Change-Number: 18648
Gerrit-PatchSet: 4
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@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-11377: Handle concurrent Iceberg INSERT OVERWRITEs

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18648 )

Change subject: IMPALA-11377: Handle concurrent Iceberg INSERT OVERWRITEs
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10908/ : 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/18648
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I812d19736c8e563541b038970786de7710b59f31
Gerrit-Change-Number: 18648
Gerrit-PatchSet: 4
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@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, 01 Jul 2022 08:03:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11331: Handle concurrent Iceberg INSERT OVERWRITEs

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/18648 )

Change subject: IMPALA-11331: Handle concurrent Iceberg INSERT OVERWRITEs
......................................................................


Patch Set 3: Code-Review+2

(1 comment)

One small nit, otherwise the change looks great!

http://gerrit.cloudera.org:8080/#/c/18648/3/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:

http://gerrit.cloudera.org:8080/#/c/18648/3/tests/query_test/test_iceberg.py@122
PS3, Line 122: re.search("Found conflicting files.*", str(e))
nit:

 you can just use "Found conflicting files" in str(e)

No need for the re module.



-- 
To view, visit http://gerrit.cloudera.org:8080/18648
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I812d19736c8e563541b038970786de7710b59f31
Gerrit-Change-Number: 18648
Gerrit-PatchSet: 3
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@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, 30 Jun 2022 17:31:56 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11377: Handle concurrent Iceberg INSERT OVERWRITEs

Posted by "Gergely Fürnstáhl (Code Review)" <ge...@cloudera.org>.
Gergely Fürnstáhl has posted comments on this change. ( http://gerrit.cloudera.org:8080/18648 )

Change subject: IMPALA-11377: Handle concurrent Iceberg INSERT OVERWRITEs
......................................................................


Patch Set 1: Code-Review+1

(1 comment)

Small additional nit-pick, otherwise LGTM

http://gerrit.cloudera.org:8080/#/c/18648/1/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:

http://gerrit.cloudera.org:8080/#/c/18648/1/tests/query_test/test_iceberg.py@117
PS1, Line 117:     assert exception is not None
nit: maybe this is a bit cleaner (and more widely used among our tests):

    try:
      self.client.wait_for_finished_timeout(handle, 30)
      assert False
    except ImpalaBeeswaxException as e:



-- 
To view, visit http://gerrit.cloudera.org:8080/18648
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I812d19736c8e563541b038970786de7710b59f31
Gerrit-Change-Number: 18648
Gerrit-PatchSet: 1
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 22 Jun 2022 15:03:17 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11377: Handle concurrent Iceberg INSERT OVERWRITEs

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18648 )

Change subject: IMPALA-11377: Handle concurrent Iceberg INSERT OVERWRITEs
......................................................................


Patch Set 5: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/18648
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I812d19736c8e563541b038970786de7710b59f31
Gerrit-Change-Number: 18648
Gerrit-PatchSet: 5
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@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, 01 Jul 2022 12:54:10 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11377: Handle concurrent Iceberg INSERT OVERWRITEs

Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/18648 )

Change subject: IMPALA-11377: Handle concurrent Iceberg INSERT OVERWRITEs
......................................................................

IMPALA-11377: Handle concurrent Iceberg INSERT OVERWRITEs

Iceberg now supports validations in the ReplacePartitions API, an
initial snapshot id can be configured for the operation and Iceberg
will validate at commit if there were any conflicting change while the
ReplacePartitions was running.

This commit utilizes this new functionality, the Coordinator sends the
initial snapshot id to the Catalog, so the Catalog can validate the
operation.

Testing:
 - Added e2e tests.

Change-Id: I812d19736c8e563541b038970786de7710b59f31
---
M be/src/service/client-request-state.cc
M common/thrift/CatalogService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M tests/query_test/test_iceberg.py
6 files changed, 62 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/48/18648/2
-- 
To view, visit http://gerrit.cloudera.org:8080/18648
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I812d19736c8e563541b038970786de7710b59f31
Gerrit-Change-Number: 18648
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@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-11331: Handle concurrent Iceberg INSERT OVERWRITEs

Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Hello Zoltan Borok-Nagy, Gergely Fürnstáhl, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18648

to look at the new patch set (#3).

Change subject: IMPALA-11331: Handle concurrent Iceberg INSERT OVERWRITEs
......................................................................

IMPALA-11331: Handle concurrent Iceberg INSERT OVERWRITEs

Iceberg now supports validations in the ReplacePartitions API, an
initial snapshot id can be configured for the operation and Iceberg
will validate at commit if there were any conflicting change while the
ReplacePartitions was running.

This commit utilizes this new functionality, the Coordinator sends the
initial snapshot id to the Catalog, so the Catalog can validate the
operation.

Testing:
 - Added e2e tests.

Change-Id: I812d19736c8e563541b038970786de7710b59f31
---
M be/src/service/client-request-state.cc
M common/thrift/CatalogService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M tests/query_test/test_iceberg.py
6 files changed, 62 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/48/18648/3
-- 
To view, visit http://gerrit.cloudera.org:8080/18648
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I812d19736c8e563541b038970786de7710b59f31
Gerrit-Change-Number: 18648
Gerrit-PatchSet: 3
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@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-11377: Handle concurrent Iceberg INSERT OVERWRITEs

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18648 )

Change subject: IMPALA-11377: Handle concurrent Iceberg INSERT OVERWRITEs
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10847/ : 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/18648
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I812d19736c8e563541b038970786de7710b59f31
Gerrit-Change-Number: 18648
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@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, 23 Jun 2022 13:21:31 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11377: Handle concurrent Iceberg INSERT OVERWRITEs

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18648 )

Change subject: IMPALA-11377: Handle concurrent Iceberg INSERT OVERWRITEs
......................................................................


Patch Set 5: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/18648
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I812d19736c8e563541b038970786de7710b59f31
Gerrit-Change-Number: 18648
Gerrit-PatchSet: 5
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@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, 01 Jul 2022 08:06:53 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11377: Handle concurrent Iceberg INSERT OVERWRITEs

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18648 )

Change subject: IMPALA-11377: Handle concurrent Iceberg INSERT OVERWRITEs
......................................................................

IMPALA-11377: Handle concurrent Iceberg INSERT OVERWRITEs

Iceberg now supports validations in the ReplacePartitions API, an
initial snapshot id can be configured for the operation and Iceberg
will validate at commit if there were any conflicting change while the
ReplacePartitions was running.

This commit utilizes this new functionality, the Coordinator sends the
initial snapshot id to the Catalog, so the Catalog can validate the
operation.

Testing:
 - Added e2e tests.

Change-Id: I812d19736c8e563541b038970786de7710b59f31
Reviewed-on: http://gerrit.cloudera.org:8080/18648
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/service/client-request-state.cc
M common/thrift/CatalogService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M tests/query_test/test_iceberg.py
6 files changed, 61 insertions(+), 5 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/18648
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I812d19736c8e563541b038970786de7710b59f31
Gerrit-Change-Number: 18648
Gerrit-PatchSet: 6
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@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-11377: Handle concurrent Iceberg INSERT OVERWRITEs

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18648 )

Change subject: IMPALA-11377: Handle concurrent Iceberg INSERT OVERWRITEs
......................................................................


Patch Set 5:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/8286/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/18648
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I812d19736c8e563541b038970786de7710b59f31
Gerrit-Change-Number: 18648
Gerrit-PatchSet: 5
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@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, 01 Jul 2022 08:06:54 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11377: Handle concurrent Iceberg INSERT OVERWRITEs

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/18648 )

Change subject: IMPALA-11377: Handle concurrent Iceberg INSERT OVERWRITEs
......................................................................


Patch Set 1:

(9 comments)

Found a couple of nits, but looks good overall.

http://gerrit.cloudera.org:8080/#/c/18648/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18648/1//COMMIT_MSG@9
PS1, Line 9: l
nit: we limit commit message lines to 72 chars width.


http://gerrit.cloudera.org:8080/#/c/18648/1/common/thrift/CatalogService.thrift
File common/thrift/CatalogService.thrift:

http://gerrit.cloudera.org:8080/#/c/18648/1/common/thrift/CatalogService.thrift@218
PS1, Line 218: spanshopt
typo


http://gerrit.cloudera.org:8080/#/c/18648/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/18648/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@310
PS1, Line 310:  ImpalaRuntimeException(e.getMessage());
nit: maybe we could also add `e` as the cause to get more information:

 ImpalaRuntimeException(e.getMessage(), e);


http://gerrit.cloudera.org:8080/#/c/18648/1/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:

http://gerrit.cloudera.org:8080/#/c/18648/1/tests/query_test/test_iceberg.py@76
PS1, Line 76: @SkipIf.not_hdfs
Can we remove this?


http://gerrit.cloudera.org:8080/#/c/18648/1/tests/query_test/test_iceberg.py@84
PS1, Line 84: @SkipIf.not_hdfs
I don't think we need this annotation here.


http://gerrit.cloudera.org:8080/#/c/18648/1/tests/query_test/test_iceberg.py@95
PS1, Line 95: cat_location = "/test-warehouse/" + unique_database
we can just create a default HiveCatalog table


http://gerrit.cloudera.org:8080/#/c/18648/1/tests/query_test/test_iceberg.py@99
PS1, Line 99:         tblproperties('iceberg.catalog'='hadoop.catalog',
            :                       'iceberg.catalog_location'='{1}')""".format(tbl_name, cat_location))
No need for these params


http://gerrit.cloudera.org:8080/#/c/18648/1/tests/query_test/test_iceberg.py@101
PS1, Line 101:     self.client.execute("insert into {0} values (1), (2), (3);".format(tbl_name))
nit: maybe add a comment that it is for warm-up.


http://gerrit.cloudera.org:8080/#/c/18648/1/tests/query_test/test_iceberg.py@103
PS1, Line 103: insert overwrite
we could have a test with INSERT INTO as well



-- 
To view, visit http://gerrit.cloudera.org:8080/18648
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I812d19736c8e563541b038970786de7710b59f31
Gerrit-Change-Number: 18648
Gerrit-PatchSet: 1
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 22 Jun 2022 11:35:09 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11377: Handle concurrent Iceberg INSERT OVERWRITEs

Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/18648 )

Change subject: IMPALA-11377: Handle concurrent Iceberg INSERT OVERWRITEs
......................................................................


Patch Set 4: Code-Review+2

(1 comment)

Thank you for the review! Fixed the nit and carrying over the +2.

http://gerrit.cloudera.org:8080/#/c/18648/3/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:

http://gerrit.cloudera.org:8080/#/c/18648/3/tests/query_test/test_iceberg.py@122
PS3, Line 122: 
> nit:
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/18648
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I812d19736c8e563541b038970786de7710b59f31
Gerrit-Change-Number: 18648
Gerrit-PatchSet: 4
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@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, 01 Jul 2022 08:05:11 +0000
Gerrit-HasComments: Yes