github-actions[bot] commented on code in PR #68786:
URL: https://github.com/apache/doris/pull/68786#discussion_r4237173029
##########
fe/be-java-extensions/jdbc-scanner/src/main/java/org/apache/doris/jdbc/OracleTypeHandler.java:
##########
@@ -108,6 +111,15 @@ public PreparedStatement initializeStatement(Connection
conn, String sql,
int fetchSize) throws
SQLException {
// Detect driver version when creating the statement (first time we
have access to connection)
detectDriverVersion(conn);
+ // ALTER SESSION alone leaves the driver's TSLTZ zone unset.
Initialize every borrowed connection.
+ try {
+ Connection physical = conn.unwrap(Connection.class);
+ Class<?> oracleConnection =
Class.forName("oracle.jdbc.OracleConnection", true,
Review Comment:
[P1] Preserve Oracle-mode OceanBase scans. OCEANBASE_ORACLE selects this
handler, but those catalogs use com.oceanbase.jdbc.Driver. This unconditional
lookup and unwrap require oracle.jdbc.OracleConnection from the OceanBase
driver classloader before any SQL is prepared, so ordinary scans fail with the
session-time-zone initialization error, even when no TIMESTAMPTZ column is
read. Apply this Oracle-specific call only to Oracle JDBC connections and use
an OceanBase-compatible setup for its mode; cover an Oracle-mode OceanBase scan.
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergTypeMapping.java:
##########
@@ -124,28 +124,25 @@ private static ConnectorType
fromPrimitive(Type.PrimitiveType primitive,
case STRING:
return ConnectorType.of("STRING");
case UUID:
- return enableMappingVarbinary
- ? ConnectorType.of("VARBINARY", 16, 0) :
ConnectorType.of("STRING");
+ // Preserve logical UUID semantics independently of the binary
mapping option.
+ return ConnectorType.of("UUID");
case BINARY:
// Iceberg BINARY is unbounded. Emit VARBINARY with NO
explicit length so
// ConnectorColumnConverter applies
ScalarType.MAX_VARBINARY_LENGTH — byte-identical to
// legacy IcebergUtils
createVarbinaryType(VarBinaryType.MAX_VARBINARY_LENGTH). A
// concrete length (e.g. 65535) would render a different
DESCRIBE / SHOW CREATE type.
- return enableMappingVarbinary
- ? ConnectorType.of("VARBINARY") :
ConnectorType.of("STRING");
+ // Binary payloads need not be valid UTF-8.
+ return ConnectorType.of("VARBINARY");
case FIXED:
int fixedLen = ((Types.FixedType) primitive).length();
- return enableMappingVarbinary
- ? ConnectorType.of("VARBINARY", fixedLen, 0)
- : ConnectorType.of("CHAR", fixedLen, 0);
+ return ConnectorType.of("VARBINARY", fixedLen, 0);
case DECIMAL:
Types.DecimalType decimal = (Types.DecimalType) primitive;
return ConnectorType.of("DECIMALV3", decimal.precision(),
decimal.scale());
case DATE:
return ConnectorType.of("DATEV2");
case TIMESTAMP:
- if (enableMappingTimestampTz
- && ((Types.TimestampType)
primitive).shouldAdjustToUTC()) {
+ if (((Types.TimestampType) primitive).shouldAdjustToUTC()) {
Review Comment:
[P2] Preserve partitioned Iceberg INSERTs during an FE-first upgrade. A new
FE now sends TIMESTAMPTZ (and native UUID) write slots, but an older BE has no
cases for these types in Iceberg partition transforms or
_get_iceberg_partition_value. An identity-partitioned write reaches Unsupported
type for partition; bucket/time transforms can fail earlier. Gate typed write
plans on BE capability or keep compatible carriers until all BEs are upgraded,
and cover a mixed-version partitioned INSERT. The existing UUID scan issue is a
separate reader path.
##########
be/src/format/transformer/vorc_transformer.cpp:
##########
@@ -424,6 +424,11 @@ std::unique_ptr<orc::Type>
VOrcTransformer::_build_orc_type(
};
switch (nested_field->field_type()->type_id()) {
case iceberg::TypeID::UUID:
+ // Native UUID serde already writes network-order bytes; retain
its ORC annotation.
+ if (primitive_type == TYPE_UUID) {
Review Comment:
[P1] Gate native UUID Iceberg ORC writes until every BE supports them. An
upgraded FE now sends TYPE_UUID for an Iceberg UUID column, including on an
unpartitioned ORC table. An older BE reaches use_iceberg_binary_type in
VOrcTransformer::_build_orc_type, whose DORIS_CHECK accepts only string,
varbinary, or binary, and aborts the BE while opening the writer. This new
TYPE_UUID branch exists only on upgraded BEs. Retain the old write carrier or
gate this plan on BE capability; cover an FE-first unpartitioned ORC INSERT.
##########
fe/fe-connector/fe-connector-maxcompute/src/main/java/org/apache/doris/connector/maxcompute/MCTypeMapping.java:
##########
@@ -87,6 +87,8 @@ public static ConnectorType toConnectorType(TypeInfo
typeInfo) {
case DATETIME:
return ConnectorType.of("DATETIMEV2", 3, 0);
case TIMESTAMP:
+ // MaxCompute TIMESTAMP is an instant; TIMESTAMP_NTZ remains a
wall clock.
+ return ConnectorType.of("TIMESTAMPTZ", 6, 0);
Review Comment:
[P2] Keep MaxCompute TIMESTAMP scans correct on older BEs. This new
TIMESTAMPTZ mapping reaches the old MaxComputeColumnValue.getTimeStampTz, which
converts an Arrow instant into the query timezone; VectorColumn then packs
those local fields as UTC. A 04:00 UTC value in an Asia/Shanghai session is
stored as 12:00 UTC and displayed as 20:00. Gate the new FE slot until scanners
have the added UTC conversion, and cover a non-UTC FE-first scan.
##########
fe/fe-connector/fe-connector-trino/src/main/java/org/apache/doris/connector/trino/TrinoTypeMapping.java:
##########
@@ -74,8 +74,11 @@ public static ConnectorType toConnectorType(Type type) {
return new ConnectorType("CHAR");
} else if (type instanceof VarcharType) {
return new ConnectorType("STRING");
+ } else if (type instanceof io.trino.spi.type.UuidType) {
+ // Preserve the logical type instead of exposing its physical
16-byte storage.
+ return ConnectorType.of("UUID");
Review Comment:
[P2] Keep native Trino UUID scans compatible with older BEs. This mapping
sends a UUID slot to the Trino JNI scanner; on an older BE,
TrinoConnectorColumnValue lacks the new getUuid override, so the interface
default runs UUID.fromString(getString()). Its getString decodes Trino's raw
16-byte UUID block as UTF-8, and an ordinary non-null UUID scan throws. Gate
this newly supported type on scanner capability during rollout; cover an
FE-first scan.
##########
fe/fe-connector/fe-connector-hudi/src/main/java/org/apache/doris/connector/hudi/HudiTypeMapping.java:
##########
@@ -186,9 +190,17 @@ private static ConnectorType mapLongType(LogicalType
logicalType) {
return ConnectorType.of("TIMEV2", 6, 0);
}
if (logicalType instanceof LogicalTypes.TimestampMillis) {
- return ConnectorType.of("DATETIMEV2", 3, 0);
+ // Avro timestamp logical types are instants, not local wall-clock
timestamps.
+ return ConnectorType.of("TIMESTAMPTZ", 3, 0);
}
if (logicalType instanceof LogicalTypes.TimestampMicros) {
+ return ConnectorType.of("TIMESTAMPTZ", 6, 0);
Review Comment:
[P2] Read historical Hudi COW Parquet timestamps with the new instant slot.
Hudi files written with Parquet Java 1.10.1 can have INT64
TIMESTAMP_MILLIS/MICROS in converted_type without the newer LogicalType field.
COW routes those files to the native reader, which still infers DATETIMEV2; V1
has no DATETIMEV2-to-TIMESTAMPTZ conversion for this new FE slot and fails an
ordinary scan with Unsupported type change, even on an upgraded BE. Preserve a
UTC-aware conversion for legacy footers in V1/V2 or keep a compatible slot, and
add a converted-only file fixture.
##########
fe/fe-connector/fe-connector-hudi/src/main/java/org/apache/doris/connector/hudi/HudiTypeMapping.java:
##########
@@ -186,9 +190,17 @@ private static ConnectorType mapLongType(LogicalType
logicalType) {
return ConnectorType.of("TIMEV2", 6, 0);
}
if (logicalType instanceof LogicalTypes.TimestampMillis) {
- return ConnectorType.of("DATETIMEV2", 3, 0);
+ // Avro timestamp logical types are instants, not local wall-clock
timestamps.
+ return ConnectorType.of("TIMESTAMPTZ", 3, 0);
Review Comment:
[P2] Gate Hudi instant slots until older MOR scanners are gone. This new
TIMESTAMPTZ mapping calls HadoopHudiColumnValue.getTimeStampTz on an old BE;
that method casts every value to java.sql.Timestamp. Hudi also supplies
LongWritable and TimestampWritableV2 timestamp carriers, which throw
ClassCastException there on ordinary MOR log scans. The added branches handle
them only on new BEs. Keep the previous carrier during rollout or gate this
mapping; cover both writable carriers in a mixed-version scan.
##########
fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/client/JdbcClickHouseConnectorClient.java:
##########
@@ -199,12 +199,13 @@ private ConnectorType mapClickHouseType(String chType,
JdbcFieldInfo fieldInfo)
// DateTime64
if (chType.startsWith("DateTime64(")) {
+ fieldInfo.setAllowNull(true);
return parseDateTimeType(chType);
}
// DateTime('timezone') — DateTime with timezone parameter, second
precision
if (chType.startsWith("DateTime(")) {
- return ConnectorType.of("DATETIMEV2", 0, -1);
+ return ConnectorType.of("TIMESTAMPTZ", 0, -1);
Review Comment:
[P2] Keep JDBC scans readable while BEs are upgraded. This new FE mapping
sends TIMESTAMPTZ for ClickHouse DateTime, but an older BE JDBC scanner parses
that slot and its ClickHouseTypeHandler has no TIMESTAMPTZ branch, so the first
row throws Unsupported column type. The new PostgreSQL/Trino UUID mappings have
the same old-handler gap. Preserve the previous carriers until all BEs have the
new handlers, or gate these plans on BE capability; cover a new-FE/old-BE scan.
This is separate from the Iceberg file-reader upgrade issue.
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergTypeMapping.java:
##########
@@ -124,28 +124,25 @@ private static ConnectorType
fromPrimitive(Type.PrimitiveType primitive,
case STRING:
return ConnectorType.of("STRING");
case UUID:
- return enableMappingVarbinary
- ? ConnectorType.of("VARBINARY", 16, 0) :
ConnectorType.of("STRING");
+ // Preserve logical UUID semantics independently of the binary
mapping option.
+ return ConnectorType.of("UUID");
Review Comment:
[P2] Keep UUID reads working while BEs are upgraded. A new FE now plans
Iceberg UUID as native UUID, but a pre-upgrade BE V1 Parquet reader maps a
UUID-annotated file to STRING or VARBINARY and has no conversion from either
carrier to a UUID slot. Thus an FE-first upgrade breaks ordinary UUID scans
until every BE is replaced. Preserve the old carrier while older BEs can
receive scans, or gate native UUID plans on BE capability; cover this upgrade
direction. The existing thread covers the reverse old-FE/new-BE direction.
##########
fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcCatalogProperties.java:
##########
@@ -390,11 +390,13 @@ public String getExcludeDatabaseList() {
}
public boolean isEnableMappingVarbinary() {
- return enableMappingVarbinary;
+ // Legacy properties remain parseable, but binary values always retain
their bytes.
+ return true;
}
public boolean isEnableMappingTimestampTz() {
- return enableMappingTimestampTz;
+ // Instant types cannot be downgraded to session-local wall clocks.
+ return true;
Review Comment:
[P2] Preserve JDBC TIMESTAMPTZ writes on older BEs. For a PostgreSQL catalog
that previously disabled zoned mapping, this now forces a TIMESTAMPTZ sink
slot. An old BE writer binds its UTC JNI fields with Timestamp.valueOf, which
interprets them in the BE JVM timezone; with a +08 JVM, 04:00 UTC is sent as
the previous day 20:00 UTC. The new UTC/OffsetDateTime bind is only on upgraded
BEs. Gate this slot on writer capability or retain the compatible carrier until
rollout finishes; cover a non-UTC JVM write.
##########
fe/fe-connector/fe-connector-trino/src/main/java/org/apache/doris/connector/trino/TrinoTypeMapping.java:
##########
@@ -88,7 +91,7 @@ public static ConnectorType toConnectorType(Type type) {
return new ConnectorType("DATETIMEV2", precision, -1);
} else if (type instanceof TimestampWithTimeZoneType) {
int precision = Math.min(((TimestampWithTimeZoneType)
type).getPrecision(), 6);
- return new ConnectorType("DATETIMEV2", precision, -1);
+ return new ConnectorType("TIMESTAMPTZ", precision, -1);
Review Comment:
[P2] Preserve Trino zoned timestamp instants during an FE-first rollout.
This new TIMESTAMPTZ slot reaches an older BE scanner whose getTimeStampTz
returns the source zone's local fields, while the JNI vector treats those
fields as UTC. For example, 12:00 Asia/Shanghai (04:00 UTC) is stored as 12:00
UTC and displays eight hours late. The added UTC conversion exists only on
upgraded BEs. Gate the new slot or keep the old carrier until scanners are
upgraded.
##########
fe/fe-connector/fe-connector-maxcompute/src/main/java/org/apache/doris/connector/maxcompute/MCTypeMapping.java:
##########
@@ -197,6 +199,8 @@ private static TypeInfo toMcScalarType(String name,
ConnectorType type) {
case "DATETIME":
case "DATETIMEV2":
return TypeInfoFactory.DATETIME;
+ case "TIMESTAMPTZ":
Review Comment:
[P2] Keep MaxCompute TIMESTAMP INSERTs working during FE-first upgrades. A
new FE now sends TIMESTAMPTZ for this sink column, but the old
MaxComputeJniWriter handles TIMESTAMP in its DATETIME branch and calls
VectorColumn.getDateTime. That decodes the TIMESTAMPTZ V2 carrier as
DateTimeV1, producing invalid fields and failing before the Arrow write. The
new getTimeStampTz branch exists only on upgraded BEs. Gate this sink slot or
keep its old carrier until writers are upgraded.
##########
fe/fe-connector/fe-connector-hudi/src/main/java/org/apache/doris/connector/hudi/HudiTypeMapping.java:
##########
@@ -59,7 +59,9 @@ public static ConnectorType fromAvroSchema(Schema avroSchema)
{
case DOUBLE:
return ConnectorType.of("DOUBLE");
case STRING:
- return ConnectorType.of("STRING");
+ // Avro stores logical UUIDs as strings, but the connector
must retain UUID semantics.
+ return logicalType instanceof LogicalTypes.Uuid
+ ? ConnectorType.of("UUID") :
ConnectorType.of("STRING");
Review Comment:
[P2] Preserve Hudi COW UUID scans for Parquet Avro string files. Avro
logical uuid is carried as STRING, and the default Parquet Avro writer stores
it as a BINARY STRING leaf; COW base files use Doris's native Parquet reader.
This new UUID slot then asks the V1 reader to convert file STRING to UUID, but
ColumnTypeConverter has no such conversion and returns Unsupported type change
even on an upgraded BE. The new UUID decoder only covers UUID-annotated 16-byte
fixed fields. Keep the compatible string mapping for these files or add
canonical text-to-UUID conversion in native readers, and test a default-written
COW UUID file.
--
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]
