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

Change subject: POC IMPALA-14805: LocalIcebergTable loads files in coordinator
......................................................................


Patch Set 7:

(14 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/7/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/7/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java@445
PS7, Line 445:     table.setIceberg_table(Utils.getTIcebergTable(this, 
ThriftObjectType.DESCRIPTOR_ONLY));
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/24049/7/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java@853
PS7, Line 853:     LOG.trace("partIds size: {} want_iceberg_table: {} 
want_partition_files: {} want_partition_metadata: {} want_partition_names: {} 
want_partition_stats {} want_table_constraints{}",
line too long (183 > 90)


http://gerrit.cloudera.org:8080/#/c/24049/7/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/7/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@1278
PS7, Line 1278:     IcebergContentFileStoreCacheKey cacheKey = new 
IcebergContentFileStoreCacheKey(tableRef);
line too long (93 > 90)


http://gerrit.cloudera.org:8080/#/c/24049/7/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@1284
PS7, Line 1284:               MetaProvider.CachedIcebergFiles last = 
lastFileStore == null ? null : lastFileStore.get();
line too long (104 > 90)


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


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


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


http://gerrit.cloudera.org:8080/#/c/24049/7/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@1304
PS7, Line 1304:         result.fileStore_.cloneWithoutMutables(), 
result.hostIndex_, result.partitionStats_);
line too long (93 > 90)


http://gerrit.cloudera.org:8080/#/c/24049/7/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/7/fe/src/main/java/org/apache/impala/catalog/local/IcebergMetaProvider.java@600
PS7, Line 600:   public static MetaProvider.CachedIcebergFiles 
getContentFileStore(final TableMetaRef table,
line too long (93 > 90)


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


http://gerrit.cloudera.org:8080/#/c/24049/7/fe/src/main/java/org/apache/impala/catalog/local/IcebergMetaProvider.java@631
PS7, Line 631:       return getContentFileStore(table, apiTable, hostIndex, 
null, false).fileStore_.toThrift();
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/24049/7/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/7/fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.java@110
PS7, Line 110:       boolean loadFilesFromCatalog = 
!BackendConfig.INSTANCE.getloadIcebergFilesIncoordinator();
line too long (96 > 90)


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


http://gerrit.cloudera.org:8080/#/c/24049/7/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/7/fe/src/test/java/org/apache/impala/catalog/local/MetaProviderDecoratorTest.java@142
PS7, 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: 7
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, 13 Mar 2026 08:29:48 +0000
Gerrit-HasComments: Yes

Reply via email to