Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20077 )

Change subject: IMPALA-11013: Support 'MIGRATE TABLE' for external Hive tables
......................................................................


Patch Set 7:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/20077/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/20077/6//COMMIT_MSG@36
PS6, Line 36:
Please mention that it is a query option, and what is the default value.


http://gerrit.cloudera.org:8080/#/c/20077/6/fe/src/main/java/org/apache/impala/util/MigrateTableUtil.java
File fe/src/main/java/org/apache/impala/util/MigrateTableUtil.java:

http://gerrit.cloudera.org:8080/#/c/20077/6/fe/src/main/java/org/apache/impala/util/MigrateTableUtil.java@201
PS6, Line 201: importDataFilesImpl
Currently we are processing partitions one-by-one, paralellizing the file 
processing inside one partition.

Alternative would be to process the partitions in parallel which would be more 
efficient if there are many partitions. We could also choose between the two 
modes based on some ratio between #files / #partitions. We have all the 
information here to make this decision.

I don't expect to add this improvement in the current CR, but we could do it in 
a follow-up Jira.


http://gerrit.cloudera.org:8080/#/c/20077/6/tests/authorization/test_ranger.py
File tests/authorization/test_ranger.py:

http://gerrit.cloudera.org:8080/#/c/20077/6/tests/authorization/test_ranger.py@1966
PS6, Line 1966:         # Grant ALL privileges on the table for non-admin user. 
Even with this the query
              :         # should fail.
If we expect it to work in the future, do we have a Jira for this? And can we 
add the Jira ID here?


http://gerrit.cloudera.org:8080/#/c/20077/6/tests/authorization/test_ranger.py@1979
PS6, Line 1979:         self.execute_query_expect_success(
              :             non_admin_client, "alter table {0} convert to 
iceberg".format(tbl_name),
              :             user=user)
Can we add another statement to verify it is an Iceberg table? E.g. DESCRIBE 
FORMATTED or DESCRIBE HISTORY



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iacdad996d680fe545cc9a45e6bc64a348a64cd80
Gerrit-Change-Number: 20077
Gerrit-PatchSet: 7
Gerrit-Owner: Gabor Kaszab <[email protected]>
Gerrit-Reviewer: Anonymous Coward <[email protected]>
Gerrit-Reviewer: Gabor Kaszab <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Tamas Mate <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Wed, 28 Jun 2023 12:47:54 +0000
Gerrit-HasComments: Yes

Reply via email to