github-actions[bot] commented on code in PR #67784:
URL: https://github.com/apache/doris/pull/67784#discussion_r4003079512
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/hive/HiveMetaStoreClientHelper.java:
##########
@@ -648,8 +648,7 @@ public static Type hiveTypeToDorisType(String hiveType, int
timeScale, boolean e
case "string":
return ScalarType.createStringType();
case "binary":
- return enableMappingVarbinary ?
ScalarType.createVarbinaryType(VarBinaryType.MAX_VARBINARY_LENGTH)
- : ScalarType.createStringType();
+ return
ScalarType.createVarbinaryType(VarBinaryType.MAX_VARBINARY_LENGTH);
Review Comment:
[P1] Preserve Hive BINARY partition support
This mapping also changes Hive partition-column types to VARBINARY, but both
partition paths still reject that type. Existing non-null partitions fail in
`HiveExternalMetaCache.toListPartitionItem` because `PartitionValue.getValue`
reaches `LiteralExpr.create`, which has no VARBINARY case; dynamic inserts fail
separately because `VHiveTableWriter::_to_partition_value` has no
`TYPE_VARBINARY` case. Please define a lossless binary partition-name
representation and support it in both metadata materialization and writer
routing, or retain a partition-specific compatible mapping.
##########
be/src/core/data_type_serde/data_type_varbinary_serde.cpp:
##########
@@ -301,6 +324,24 @@ Status
DataTypeVarbinarySerDe::deserialize_one_cell_from_json(IColumn& column, S
return Status::OK();
}
+Status DataTypeVarbinarySerDe::from_string(StringRef& str, IColumn& column,
+ const FormatOptions& options) const
{
+ // Partition structs use the same hex representation as nested VARBINARY
output. Decode it
+ // before appending so arbitrary bytes survive JSON transport instead of
becoming NULL.
+ if (str.size < 2 || str.data[0] != '0' || str.data[1] != 'x' || (str.size
- 2) % 2 != 0 ||
Review Comment:
[P1] Carry static binary partitions end to end
Static Iceberg overwrite sends `Literal.getStringValue()` verbatim. For
`X'DEAD'` that is `DEAD`, while this parser accepts only `0xDEAD`, so a hybrid
static BINARY/FIXED/UUID partition fails before writing. Full-static mode
bypasses this parser but still fails during data-file commit when
`IcebergWriterHelper.convertToPartitionData` calls
`IcebergUtils.parsePartitionValueFromString`, whose switch has no
UUID/FIXED/BINARY cases. Please use one typed binary representation end to end
and cover full-static and hybrid overwrite.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergUtils.java:
##########
@@ -716,14 +716,16 @@ private static Type
icebergPrimitiveTypeToDorisType(org.apache.iceberg.types.Typ
case STRING:
return Type.STRING;
case UUID:
- return enableMappingVarbinary ?
ScalarType.createVarbinaryType(16) : Type.STRING;
+ return ScalarType.createVarbinaryType(16);
case BINARY:
- return enableMappingVarbinary ?
ScalarType.createVarbinaryType(VarBinaryType.MAX_VARBINARY_LENGTH)
- : Type.STRING;
+ // Arbitrary binary payloads are not valid UTF-8 in general,
so exposing them as
+ // STRING makes Arrow clients reject otherwise valid Iceberg
values.
+ return
ScalarType.createVarbinaryType(VarBinaryType.MAX_VARBINARY_LENGTH);
Review Comment:
[P1] Keep binary-partitioned Iceberg tables writable
These mappings make the sink pass `TYPE_VARBINARY` into the partition
writer, but that writer has no VARBINARY handling: `bucket`/`truncate` throw in
`PartitionColumnTransforms::create`, while dynamic `identity` gets past
construction and then throws in `_get_iceberg_partition_value` (and would also
fail path rendering). Thus non-null writes to existing
BINARY/FIXED/UUID-partitioned tables regress; the existing transform-write
suite already inserts into both bucket(binary) and truncate(binary) tables.
Please add raw-byte VARBINARY support across transform, extraction, and
path/commit serialization, or retain a compatible partition input type.
--
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]
