Gabriel39 commented on code in PR #68532:
URL: https://github.com/apache/doris/pull/68532#discussion_r4124422741
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergUtils.java:
##########
@@ -750,21 +755,24 @@ 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;
Review Comment:
Fixed in 7de5af07ce4. VARBINARY now uses raw-byte Murmur3 bucket hashing and
a byte-counted truncate transform that retains the binary result type, NULL
map, and constant-column behavior. The string bucket path shares the same
byte-hash implementation. Added tests for empty values, NULs, invalid UTF-8,
partial UTF-8 prefixes, and arena-backed values; expected buckets were checked
against Iceberg Java. Both new test methods reproduced the reported rejection
before the fix; all 34 selected ASAN BE tests and clang-format 16 pass
afterward. Added Parquet/ORC append/overwrite regressions with Spark byte
comparisons and equality pruning. Groovy parsing passed; external integration
execution is pending CI.
##########
be/src/core/data_type_serde/data_type_string_serde.cpp:
##########
@@ -568,7 +568,12 @@ Status
DataTypeStringSerDeBase<ColumnType>::write_column_to_iceberg_arrow(
arrow::ArrayBuilder* array_builder, int64_t start, int64_t end,
const cctz::time_zone& ctz) const {
if (!is_iceberg_uuid_field(field)) {
- // Keep the existing CHAR/STRING fixed-binary binding until external
type mappings change.
+ // Iceberg fixed(N) is a byte binding, so text padding must not change
its physical value.
+ if (array_builder->type()->id() == arrow::Type::FIXED_SIZE_BINARY) {
Review Comment:
Acknowledged as an old-FE/new-BE compatibility issue for Iceberg FIXED
writes. The requested scope explicitly excludes rolling-upgrade and
compatibility fixes, so the legacy CHAR-to-FIXED binding is intentionally not
restored in this PR. The PR description now records this known limitation.
##########
fe/be-java-extensions/hadoop-hudi-scanner/src/main/java/org/apache/doris/hudi/HadoopHudiColumnValue.java:
##########
@@ -131,25 +145,9 @@ public LocalDate getDate() {
public LocalDateTime getDateTime() {
if (fieldData instanceof Timestamp) {
return ((Timestamp) fieldData).toLocalDateTime();
- } else if (fieldData instanceof TimestampWritableV2) {
- return
LocalDateTime.ofInstant(Instant.ofEpochSecond((((TimestampObjectInspector)
fieldInspector)
- .getPrimitiveJavaObject(fieldData)).toEpochSecond()),
zoneId);
- } else {
- long datetime = ((LongWritable) fieldData).get();
- long seconds;
- long nanoseconds;
- if (dorisType.getPrecision() == 3) {
- seconds = datetime / 1000;
- nanoseconds = (datetime % 1000) * 1000000;
- } else if (dorisType.getPrecision() == 6) {
- seconds = datetime / 1000000;
- nanoseconds = (datetime % 1000000) * 1000;
- } else {
- throw new RuntimeException("Hoodie timestamp only support
milliseconds and microseconds, "
- + "wrong precision = " + dorisType.getPrecision());
- }
- return LocalDateTime.ofInstant(Instant.ofEpochSecond(seconds,
nanoseconds), zoneId);
}
+ // DATETIMEV2 now denotes local-timestamp annotations: decode their
fields without a zone shift.
Review Comment:
Acknowledged as an old-FE/new-BE compatibility issue for Hudi DATETIMEV2
timestamp reads. The requested scope explicitly excludes rolling-upgrade and
compatibility fixes, so the legacy session-zone behavior is intentionally not
restored in this PR. The PR description now records this known limitation.
##########
fe/be-java-extensions/jdbc-scanner/src/main/java/org/apache/doris/jdbc/MySQLJdbcExecutor.java:
##########
@@ -70,6 +73,14 @@ protected void abortReadConnection(Connection connection,
ResultSet resultSet)
@Override
protected void initializeStatement(Connection conn, JdbcDataSourceConfig
config, String sql) throws SQLException {
+ if (usesMySqlTimestampProtocol(config)) {
+ // MySQL sends TIMESTAMP as session-local fields. Read and bind
those fields in UTC
+ // without the driver's legacy Calendar conversion, which may
apply the JVM offset.
+ // Set this on every checkout because pooled sessions may have
been modified by a query.
+ try (Statement timezoneStatement = conn.createStatement()) {
Review Comment:
Acknowledged as an old-FE/new-BE compatibility issue for MySQL/OceanBase
legacy DATETIMEV2 reads. The requested scope explicitly excludes
rolling-upgrade and compatibility fixes, so a legacy-plan gate for the UTC
session reset is intentionally not added in this PR. The PR description now
records this known limitation.
--
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]
