You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Gabor Kaszab (Code Review)" <ge...@cloudera.org> on 2022/08/11 14:13:51 UTC

[Impala-ASF-CR] IMPALA-11429: Set table owner after creating an Iceberg table

Gabor Kaszab has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18837


Change subject: IMPALA-11429: Set table owner after creating an Iceberg table
......................................................................

IMPALA-11429: Set table owner after creating an Iceberg table

Iceberg tables are created using Apache Iceberg's API. However,
currently Iceberg gets the owner of the process running Iceberg for the
owner of the newly created tables. In our case it's the user running
catalogd and not the user running the CREATE TABLE statement.

Until the Iceberg API is enhanced to accept an owner parameter for
table creation, as a workaround this patch adds an extra alter table
step right after Iceberg table creation.

Testing:
  - Manually creating Iceberg tables and checking the owner.
  - Added one automated test to create Iceberg tables with different
    users and checking the output of DESCRIBE FORMATTED to verify the
    owner.

Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
1 file changed, 16 insertions(+), 1 deletion(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
Gerrit-Change-Number: 18837
Gerrit-PatchSet: 1
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>

[Impala-ASF-CR] IMPALA-11429: Set table owner after creating an Iceberg table

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

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

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

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

Change subject: IMPALA-11429: Set table owner after creating an Iceberg table
......................................................................

IMPALA-11429: Set table owner after creating an Iceberg table

Iceberg tables are created using Apache Iceberg's API. However,
currently Iceberg gets the owner of the process running Iceberg for the
owner of the newly created tables. In our case it's the user running
catalogd and not the user running the CREATE TABLE statement.

Until the Iceberg API is enhanced to accept an owner parameter for
table creation, as a workaround this patch adds an extra alter table
step right after Iceberg table creation.

TestIcebergTable.test_drop_incomplete_table test had to be skipped
with this implementation because this would run into a known bug where
Impala runs into an infinite loop when the table location is being
dropped somewhere during the execution of the first command after
CREATE TABLE. In this case the ALTER TABLE SET OWNER is going to be
the first command, so IMPALA-11509 would be triggered in case we
dropped the table location as this test would.

Testing:
  - Manually creating Iceberg tables and checking the owner.
  - Added one automated test to create Iceberg tables with different
    users and checking the output of DESCRIBE FORMATTED to verify the
    owner.
  - Turned off TestIcebergTable.test_drop_incomplete_table as described
    above.

Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M tests/query_test/test_iceberg.py
2 files changed, 63 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/18837/7
-- 
To view, visit http://gerrit.cloudera.org:8080/18837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
Gerrit-Change-Number: 18837
Gerrit-PatchSet: 7
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
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>

[Impala-ASF-CR] IMPALA-11429: Set table owner after creating an Iceberg table

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

Change subject: IMPALA-11429: Set table owner after creating an Iceberg table
......................................................................


Patch Set 8: Code-Review+2

Thanks for adding these changes, LGTM!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
Gerrit-Change-Number: 18837
Gerrit-PatchSet: 8
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
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-Comment-Date: Thu, 25 Aug 2022 15:25:23 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11429: Set table owner after creating an Iceberg table

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

Change subject: IMPALA-11429: Set table owner after creating an Iceberg table
......................................................................


Patch Set 5:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/18837/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/18837/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3626
PS5, Line 3626: it has
              :         // hardcoded that the
nit: this part was fuzzy for me, maybe something like:
"however, the table owner is hardcoded to be the user running the Iceberg process"


http://gerrit.cloudera.org:8080/#/c/18837/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3633
PS5, Line 3633:         throw new ImpalaRuntimeException("Failed to set table owner after creating " +
              :             "Iceberg table but the table has been created successfully", e);
With Ranger enabled and a failing alter operation this table would be inaccessible for the user right?


http://gerrit.cloudera.org:8080/#/c/18837/5/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:

http://gerrit.cloudera.org:8080/#/c/18837/5/tests/query_test/test_iceberg.py@130
PS5, Line 130: pytest.skip()
Wouldn't a time.sleep(5) solve the issue after create?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
Gerrit-Change-Number: 18837
Gerrit-PatchSet: 5
Gerrit-Owner: 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-Comment-Date: Fri, 19 Aug 2022 13:48:34 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11429: Set table owner after creating an Iceberg table

Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-11429: Set table owner after creating an Iceberg table
......................................................................

IMPALA-11429: Set table owner after creating an Iceberg table

Iceberg tables are created using Apache Iceberg's API. However,
currently Iceberg gets the owner of the process running Iceberg for the
owner of the newly created tables. In our case it's the user running
catalogd and not the user running the CREATE TABLE statement.

Until the Iceberg API is enhanced to accept an owner parameter for
table creation, as a workaround this patch adds an extra alter table
step right after Iceberg table creation.

TestIcebergTable.test_drop_incomplete_table test had to be skipped
with this implementation because this would run into a known bug where
Impala runs into an infinite loop when the table location is being
dropped somewhere during the execution of the first command after
CREATE TABLE. In this case the ALTER TABLE SET OWNER is going to be
the first command, so IMPALA-11509 would be triggered in case we
dropped the table location as this test would.

Testing:
  - Manually creating Iceberg tables and checking the owner.
  - Added one automated test to create Iceberg tables with different
    users and checking the output of DESCRIBE FORMATTED to verify the
    owner.
  - Turned off TestIcebergTable.test_drop_incomplete_table as described
    above.

Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M tests/query_test/test_iceberg.py
2 files changed, 62 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/18837/5
-- 
To view, visit http://gerrit.cloudera.org:8080/18837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
Gerrit-Change-Number: 18837
Gerrit-PatchSet: 5
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-11429: Set table owner after creating an Iceberg table

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

Change subject: IMPALA-11429: Set table owner after creating an Iceberg table
......................................................................


Patch Set 7:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/18837/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/18837/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3633
PS5, Line 3633:         throw new ImpalaRuntimeException("Failed to set table owner after creating " +
              :             "Iceberg table but the table has been created successfully", e);
> I was thinking about a rollback, which is probably a drop table in this cas
Rollback would in fact be a good choice here but I don't think I should implement that at this point for the following reasons:
- If the ALTER TABLE part fails then we are where we were without this patch so actually we haven't made made anything worse, but with the ALTER TABLE part succeeding we get to the state we wanted to.
- This ALTER TABLE step is a temporary one until we manage to enhance the Iceberg API to also accept a table owner during table creation.
- Adding one more step would result a CREATE TABLE as a 3 step operation in some use cases and I feel that over complicated and too error prone.
- If the "automatic" ALTER TABLE fails there is still the opportunity to run a manual ALTER TABLE with a user with enough permissions to fix the table.


http://gerrit.cloudera.org:8080/#/c/18837/5/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:

http://gerrit.cloudera.org:8080/#/c/18837/5/tests/query_test/test_iceberg.py@130
PS5, Line 130: loading fails
> Thank you for explaining. Could you add this Jira to the above comment as w
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
Gerrit-Change-Number: 18837
Gerrit-PatchSet: 7
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
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-Comment-Date: Tue, 23 Aug 2022 08:47:08 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11429: Set table owner after creating an Iceberg table

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

Change subject: IMPALA-11429: Set table owner after creating an Iceberg table
......................................................................


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18837/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/18837/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3633
PS5, Line 3633:       } catch (Exception e) {
              :         LOG.warn("Failed to set table owner after creating " +
> I see your points, 3 operations indeed a bit too much, I am happy to go wit
I agree this shouldn't be an error. I changed this part to simply write a warning log, and return with success while showing a message to the user that the table is created but the owner wasn't changed.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
Gerrit-Change-Number: 18837
Gerrit-PatchSet: 8
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
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-Comment-Date: Thu, 25 Aug 2022 11:25:15 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11429: Set table owner after creating an Iceberg table

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

Change subject: IMPALA-11429: Set table owner after creating an Iceberg table
......................................................................


Patch Set 3:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
Gerrit-Change-Number: 18837
Gerrit-PatchSet: 3
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Aug 2022 16:21:33 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11429: Set table owner after creating an Iceberg table

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/18837 )

Change subject: IMPALA-11429: Set table owner after creating an Iceberg table
......................................................................


Patch Set 5: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18837/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/18837/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3626
PS5, Line 3626: has
nit: "is" feels a bit more natural



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
Gerrit-Change-Number: 18837
Gerrit-PatchSet: 5
Gerrit-Owner: 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-Comment-Date: Fri, 19 Aug 2022 13:28:30 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11429: Set table owner after creating an Iceberg table

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

Change subject: IMPALA-11429: Set table owner after creating an Iceberg table
......................................................................


Patch Set 8:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
Gerrit-Change-Number: 18837
Gerrit-PatchSet: 8
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
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-Comment-Date: Thu, 25 Aug 2022 11:43:02 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11429: Set table owner after creating an Iceberg table

Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-11429: Set table owner after creating an Iceberg table
......................................................................

IMPALA-11429: Set table owner after creating an Iceberg table

Iceberg tables are created using Apache Iceberg's API. However,
currently Iceberg gets the owner of the process running Iceberg for the
owner of the newly created tables. In our case it's the user running
catalogd and not the user running the CREATE TABLE statement.

Until the Iceberg API is enhanced to accept an owner parameter for
table creation, as a workaround this patch adds an extra alter table
step right after Iceberg table creation.

Testing:
  - Manually creating Iceberg tables and checking the owner.
  - Added one automated test to create Iceberg tables with different
    users and checking the output of DESCRIBE FORMATTED to verify the
    owner.

Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M tests/query_test/test_iceberg.py
2 files changed, 57 insertions(+), 0 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
Gerrit-Change-Number: 18837
Gerrit-PatchSet: 4
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-11429: Set table owner after creating an Iceberg table

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

Change subject: IMPALA-11429: Set table owner after creating an Iceberg table
......................................................................


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/18837/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/18837/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3633
PS5, Line 3633:         throw new ImpalaRuntimeException("Failed to set table owner after creating " +
              :             "Iceberg table but the table has been created successfully", e);
> Well yes, if the ALTER TABLE fails then we have the same situation we are n
I was thinking about a rollback, which is probably a drop table in this case, then fail the operation.
If I understand it well, HMS is accessed through Iceberg, so a rollback/drop table should remove it form the Iceberg catalog and HMS as well.


http://gerrit.cloudera.org:8080/#/c/18837/5/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:

http://gerrit.cloudera.org:8080/#/c/18837/5/tests/query_test/test_iceberg.py@130
PS5, Line 130: pytest.skip()
> If I added a sleep than instead of IMPALA-11509 we would bump into IMPALA-1
Thank you for explaining. Could you add this Jira to the above comment as well?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
Gerrit-Change-Number: 18837
Gerrit-PatchSet: 5
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
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-Comment-Date: Mon, 22 Aug 2022 14:17:28 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11429: Set table owner after creating an Iceberg table

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

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

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

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

Change subject: IMPALA-11429: Set table owner after creating an Iceberg table
......................................................................

IMPALA-11429: Set table owner after creating an Iceberg table

Iceberg tables are created using Apache Iceberg's API. However,
currently Iceberg gets the owner of the process running Iceberg for the
owner of the newly created tables. In our case it's the user running
catalogd and not the user running the CREATE TABLE statement.

Until the Iceberg API is enhanced to accept an owner parameter for
table creation, as a workaround this patch adds an extra alter table
step right after Iceberg table creation.

TestIcebergTable.test_drop_incomplete_table test had to be skipped
with this implementation because this would run into a known bug where
Impala runs into an infinite loop when the table location is being
dropped somewhere during the execution of the first command after
CREATE TABLE. In this case the ALTER TABLE SET OWNER is going to be
the first command, so IMPALA-11509 would be triggered in case we
dropped the table location as this test would.

Testing:
  - Manually creating Iceberg tables and checking the owner.
  - Added one automated test to create Iceberg tables with different
    users and checking the output of DESCRIBE FORMATTED to verify the
    owner.
  - Turned off TestIcebergTable.test_drop_incomplete_table as described
    above.
  - Manually tested when ALTER TABLE fails (hacked Impala code) that
    the table is created properly.

Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M tests/query_test/test_iceberg.py
2 files changed, 69 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/18837/8
-- 
To view, visit http://gerrit.cloudera.org:8080/18837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
Gerrit-Change-Number: 18837
Gerrit-PatchSet: 8
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
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>

[Impala-ASF-CR] IMPALA-11429: Set table owner after creating an Iceberg table

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

Change subject: IMPALA-11429: Set table owner after creating an Iceberg table
......................................................................


Patch Set 9:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
Gerrit-Change-Number: 18837
Gerrit-PatchSet: 9
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
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-Comment-Date: Thu, 25 Aug 2022 21:38:55 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11429: Set table owner after creating an Iceberg table

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

Change subject: IMPALA-11429: Set table owner after creating an Iceberg table
......................................................................


Patch Set 9: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
Gerrit-Change-Number: 18837
Gerrit-PatchSet: 9
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
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-Comment-Date: Fri, 26 Aug 2022 02:24:42 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11429: Set table owner after creating an Iceberg table

Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-11429: Set table owner after creating an Iceberg table
......................................................................

IMPALA-11429: Set table owner after creating an Iceberg table

Iceberg tables are created using Apache Iceberg's API. However,
currently Iceberg gets the owner of the process running Iceberg for the
owner of the newly created tables. In our case it's the user running
catalogd and not the user running the CREATE TABLE statement.

Until the Iceberg API is enhanced to accept an owner parameter for
table creation, as a workaround this patch adds an extra alter table
step right after Iceberg table creation.

Testing:
  - Manually creating Iceberg tables and checking the owner.
  - Added one automated test to create Iceberg tables with different
    users and checking the output of DESCRIBE FORMATTED to verify the
    owner.

Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M tests/query_test/test_iceberg.py
2 files changed, 57 insertions(+), 0 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
Gerrit-Change-Number: 18837
Gerrit-PatchSet: 2
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-11429: Set table owner after creating an Iceberg table

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

Change subject: IMPALA-11429: Set table owner after creating an Iceberg table
......................................................................


Patch Set 4:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
Gerrit-Change-Number: 18837
Gerrit-PatchSet: 4
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Aug 2022 16:25:53 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11429: Set table owner after creating an Iceberg table

Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-11429: Set table owner after creating an Iceberg table
......................................................................

IMPALA-11429: Set table owner after creating an Iceberg table

Iceberg tables are created using Apache Iceberg's API. However,
currently Iceberg gets the owner of the process running Iceberg for the
owner of the newly created tables. In our case it's the user running
catalogd and not the user running the CREATE TABLE statement.

Until the Iceberg API is enhanced to accept an owner parameter for
table creation, as a workaround this patch adds an extra alter table
step right after Iceberg table creation.

Testing:
  - Manually creating Iceberg tables and checking the owner.
  - Added one automated test to create Iceberg tables with different
    users and checking the output of DESCRIBE FORMATTED to verify the
    owner.

Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M tests/query_test/test_iceberg.py
2 files changed, 60 insertions(+), 1 deletion(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
Gerrit-Change-Number: 18837
Gerrit-PatchSet: 3
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-11429: Set table owner after creating an Iceberg table

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

Change subject: IMPALA-11429: Set table owner after creating an Iceberg table
......................................................................


Patch Set 6:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
Gerrit-Change-Number: 18837
Gerrit-PatchSet: 6
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
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-Comment-Date: Mon, 22 Aug 2022 12:45:32 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11429: Set table owner after creating an Iceberg table

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/18837 )

Change subject: IMPALA-11429: Set table owner after creating an Iceberg table
......................................................................

IMPALA-11429: Set table owner after creating an Iceberg table

Iceberg tables are created using Apache Iceberg's API. However,
currently Iceberg gets the owner of the process running Iceberg for the
owner of the newly created tables. In our case it's the user running
catalogd and not the user running the CREATE TABLE statement.

Until the Iceberg API is enhanced to accept an owner parameter for
table creation, as a workaround this patch adds an extra alter table
step right after Iceberg table creation.

TestIcebergTable.test_drop_incomplete_table test had to be skipped
with this implementation because this would run into a known bug where
Impala runs into an infinite loop when the table location is being
dropped somewhere during the execution of the first command after
CREATE TABLE. In this case the ALTER TABLE SET OWNER is going to be
the first command, so IMPALA-11509 would be triggered in case we
dropped the table location as this test would.

Testing:
  - Manually creating Iceberg tables and checking the owner.
  - Added one automated test to create Iceberg tables with different
    users and checking the output of DESCRIBE FORMATTED to verify the
    owner.
  - Turned off TestIcebergTable.test_drop_incomplete_table as described
    above.
  - Manually tested when ALTER TABLE fails (hacked Impala code) that
    the table is created properly.

Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
Reviewed-on: http://gerrit.cloudera.org:8080/18837
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M tests/query_test/test_iceberg.py
2 files changed, 69 insertions(+), 5 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
Gerrit-Change-Number: 18837
Gerrit-PatchSet: 10
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
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>

[Impala-ASF-CR] IMPALA-11429: Set table owner after creating an Iceberg table

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

Change subject: IMPALA-11429: Set table owner after creating an Iceberg table
......................................................................


Patch Set 6:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/18837/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/18837/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3626
PS5, Line 3626: le 
> nit: "is" feels a bit more natural
Done


http://gerrit.cloudera.org:8080/#/c/18837/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3626
PS5, Line 3626: table owner
              :         // is hardcoded to be
> nit: this part was fuzzy for me, maybe something like:
Done


http://gerrit.cloudera.org:8080/#/c/18837/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3633
PS5, Line 3633:         throw new ImpalaRuntimeException("Failed to set table owner after creating " +
              :             "Iceberg table but the table has been created successfully", e);
> With Ranger enabled and a failing alter operation this table would be inacc
Well yes, if the ALTER TABLE fails then we have the same situation we are now without this patch. I don't see there is anything we can do about it but I'm open for ideas :)
The ideal solution would be to provide owner param to Iceberg API and make this one step, but the API is not prepared for that ATM.


http://gerrit.cloudera.org:8080/#/c/18837/5/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:

http://gerrit.cloudera.org:8080/#/c/18837/5/tests/query_test/test_iceberg.py@130
PS5, Line 130: pytest.skip()
> Wouldn't a time.sleep(5) solve the issue after create?
If I added a sleep than instead of IMPALA-11509 we would bump into IMPALA-11502 that wouldn't make Impala stuck in an infinite loop but would still fail this test.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
Gerrit-Change-Number: 18837
Gerrit-PatchSet: 6
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
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-Comment-Date: Mon, 22 Aug 2022 12:25:11 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11429: Set table owner after creating an Iceberg table

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

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

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

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

Change subject: IMPALA-11429: Set table owner after creating an Iceberg table
......................................................................

IMPALA-11429: Set table owner after creating an Iceberg table

Iceberg tables are created using Apache Iceberg's API. However,
currently Iceberg gets the owner of the process running Iceberg for the
owner of the newly created tables. In our case it's the user running
catalogd and not the user running the CREATE TABLE statement.

Until the Iceberg API is enhanced to accept an owner parameter for
table creation, as a workaround this patch adds an extra alter table
step right after Iceberg table creation.

TestIcebergTable.test_drop_incomplete_table test had to be skipped
with this implementation because this would run into a known bug where
Impala runs into an infinite loop when the table location is being
dropped somewhere during the execution of the first command after
CREATE TABLE. In this case the ALTER TABLE SET OWNER is going to be
the first command, so IMPALA-11509 would be triggered in case we
dropped the table location as this test would.

Testing:
  - Manually creating Iceberg tables and checking the owner.
  - Added one automated test to create Iceberg tables with different
    users and checking the output of DESCRIBE FORMATTED to verify the
    owner.
  - Turned off TestIcebergTable.test_drop_incomplete_table as described
    above.

Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M tests/query_test/test_iceberg.py
2 files changed, 62 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/18837/6
-- 
To view, visit http://gerrit.cloudera.org:8080/18837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
Gerrit-Change-Number: 18837
Gerrit-PatchSet: 6
Gerrit-Owner: 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>

[Impala-ASF-CR] IMPALA-11429: Set table owner after creating an Iceberg table

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

Change subject: IMPALA-11429: Set table owner after creating an Iceberg table
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18837/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/18837/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3633
PS5, Line 3633:         throw new ImpalaRuntimeException("Failed to set table owner after creating " +
              :             "Iceberg table but the table has been created successfully", e);
> Rollback would in fact be a good choice here but I don't think I should imp
I see your points, 3 operations indeed a bit too much, I am happy to go with this solution.
One more question, does this exception propagated to the user?  Looks like throwing an exception here would trigger another ImpalaRuntimeException instead of a "successful" operation similar to "Table already exists.".



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
Gerrit-Change-Number: 18837
Gerrit-PatchSet: 5
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
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-Comment-Date: Wed, 24 Aug 2022 11:57:43 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11429: Set table owner after creating an Iceberg table

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

Change subject: IMPALA-11429: Set table owner after creating an Iceberg table
......................................................................


Patch Set 2:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
Gerrit-Change-Number: 18837
Gerrit-PatchSet: 2
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Aug 2022 14:41:54 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11429: Set table owner after creating an Iceberg table

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

Change subject: IMPALA-11429: Set table owner after creating an Iceberg table
......................................................................


Patch Set 1:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
Gerrit-Change-Number: 18837
Gerrit-PatchSet: 1
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Aug 2022 14:34:25 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11429: Set table owner after creating an Iceberg table

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

Change subject: IMPALA-11429: Set table owner after creating an Iceberg table
......................................................................


Patch Set 3:

(13 comments)

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

http://gerrit.cloudera.org:8080/#/c/18837/3/tests/query_test/test_iceberg.py@43
PS3, Line 43: from tests.shell.util import run_impala_shell_cmd
flake8: E402 module level import not at top of file


http://gerrit.cloudera.org:8080/#/c/18837/3/tests/query_test/test_iceberg.py@45
PS3, Line 45: class TestIcebergTable(ImpalaTestSuite):
flake8: E302 expected 2 blank lines, found 1


http://gerrit.cloudera.org:8080/#/c/18837/3/tests/query_test/test_iceberg.py@742
PS3, Line 742: <<<<<<< 0ce2403b58afd94877e36e1647ec34e8a9df7dba
flake8: E305 expected 2 blank lines after class or function definition, found 1


http://gerrit.cloudera.org:8080/#/c/18837/3/tests/query_test/test_iceberg.py@742
PS3, Line 742: <
flake8: E999 SyntaxError: invalid syntax


http://gerrit.cloudera.org:8080/#/c/18837/3/tests/query_test/test_iceberg.py@742
PS3, Line 742: <
flake8: E227 missing whitespace around bitwise or shift operator


http://gerrit.cloudera.org:8080/#/c/18837/3/tests/query_test/test_iceberg.py@742
PS3, Line 742: <
flake8: E225 missing whitespace around operator


http://gerrit.cloudera.org:8080/#/c/18837/3/tests/query_test/test_iceberg.py@750
PS3, Line 750: =
flake8: E225 missing whitespace around operator


http://gerrit.cloudera.org:8080/#/c/18837/3/tests/query_test/test_iceberg.py@750
PS3, Line 750: =
flake8: E225 missing whitespace around operator


http://gerrit.cloudera.org:8080/#/c/18837/3/tests/query_test/test_iceberg.py@750
PS3, Line 750: =
flake8: E225 missing whitespace around operator


http://gerrit.cloudera.org:8080/#/c/18837/3/tests/query_test/test_iceberg.py@751
PS3, Line 751: d
flake8: E113 unexpected indentation


http://gerrit.cloudera.org:8080/#/c/18837/3/tests/query_test/test_iceberg.py@759
PS3, Line 759: d
flake8: E126 continuation line over-indented for hanging indent


http://gerrit.cloudera.org:8080/#/c/18837/3/tests/query_test/test_iceberg.py@780
PS3, Line 780: >
flake8: E225 missing whitespace around operator


http://gerrit.cloudera.org:8080/#/c/18837/3/tests/query_test/test_iceberg.py@780
PS3, Line 780: -
flake8: E226 missing whitespace around arithmetic operator



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
Gerrit-Change-Number: 18837
Gerrit-PatchSet: 3
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Aug 2022 16:00:34 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11429: Set table owner after creating an Iceberg table

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

Change subject: IMPALA-11429: Set table owner after creating an Iceberg table
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/18837/4/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:

http://gerrit.cloudera.org:8080/#/c/18837/4/tests/query_test/test_iceberg.py@43
PS4, Line 43: from tests.shell.util import run_impala_shell_cmd
flake8: E402 module level import not at top of file


http://gerrit.cloudera.org:8080/#/c/18837/4/tests/query_test/test_iceberg.py@758
PS4, Line 758: d
flake8: E126 continuation line over-indented for hanging indent



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
Gerrit-Change-Number: 18837
Gerrit-PatchSet: 4
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Aug 2022 16:06:09 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11429: Set table owner after creating an Iceberg table

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

Change subject: IMPALA-11429: Set table owner after creating an Iceberg table
......................................................................


Patch Set 7:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
Gerrit-Change-Number: 18837
Gerrit-PatchSet: 7
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
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-Comment-Date: Tue, 23 Aug 2022 09:07:53 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11429: Set table owner after creating an Iceberg table

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

Change subject: IMPALA-11429: Set table owner after creating an Iceberg table
......................................................................


Patch Set 9: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
Gerrit-Change-Number: 18837
Gerrit-PatchSet: 9
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
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-Comment-Date: Thu, 25 Aug 2022 21:38:54 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11429: Set table owner after creating an Iceberg table

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

Change subject: IMPALA-11429: Set table owner after creating an Iceberg table
......................................................................


Patch Set 5:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5cac198a4a53be3599cb582864ee5f8c269202c0
Gerrit-Change-Number: 18837
Gerrit-PatchSet: 5
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Aug 2022 13:15:58 +0000
Gerrit-HasComments: No