github-actions[bot] commented on code in PR #68786: URL: https://github.com/apache/doris/pull/68786#discussion_r4236295196
########## fe/fe-core/src/main/java/org/apache/doris/datasource/connector/converter/ConnectorWriteValueConverter.java: ########## @@ -0,0 +1,156 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you 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 org.apache.doris.datasource.connector.converter; + +import org.apache.doris.catalog.Column; +import org.apache.doris.nereids.trees.expressions.ArrayItemReference; +import org.apache.doris.nereids.trees.expressions.Cast; +import org.apache.doris.nereids.trees.expressions.Expression; +import org.apache.doris.nereids.trees.expressions.IsNull; +import org.apache.doris.nereids.trees.expressions.functions.scalar.ArrayMap; +import org.apache.doris.nereids.trees.expressions.functions.scalar.CreateMap; +import org.apache.doris.nereids.trees.expressions.functions.scalar.CreateNamedStruct; +import org.apache.doris.nereids.trees.expressions.functions.scalar.ElementAt; +import org.apache.doris.nereids.trees.expressions.functions.scalar.If; +import org.apache.doris.nereids.trees.expressions.functions.scalar.Lambda; +import org.apache.doris.nereids.trees.expressions.functions.scalar.MapEntries; +import org.apache.doris.nereids.trees.expressions.functions.scalar.MapFromEntries; +import org.apache.doris.nereids.trees.expressions.literal.IntegerLiteral; +import org.apache.doris.nereids.trees.expressions.literal.MapLiteral; +import org.apache.doris.nereids.trees.expressions.literal.NullLiteral; +import org.apache.doris.nereids.trees.expressions.literal.StringLikeLiteral; +import org.apache.doris.nereids.trees.expressions.literal.StringLiteral; +import org.apache.doris.nereids.trees.expressions.literal.StructLiteral; +import org.apache.doris.nereids.trees.expressions.literal.UuidLiteral; +import org.apache.doris.nereids.types.ArrayType; +import org.apache.doris.nereids.types.DataType; +import org.apache.doris.nereids.types.MapType; +import org.apache.doris.nereids.types.StructField; +import org.apache.doris.nereids.types.StructType; +import org.apache.doris.nereids.types.UuidType; +import org.apache.doris.nereids.util.TypeCoercionUtils; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +/** Applies connector-declared textual input semantics before ordinary sink type coercion. */ +public final class ConnectorWriteValueConverter { + private ConnectorWriteValueConverter() { + } + + public static Expression convert(Column column, Expression input) { + if (column.getConnectorStringWriteType() == null) { + return input; + } + return convert(input, DataType.fromCatalogType(column.getConnectorStringWriteType()), + DataType.fromCatalogType(column.getType())); + } + + private static Expression convert(Expression input, DataType semanticType, DataType targetType) { + if (!needsConversion(input.getDataType(), semanticType)) { + return input; + } + if (semanticType instanceof ArrayType) { + DataType elementSemantic = ((ArrayType) semanticType).getItemType(); + DataType elementTarget = ((ArrayType) targetType).getItemType(); + ArrayItemReference item = new ArrayItemReference("item", input); + return new ArrayMap(new Lambda(Collections.singletonList(item.getName()), + convert(item.toSlot(), elementSemantic, elementTarget), Collections.singletonList(item))); + } + if (semanticType instanceof MapType) { + MapType semantics = (MapType) semanticType; + MapType target = (MapType) targetType; + if (input instanceof CreateMap) { + List<Expression> children = new ArrayList<>(); + for (int i = 0; i < input.arity(); i++) { + DataType semantic = i % 2 == 0 ? semantics.getKeyType() : semantics.getValueType(); + DataType targetChild = i % 2 == 0 ? target.getKeyType() : target.getValueType(); + Expression child = input.child(i); + // A NULL-only map value has already been assigned TINYINT by generic function binding. + children.add(child instanceof NullLiteral ? new NullLiteral(targetChild) + : convert(child, semantic, targetChild)); + } + return TypeCoercionUtils.processBoundFunction(new CreateMap(children.toArray(new Expression[0]))); + } + if (input instanceof MapLiteral) { + // Preserve NULL leaves before collection binding assigns placeholder types. + return ((MapLiteral) input).checkedCastWithStrictChecking(targetType); + } + // Convert entries together so volatile map expressions are evaluated once and keys stay paired. + Expression entries = TypeCoercionUtils.processBoundFunction(new MapEntries(input)); + return TypeCoercionUtils.processBoundFunction(new MapFromEntries( + convert(entries, mapEntryArrayType(semantics), mapEntryArrayType(target)))); + } + if (semanticType instanceof StructType) { + List<StructField> semanticFields = ((StructType) semanticType).getFields(); + List<StructField> targetFields = ((StructType) targetType).getFields(); + List<Expression> fields = new ArrayList<>(); + for (int i = 0; i < semanticFields.size(); i++) { + Expression field = input instanceof StructLiteral ? ((StructLiteral) input).getValue().get(i) + : TypeCoercionUtils.processBoundFunction(new ElementAt(input, new IntegerLiteral(i + 1))); + fields.add(new StringLiteral(targetFields.get(i).getName())); Review Comment: [P2] Evaluate a UUID-bearing struct source once in inline VALUES writes. For a target `STRUCT<i:INT,u:UUID>`, a VALUES expression choosing between `(i=1,u=A)` and `(i=2,u=B)` with `IF(RAND()<0.5, ...)` reaches this loop before a source slot exists. The separate `ElementAt(input,1)` and `ElementAt(input,2)` calls each contain that volatile `IF`, so the stored struct can combine `i=1` with `u=B`. Materialize the input once before extracting fields, and cover a correlated VALUES case. ########## fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergPartitionUtils.java: ########## @@ -840,6 +889,15 @@ private static IcebergRawPartition generateRawPartition( int ordinal = partitionFieldOrdinals.get(partitionField.fieldId()); Object o = partitionData.get(ordinal, fieldClass); String fieldValue = o == null ? null : o.toString(); + Type fieldType = partitionSpec.partitionType().fields().get(i).type(); + valueTypes.add(IcebergTypeMapping.fromIcebergType(fieldType, true, true)); + // ByteBuffer.toString() describes its bounds, not the partition bytes. Use the + // transformed type so bucket(binary) remains an integer while identity/truncate keep bytes. + if (fieldType.typeId() == Type.TypeID.BINARY || fieldType.typeId() == Type.TypeID.FIXED) { + fieldValue = serializePartitionValue(fieldType, o, ZoneOffset.UTC); + } else if (fieldType.typeId() == Type.TypeID.UUID && o != null) { + fieldValue = "0x" + o.toString().replace("-", ""); + } Review Comment: [P2] Render UUID LIST values as UUID text. This pairs a UUID value type with `0x` plus compact hex, but `PluginDrivenMvccExternalTable.listPartitions` builds a UUID literal that accepts only canonical or unprefixed compact text. Its per-partition catch skips every non-null UUID identity partition. The Iceberg scan still reads its files, but EXPLAIN can show `partition=0/0`; under COUNT pushdown the zero selected-partition count can also bypass a `sql_block_rule partition_num` limit. Emit canonical UUID text for this typed value and cover UUID partition enumeration. ########## be/src/format/parquet/schema_desc.cpp: ########## @@ -316,12 +316,8 @@ std::pair<DataTypePtr, bool> FieldDescriptor::convert_to_doris_type( } else if (logicalType.__isset.JSON) { ans.first = DataTypeFactory::instance().create_data_type(TYPE_STRING, nullable); } else if (logicalType.__isset.UUID) { - if (_enable_mapping_varbinary) { - ans.first = DataTypeFactory::instance().create_data_type(TYPE_VARBINARY, nullable, -1, - -1, 16); - } else { - ans.first = DataTypeFactory::instance().create_data_type(TYPE_STRING, nullable); - } + // UUID is a logical type; its binary storage does not make it an untyped byte array. + ans.first = DataTypeFactory::instance().create_data_type(TYPE_UUID, nullable); } else if (logicalType.__isset.FLOAT16) { Review Comment: [P2] Preserve old-FE STRING UUID reads in V1 scanners. During a BE-first upgrade, an old FE with the default binary mapping still plans Iceberg UUID columns as STRING. This change makes the V1 Parquet file type UUID; `NativeUUIDConverter` then requests `ColumnTypeConverter(UUID, STRING)`, which is unsupported, so ordinary reads fail before rows. The parallel V1 ORC UUID mapping has the same gap. Support UUID-to-STRING conversion for old plans or retain their carrier contract, and cover an old-FE/new-BE read. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
