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

Change subject: IMPALA-9741: Support querying Iceberg table by impala
......................................................................


Patch Set 28:

(2 comments)

I was able to figure what was the problem with test_create_iceberg_tables.
You can reproduce the error with local catalog:

bin/start-impala-cluster.py --impalad_args --enable_minidumps=false 
--impalad_args --use_local_catalog=true --catalogd_args 
--catalog_topic_mode=minimal

bin/impala-py.test 
tests/query_test/test_iceberg.py::TestCreatingIcebergTable::test_create_iceberg_tables

The problem is that in CatalogService.doGetPartialCatalogObject() we use 
'IcebergTable.getHdfsTable()' in the response.
IcebergTable.hdfsTable_ is not necessarily the latest version of the table, as 
it is not updated in IcebergTable.load().
It means in the test we just return the very first, columnless version of the 
table.

I've added comments at the relevant places.

In my environment there's another problem unfortunately, as one of the DROP 
TABLE statements at the end of the test fails.

http://gerrit.cloudera.org:8080/#/c/16143/28/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:

http://gerrit.cloudera.org:8080/#/c/16143/28/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3199
PS28, Line 3199:               ((IcebergTable) table).getHdfsTable();
Here we return IcebergTable.hdfsTable_, which might be obsolete. See 
IcebergTable.load().


http://gerrit.cloudera.org:8080/#/c/16143/28/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
File fe/src/main/java/org/apache/impala/catalog/IcebergTable.java:

http://gerrit.cloudera.org:8080/#/c/16143/28/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java@185
PS28, Line 185:       hdfsTable_.load(false, msClient, msTbl, true, true, 
false, null, reason);
We load 'hdfsTable_' here, however the schema might be updated at L192. Because 
of this CatalogServiceCatalog's doGetPartialCatalogObject() will return this 
old version.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I856cfee4f3397d1a89cf17650e8d4fbfe1f2b006
Gerrit-Change-Number: 16143
Gerrit-PatchSet: 28
Gerrit-Owner: wangsheng <sky...@163.com>
Gerrit-Reviewer: Anonymous Coward (606)
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com>
Gerrit-Reviewer: wangsheng <sky...@163.com>
Gerrit-Comment-Date: Thu, 03 Sep 2020 14:26:50 +0000
Gerrit-HasComments: Yes

Reply via email to