wangsheng has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16446 )

Change subject: IMPALA-10164: Supporting HadoopCatalog for Iceberg table
......................................................................


Patch Set 3:

(6 comments)

Done

http://gerrit.cloudera.org:8080/#/c/16446/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/16446/2//COMMIT_MSG@21
PS2, Line 21: We supported two values ('hadoop.catalog', 'hadoop.tables') for
            : 'iceberg.catalog'
> I'd list the currently possible values ('hadoop.catalog', 'hadoop.table').
Done


http://gerrit.cloudera.org:8080/#/c/16446/2//COMMIT_MSG@22
PS2, Line 22: now. If you don't specify this property i
> For me talking about external tables would be more readable in a separate p
Done


http://gerrit.cloudera.org:8080/#/c/16446/2//COMMIT_MSG@28
PS2, Line 28:
> nit: please avoid using tabs for indentation
Done


http://gerrit.cloudera.org:8080/#/c/16446/2/testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
File 
testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test:

http://gerrit.cloudera.org:8080/#/c/16446/2/testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test@56
PS2, Line 56: TBLPROPERTIES('iceberg_table_name'='fake_db.fake_table');
> Is it possible to create an external table using some invalid 'iceberg_tabl
Done


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

http://gerrit.cloudera.org:8080/#/c/16446/2/testdata/workloads/functional-query/queries/QueryTest/iceberg-query.test@191
PS2, Line 191: SELECT count(*) from hadoop_catalog_test_external
> I know that we can't write Iceberg tables at the moment but is there any wa
This test files are generated by spark-shell, if you want to test non-external 
tables with 'hadoop_catalog', we need to create Iceberg table by Impala first, 
and then generate data by spark-shell. This maybe difficult to implement by 
impala test.
Now we can only query empty Iceberg table created by Impala.


http://gerrit.cloudera.org:8080/#/c/16446/2/testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
File 
testdata/workloads/functional-query/queries/QueryTest/show-create-table.test:

http://gerrit.cloudera.org:8080/#/c/16446/2/testdata/workloads/functional-query/queries/QueryTest/show-create-table.test@585
PS2, Line 585: TBLPROPERTIES('iceberg_file_format'='parquet', 
'iceberg_catalog'='hadoop.catalog')
> Could you also add test coverage here for the default value of 'iceberg_cat
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic1893c50a633ca22d4bca6726c9937b026f5d5ef
Gerrit-Change-Number: 16446
Gerrit-PatchSet: 3
Gerrit-Owner: wangsheng <[email protected]>
Gerrit-Reviewer: Gabor Kaszab <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Reviewer: wangsheng <[email protected]>
Gerrit-Comment-Date: Mon, 21 Sep 2020 12:32:45 +0000
Gerrit-HasComments: Yes

Reply via email to