Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23891 )
Change subject: Refactor FeTable ...................................................................... Patch Set 2: (25 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/23891/2/fe/src/main/java/org/apache/impala/analysis/CopyTestCaseStmt.java File fe/src/main/java/org/apache/impala/analysis/CopyTestCaseStmt.java: http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/analysis/CopyTestCaseStmt.java@172 PS2, Line 172: Collections.sort(referencedTbls, Comparator.comparing((FeTable t) -> t.getTableName().fullName())); line too long (103 > 90) http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/analysis/paimon/PaimonAnalyzer.java File fe/src/main/java/org/apache/impala/analysis/paimon/PaimonAnalyzer.java: http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/analysis/paimon/PaimonAnalyzer.java@97 PS2, Line 97: throw new AnalysisException("Table is not partitioned: " + table.getTableName()); line too long (91 > 90) http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/catalog/Column.java File fe/src/main/java/org/apache/impala/catalog/Column.java: http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/catalog/Column.java@163 PS2, Line 163: public static List<Column> filterColumnsNotStoredInHms(org.apache.hadoop.hive.metastore.api.Table tbl, List<Column> columns) { line too long (130 > 90) http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/catalog/Column.java@164 PS2, Line 164: boolean isFullAcid = tbl != null && AcidUtils.isFullAcidTable(tbl.getParameters()); line too long (91 > 90) http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java File fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java: http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java@337 PS2, Line 337: TTableType.DATA_SOURCE_TABLE, Column.toTColumnDescriptors(getColumns()), numClusteringCols_, line too long (100 > 90) http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java File fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java: http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java@168 PS2, Line 168: LOG.info("Sideload stats for {}.{}. {}", table.getTableName(), stats.getColName(), colStatsData); line too long (107 > 90) http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java@174 PS2, Line 174: "for {}.", table.getTableName(), col.getName(), col.getType(), table.getTableName()); line too long (101 > 90) http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/catalog/HBaseTable.java File fe/src/main/java/org/apache/impala/catalog/HBaseTable.java: http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/catalog/HBaseTable.java@172 PS2, Line 172: new TTableDescriptor(tableId, TTableType.HBASE_TABLE, Column.toTColumnDescriptors(getColumns()), line too long (104 > 90) http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java File fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java: http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java@635 PS2, Line 635: + " for writeIdList {}", getPartitionName(), getTable().getTableName(), line too long (91 > 90) http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java: http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@658 PS2, Line 658: accessLevel_ = getAvailableAccessLevel(getTableName().fullName(), tblLocation, permCache); line too long (94 > 90) http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@948 PS2, Line 948: TAccessLevel accessLevel = getAvailableAccessLevel(getTableName().fullName(), partDirPath, line too long (96 > 90) http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@2142 PS2, Line 2142: Column.toTColumnDescriptors(getColumns()), numClusteringCols_, name_, db_.getName()); line too long (93 > 90) http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@2357 PS2, Line 2357: numFilesFiltered, getTableName(), reqWriteIdList, getFileMetadataCacheHitRate()); line too long (91 > 90) http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@2389 PS2, Line 2389: getTableName(), isPartitioned() ? " partition " + part.getPartitionName() : "", line too long (91 > 90) http://gerrit.cloudera.org:8080/#/c/23891/2/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/23891/2/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java@790 PS2, Line 790: Column.toTColumnDescriptors(getColumns()), numClusteringCols_, name_, db_.getName()); line too long (93 > 90) http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/catalog/KuduTable.java File fe/src/main/java/org/apache/impala/catalog/KuduTable.java: http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@504 PS2, Line 504: Column.toTColumnDescriptors(getColumns()), numClusteringCols_, name_, db_.getName()); line too long (93 > 90) http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/catalog/Table.java File fe/src/main/java/org/apache/impala/catalog/Table.java: http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/catalog/Table.java@243 PS2, Line 243: LOG.debug("createEventId_ for table: {} set to: {}", getTableName(), createEventId_); line too long (91 > 90) http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/catalog/Table.java@483 PS2, Line 483: List<Column> columns = Column.filterColumnsNotStoredInHms(getMetaStoreTable(), getColumns()); line too long (97 > 90) http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java File fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java: http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java@119 PS2, Line 119: Column.toTColumnDescriptors(getColumns()), numClusteringCols_, name_, db_.getName()); line too long (93 > 90) http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/catalog/local/LocalPaimonTable.java File fe/src/main/java/org/apache/impala/catalog/local/LocalPaimonTable.java: http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/catalog/local/LocalPaimonTable.java@82 PS2, Line 82: TTableType.PAIMON_TABLE, Column.toTColumnDescriptors(getColumns()), 0, name_, db_.getName()); line too long (101 > 90) http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/catalog/paimon/PaimonTable.java File fe/src/main/java/org/apache/impala/catalog/paimon/PaimonTable.java: http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/catalog/paimon/PaimonTable.java@79 PS2, Line 79: new TTableDescriptor(tableId, TTableType.PAIMON_TABLE, Column.toTColumnDescriptors(getColumns()), line too long (105 > 90) http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/planner/HBaseTableSink.java File fe/src/main/java/org/apache/impala/planner/HBaseTableSink.java: http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/planner/HBaseTableSink.java@45 PS2, Line 45: output.append(prefix + "WRITE TO HBASE table=" + targetTable_.getTableName().fullName() + "\n"); line too long (100 > 90) http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java File fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java: http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@675 PS2, Line 675: "Failed to load data files for Iceberg table: %s", getIceTable().getTableName()), line too long (91 > 90) http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java: http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1208 PS2, Line 1208: "Table %s still has incomplete self-event registration.", table_.getTableName()); line too long (91 > 90) http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java File fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java: http://gerrit.cloudera.org:8080/#/c/23891/2/fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java@110 PS2, Line 110: Assert.assertTrue(t.getValidWriteIds().isWriteIdValid(MetastoreShim.getWriteIdFromMSTable(t.getMetaStoreTable()))); line too long (121 > 90) -- To view, visit http://gerrit.cloudera.org:8080/23891 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia39a77c11b921d19f7a0c42266ff350993a25d01 Gerrit-Change-Number: 23891 Gerrit-PatchSet: 2 Gerrit-Owner: Anonymous Coward <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Noemi Pap-Takacs <[email protected]> Gerrit-Reviewer: Peter Rozsa <[email protected]> Gerrit-Comment-Date: Thu, 22 Jan 2026 15:16:15 +0000 Gerrit-HasComments: Yes
