[email protected] has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19145 )

Change subject: IMPALA-11339: Add Iceberg LOAD DATA INPATH statement
......................................................................


Patch Set 3:

(4 comments)

Thanks for implementing this feature!
I left a few comments.

http://gerrit.cloudera.org:8080/#/c/19145/3/common/thrift/Frontend.thrift
File common/thrift/Frontend.thrift:

http://gerrit.cloudera.org:8080/#/c/19145/3/common/thrift/Frontend.thrift@380
PS3, Line 380: whith
nit: with?


http://gerrit.cloudera.org:8080/#/c/19145/3/fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
File fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java:

http://gerrit.cloudera.org:8080/#/c/19145/3/fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java@66
PS3, Line 66: // Set during analysis
nit: Maybe it can be omitted because of the same comment above(line 63)


http://gerrit.cloudera.org:8080/#/c/19145/3/fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java@233
PS3, Line 233:     StringBuilder createTmpTblQueryBuilder = new StringBuilder();
> Building the queries manually with string appends is a bit hard to follow/m
Yes, I agree.


http://gerrit.cloudera.org:8080/#/c/19145/3/fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java@316
PS3, Line 316: insertTblQuery_
If insert_tbl_query fails, will the temporary table created above be cleaned up?
If not, although it will not affect the user's re execution (because the suffix 
of the temporary table name is random uuid string), the temporary table should 
be manually cleaned up by the user. Should we consider how to remind the user 
about it?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8499945fa57ea0499f65b455976141dcd6d789eb
Gerrit-Change-Number: 19145
Gerrit-PatchSet: 3
Gerrit-Owner: Tamas Mate <[email protected]>
Gerrit-Reviewer: Anonymous Coward <[email protected]>
Gerrit-Reviewer: Gergely Fürnstáhl <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Tue, 08 Nov 2022 05:51:34 +0000
Gerrit-HasComments: Yes

Reply via email to