Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/24049 )

Change subject: POC: LocalIcebergTable loads files in coordinator
......................................................................


Patch Set 4:

(11 comments)

gerrit-auto-critic failed. You can reproduce it locally using command:

  python3 bin/jenkins/critique-gerrit-review.py --dryrun

To run it, you might need a virtual env with Python3's venv installed.

http://gerrit.cloudera.org:8080/#/c/24049/4/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/24049/4/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java@820
PS4, Line 820:     LOG.trace("partIds size: {} want_partition_files: {} 
want_partition_metadata: {} want_partition_names: {} want_partition_stats {}",
line too long (135 > 90)


http://gerrit.cloudera.org:8080/#/c/24049/4/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
File fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java:

http://gerrit.cloudera.org:8080/#/c/24049/4/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@1277
PS4, Line 1277:     IcebergContentFileStoreCacheKey cacheKey = new 
IcebergContentFileStoreCacheKey(tableRef);
line too long (93 > 90)


http://gerrit.cloudera.org:8080/#/c/24049/4/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@1282
PS4, Line 1282:             public Pair<IcebergContentFileStore, 
ListMap<TNetworkAddress>> call() throws Exception {
line too long (100 > 90)


http://gerrit.cloudera.org:8080/#/c/24049/4/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@1283
PS4, Line 1283:               Pair<IcebergContentFileStore, 
ListMap<TNetworkAddress>> last = lastFileStore == null ? null : 
lastFileStore.get();
line too long (128 > 90)


http://gerrit.cloudera.org:8080/#/c/24049/4/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@1284
PS4, Line 1284:               ListMap<TNetworkAddress> hostIndex = last == null 
? new ListMap<>() : last.getSecond();
line too long (101 > 90)


http://gerrit.cloudera.org:8080/#/c/24049/4/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@1285
PS4, Line 1285:               IcebergContentFileStore oldFileStore = last == 
null ? null : last.getFirst();
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/24049/4/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@1287
PS4, Line 1287:                   
IcebergMetaProvider.getContentFileStore(tableRef, apiTable, hostIndex, 
oldFileStore);
line too long (103 > 90)


http://gerrit.cloudera.org:8080/#/c/24049/4/fe/src/main/java/org/apache/impala/catalog/local/IcebergMetaProvider.java
File fe/src/main/java/org/apache/impala/catalog/local/IcebergMetaProvider.java:

http://gerrit.cloudera.org:8080/#/c/24049/4/fe/src/main/java/org/apache/impala/catalog/local/IcebergMetaProvider.java@598
PS4, Line 598:           oldFileStore == null ? Collections.emptyList() : 
oldFileStore.getPartitionList(),
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/24049/4/fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.java
File fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.java:

http://gerrit.cloudera.org:8080/#/c/24049/4/fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.java@96
PS4, Line 96:       boolean loadFilesFromCatalog = 
!BackendConfig.INSTANCE.getloadIcebergFilesIncoordinator();
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/24049/4/fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.java@115
PS4, Line 115:             
db.getCatalog().getMetaProvider().getIcebergContentFileStore(ref, 
icebergApiTable);
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/24049/4/fe/src/test/java/org/apache/impala/catalog/local/MetaProviderDecoratorTest.java
File 
fe/src/test/java/org/apache/impala/catalog/local/MetaProviderDecoratorTest.java:

http://gerrit.cloudera.org:8080/#/c/24049/4/fe/src/test/java/org/apache/impala/catalog/local/MetaProviderDecoratorTest.java@142
PS4, Line 142:     
when(mockDecorated.loadIcebergTable(any(TableMetaRef.class),anyBoolean())) // 
TODO: different result if loadFiles=false?
line too long (124 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6732af76a2e040fa57e39260302951466037b934
Gerrit-Change-Number: 24049
Gerrit-PatchSet: 4
Gerrit-Owner: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Noemi Pap-Takacs <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Fri, 27 Feb 2026 20:00:24 +0000
Gerrit-HasComments: Yes

Reply via email to