Zoltan Borok-Nagy 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:

(7 comments)

Thanks for working on this.

http://gerrit.cloudera.org:8080/#/c/19145/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19145/3//COMMIT_MSG@18
PS3, Line 18: 
Please mention the current limitations:

* no single files supported
* no partition clause

Probably with Jira IDs if separate tickets are created for these.


http://gerrit.cloudera.org:8080/#/c/19145/3/be/src/service/client-request-state.cc
File be/src/service/client-request-state.cc:

http://gerrit.cloudera.org:8080/#/c/19145/3/be/src/service/client-request-state.cc@803
PS3, Line 803: __isset
If this is an Iceberg table then all these statements must be set, right? 
Should we rather add a DCHECK for them?


http://gerrit.cloudera.org:8080/#/c/19145/3/be/src/service/client-request-state.cc@827
PS3, Line 827: No partitions selected for incremental stats update.
Please fix message.


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@264
PS3, Line 264: PARQUET
Use the same as the inferred file format. Please also add tests for ORC.


http://gerrit.cloudera.org:8080/#/c/19145/3/fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java@301
PS3, Line 301: "Could not infer file format."
> This error message could be misleading in case of a valid but not orc nor p
If the file format cannot be inferred from the filename, then maybe we could 
read the magic bytes of the file. Or we could try opening the file via 
Parquet/ORC file readers.


http://gerrit.cloudera.org:8080/#/c/19145/3/testdata/workloads/functional-query/queries/QueryTest/iceberg-load.test
File testdata/workloads/functional-query/queries/QueryTest/iceberg-load.test:

http://gerrit.cloudera.org:8080/#/c/19145/3/testdata/workloads/functional-query/queries/QueryTest/iceberg-load.test@44
PS3, Line 44: ====
Can we have tests for the case when there's a schema mismatch between table and 
file?


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

http://gerrit.cloudera.org:8080/#/c/19145/3/tests/query_test/test_iceberg.py@799
PS3, Line 799:
nit: +2 indent needed



--
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: Mon, 14 Nov 2022 15:38:14 +0000
Gerrit-HasComments: Yes

Reply via email to