Nandor Kollar has posted comments on this change. ( http://gerrit.cloudera.org:8080/23801 )
Change subject: IMPALA-11926: Only load Avro schema in LocalFsTable if the format is Avro ...................................................................... Patch Set 8: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/23801/8/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java File fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java: http://gerrit.cloudera.org:8080/#/c/23801/8/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@135 PS8, Line 135: String avroSchema = null; nit: I'd consider a refactor here, create an overloaded constructor without avro schema parameter, something like this: private LocalFsTable(LocalDb db, Table msTbl, TableMetaRef ref, ColumnMap cmap) { this(db, msTbl, ref, cmap, null); } we can also eliminate the else branch of 'if (isAvroFormat(msTbl))' with a simple refactoring: public static LocalFsTable load(LocalDb db, Table msTbl, TableMetaRef ref) { String fullName = msTbl.getDbName() + "." + msTbl.getTableName(); // Set Avro schema if necessary. try { // If the table's format is Avro, then we should override the columns // based on the schema (either inferred or explicit). Otherwise, even if // there is an Avro schema set, we don't override the table-level columns: // the Avro schema in that case is just used in case there is an Avro-formatted // partition. ColumnMap cmap = ColumnMap.fromMsTable(msTbl); if (isAvroFormat(msTbl)) { // Load the avro schema if it's external (explicitly specified). String avroSchema = loadAvroSchema(msTbl); if (avroSchema == null) { // No Avro schema was explicitly set in the table metadata, so infer the Avro // schema from the column definitions. Schema inferredSchema = AvroSchemaConverter.convertFieldSchemas( msTbl.getSd().getCols(), fullName); avroSchema = inferredSchema.toString(); } List<FieldSchema> reconciledFieldSchemas = AvroSchemaUtils.reconcileAvroSchema( msTbl, avroSchema); Table msTblWithExplicitAvroSchema = msTbl.deepCopy(); msTblWithExplicitAvroSchema.getSd().setCols(reconciledFieldSchemas); cmap = ColumnMap.fromMsTable(msTblWithExplicitAvroSchema); return new LocalFsTable(db, msTbl, ref, cmap, avroSchema); } return new LocalFsTable(db, msTbl, ref, cmap); } catch (AnalysisException e) { throw new LocalCatalogException("Failed to load Avro schema for table " + fullName); } } -- To view, visit http://gerrit.cloudera.org:8080/23801 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I202665f978401894a1c837293529c06fa4270985 Gerrit-Change-Number: 23801 Gerrit-PatchSet: 8 Gerrit-Owner: Raghav Jindal <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Nandor Kollar <[email protected]> Gerrit-Reviewer: Noemi Pap-Takacs <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Raghav Jindal <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Wed, 30 Sep 2026 20:09:27 +0000 Gerrit-HasComments: Yes
