OjashKush commented on code in PR #20012: URL: https://github.com/apache/hudi/pull/20012#discussion_r4230281907
########## hudi-trino/src/main/java/io/trino/plugin/hudi/util/HudiSchemaConverter.java: ########## @@ -0,0 +1,277 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.trino.plugin.hudi.util; + +import com.google.common.collect.ImmutableList; +import io.trino.spi.TrinoException; +import io.trino.spi.connector.ColumnMetadata; +import io.trino.spi.type.ArrayType; +import io.trino.spi.type.CharType; +import io.trino.spi.type.DecimalType; +import io.trino.spi.type.MapType; +import io.trino.spi.type.RowType; +import io.trino.spi.type.TimeType; +import io.trino.spi.type.TimestampType; +import io.trino.spi.type.TimestampWithTimeZoneType; +import io.trino.spi.type.Type; +import io.trino.spi.type.VarbinaryType; +import io.trino.spi.type.VarcharType; +import org.apache.hudi.common.schema.HoodieSchema; +import org.apache.hudi.common.schema.HoodieSchemaField; +import org.apache.hudi.common.schema.HoodieSchemaType; +import org.apache.hudi.common.schema.HoodieSchemaUtils; + +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +import static io.trino.spi.StandardErrorCode.NOT_SUPPORTED; +import static io.trino.spi.type.BigintType.BIGINT; +import static io.trino.spi.type.BooleanType.BOOLEAN; +import static io.trino.spi.type.DateType.DATE; +import static io.trino.spi.type.DoubleType.DOUBLE; +import static io.trino.spi.type.IntegerType.INTEGER; +import static io.trino.spi.type.RealType.REAL; +import static io.trino.spi.type.SmallintType.SMALLINT; +import static io.trino.spi.type.TinyintType.TINYINT; +import static io.trino.spi.type.UuidType.UUID; +import static java.lang.String.format; +import static org.apache.hudi.common.schema.HoodieSchemaUtils.getRecordQualifiedName; +import static org.apache.hudi.common.schema.HoodieSchemaUtils.sanitizeName; + +/** + * Converts a Trino column list into the {@link HoodieSchema} that becomes a table's + * {@code hoodie.table.create.schema}. + * <p> + * This direction did not previously exist in the connector: the read path only maps Hudi/Avro + * types into Trino types. The mapping here is the inverse of Trino's own + * {@code NativeLogicalTypesAvroTypeManager}, so a column created through this converter reads back + * as the same Trino type -- except where a mapping is deliberately widening, noted per case below. + * <p> + * Mappings that widen, so a column does not read back as the type it was declared with: + * <ul> + * <li>{@code TINYINT} and {@code SMALLINT} become Avro {@code int} and read back as + * {@code INTEGER}. Avro has no narrower integer, and Spark's Avro conversion widens the same + * way, so rejecting them would make the connector stricter than its peers for no gain.</li> + * <li>{@code VARCHAR(n)} becomes Avro {@code string} and reads back as unbounded {@code VARCHAR}. + * Avro strings carry no length bound; nothing enforces {@code n} once another engine writes.</li> + * <li>{@code TIMESTAMP(p)} for a {@code p} that is not exactly 3 or 6 rounds up to the next + * representable precision, since Avro offers only millisecond and microsecond logical types. + * The widening is lossless. Separately, the Hive Metastore's {@code timestamp} carries no + * precision at all, so every timestamp column reads back at the precision the connector + * requests from the metastore rather than the one it was declared with.</li> + * </ul> + * Types rejected outright, because the alternative is a mapping that is quietly wrong rather than + * merely wider: + * <ul> + * <li>{@code CHAR(n)} -- Avro has no fixed-width string, so the blank-padding semantics that + * distinguish {@code CHAR} from {@code VARCHAR} would be silently dropped.</li> + * <li>{@code TIMESTAMP(p) WITH TIME ZONE} -- Avro's timestamp logical types carry an instant, not + * an instant plus a zone, so the per-value zone would be lost.</li> + * <li>{@code TIMESTAMP(p)} beyond microsecond precision -- Avro's nanosecond logical types are not + * among those the connector's read path decodes, so such a column would be written and then be + * unreadable.</li> + * <li>{@code UUID} and {@code TIME(p)} -- both have a faithful Avro logical type, but neither has a + * Hive counterpart, so {@code HiveTypeTranslator#toHiveType} rejects them and the column could + * never be registered in the metastore. Since {@code HudiMetadata#getColumnHandles} reads + * columns from the metastore, such a column would also be unreadable. Rejecting here keeps the + * failure at the column that caused it.</li> + * <li>{@code MAP} with a non-{@code VARCHAR} key type -- Avro map keys are always strings.</li> + * <li>Unnamed {@code ROW} fields -- Avro record fields must be named.</li> + * </ul> + * {@code DECIMAL} maps to Avro {@code bytes} rather than {@code fixed}. Both are decodable by the + * read path, and {@code bytes} is what Hudi's own {@link HoodieSchema#createDecimal(int, int)} + * helper produces; {@code fixed} would additionally require inventing a unique schema name per + * decimal column, since Avro fixed types are named and must not collide within one schema. + * <p> + * Nullability: Trino carries nullability per column but not per element inside a {@code ROW}, + * {@code ARRAY} or {@code MAP}. A column's own {@link ColumnMetadata#isNullable()} is honoured at + * the top level; everything nested is made nullable, which is what Spark's Avro conversion also + * produces. + */ +public final class HudiSchemaConverter Review Comment: I’ll keep [#20012](https://github.com/apache/hudi/pull/20012) focused on Stage 1 DDL. I’ll address the bidirectional converter, consolidated type-rejection rules, and round-trip tests in a follow-up PR tracked by #20210 . -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
