Gabriel39 commented on code in PR #67784:
URL: https://github.com/apache/doris/pull/67784#discussion_r4023606041
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcPostgreSQLClient.java:
##########
@@ -151,8 +151,8 @@ protected Type jdbcTypeToDoris(JdbcFieldSchema fieldSchema)
{
if (scale > 6) {
scale = 6;
}
- return enableMappingTimestampTz ?
ScalarType.createTimeStampTzType(scale)
- : ScalarType.createDatetimeV2Type(scale);
+ // Never discard the instant semantics declared by PostgreSQL
timestamptz.
+ return ScalarType.createTimeStampTzType(scale);
Review Comment:
Addressed in the current head (`68df1fdb0f`). Added recursive TIMESTAMPTZ
array conversion from pgjdbc Timestamp to UTC LocalDateTime, preserving NULLs.
Verified with pgjdbc 42.7.13/PostgreSQL 16 and through a real Doris JNI scan of
flat and multidimensional arrays across UTC, Asia/Shanghai and
America/New_York, including both DST-overlap instants.
##########
fe/be-java-extensions/jdbc-scanner/src/main/java/org/apache/doris/jdbc/SQLServerJdbcExecutor.java:
##########
@@ -68,6 +73,21 @@ protected void initializeBlock(int columnCount, String[]
replaceStringList, int
@Override
protected Object getColumnValue(int columnIndex, ColumnType type, String[]
replaceStringList) throws SQLException {
switch (type.getType()) {
+ case TIMESTAMPTZ: {
+ // JNI carries instants as UTC components, not the source
zone's wall clock.
+ if (!useLegacyTimestampRead) {
+ try {
+ OffsetDateTime value = resultSet.getObject(columnIndex
+ 1, OffsetDateTime.class);
+ return value == null ? null :
LocalDateTime.ofInstant(value.toInstant(), ZoneOffset.UTC);
+ } catch (SQLFeatureNotSupportedException e) {
Review Comment:
Addressed in the current head (`68df1fdb0f`). Removed exception-based
typed-getter probing. getTimestamp resolves datetimeoffset's explicit offset on
both legacy and current drivers, while genuine SQLExceptions still propagate.
Also fixed a related write-side JVM-timezone shift by binding an explicit UTC
ISO timestamp. Real read/write tests pass on SQL Server 2022 with JDBC 6.2.1,
6.4.0, 7.0.0 and 13.4.0.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergUtils.java:
##########
@@ -716,21 +718,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;
+ // 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);
case FIXED:
Types.FixedType fixed = (Types.FixedType) primitive;
- return enableMappingVarbinary ?
ScalarType.createVarbinaryType(fixed.length())
- : ScalarType.createCharType(fixed.length());
+ // Iceberg fixed(N) is an arbitrary N-byte value, not text, so
retain both its
+ // binary semantics and declared width.
+ return ScalarType.createVarbinaryType(fixed.length());
case DECIMAL:
Types.DecimalType decimal = (Types.DecimalType) primitive;
return ScalarType.createDecimalV3Type(decimal.precision(),
decimal.scale());
case DATE:
return ScalarType.createDateV2Type();
case TIMESTAMP:
- if (enableMappingTimestampTz && ((TimestampType)
primitive).shouldAdjustToUTC()) {
+ // Preserve the logical distinction between instants and
wall-clock timestamps.
+ if (((TimestampType) primitive).shouldAdjustToUTC()) {
Review Comment:
Addressed in the current head (`68df1fdb0f`). Added typed UTC TIMESTAMPTZ
bucket/year/month/day/hour transforms and identity extraction/serialization,
including static partition normalization. Also fixed fractional-microsecond
hashing, pre-epoch calendar ordinals, and offset-preserving FE partition
metadata serialization. Unit tests cover DST folds, NULLs and negative epochs;
the partitioned Parquet/ORC write/read/overwrite/delete regression is included.
##########
be/src/storage/olap_common.h:
##########
@@ -170,6 +170,8 @@ enum class FieldType {
OLAP_FIELD_TYPE_IPV4 = 38,
OLAP_FIELD_TYPE_IPV6 = 39,
OLAP_FIELD_TYPE_TIMESTAMPTZ = 40,
+ // A distinct persisted type keeps binary payloads out of character
conversion paths.
+ OLAP_FIELD_TYPE_VARBINARY = 41,
Review Comment:
Addressed in the current head (`68df1fdb0f`). Addressed by the
native-storage rollback in ba8f0cb673. OLAP_FIELD_TYPE_VARBINARY and its
persisted storage ID were removed entirely, so this PR cannot write conflicting
VARBINARY segment metadata. The execution-layer mapping is retained, and native
VARBINARY DDL/CTAS/MTMV rejection is covered by tests.
##########
be/src/exec/sink/vtablet_block_convertor.cpp:
##########
@@ -330,6 +332,26 @@ Status
OlapTableBlockConvertor::_internal_validate_column(RuntimeState* state, B
};
switch (type->get_primitive_type()) {
+ case TYPE_VARBINARY: {
+ const auto* binary_type =
+ assert_cast<const
DataTypeVarbinary*>(remove_nullable(type).get());
+ const auto* binary = assert_cast<const
ColumnVarbinary*>(real_column_ptr.get());
+ int limit = config::string_type_length_soft_limit_bytes;
+ if (binary_type->len() >= 0) {
+ limit = std::min(limit, binary_type->len());
+ }
+ // VARBINARY(n) limits bytes, including nested leaves; text truncation
would corrupt keys.
+ for (size_t j = 0; j < row_count; ++j) {
+ const auto row = rows ? (*rows)[j] : j;
+ if (need_to_validate(j, row, _filter_map, null_map) &&
Review Comment:
Addressed in the current head (`68df1fdb0f`). The native VARBINARY
validation path that introduced this access was removed in ba8f0cb673, in
accordance with the execution-only scope. vtablet_block_convertor.cpp now has
no PR diff and no bounded VARBINARY loop. Native nested VARBINARY schemas
remain rejected; this PR does not modify native load validation.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/CreateTableInfo.java:
##########
@@ -561,7 +558,8 @@ public void validate(ConnectContext ctx) {
break;
}
keys.add(column.getName());
- if (type.isVarcharType()) {
+ // Variable-length binary keys, like VARCHAR,
terminate the short-key prefix.
+ if (type.isVarcharType() ||
type.isVarBinaryType()) {
Review Comment:
Addressed in the current head (`68df1fdb0f`). Addressed by removing native
VARBINARY support rather than introducing a new persisted key contract. Native
VARBINARY keys and materialization remain rejected, and this PR no longer
modifies the native short-key encoding/selection paths.
##########
be/src/storage/key_coder.h:
##########
@@ -356,6 +356,10 @@ class KeyCoderTraits<FieldType::OLAP_FIELD_TYPE_STRING> {
}
};
+template <>
+class KeyCoderTraits<FieldType::OLAP_FIELD_TYPE_VARBINARY>
Review Comment:
Addressed in the current head (`68df1fdb0f`). Addressed by the
native-storage rollback in ba8f0cb673: there is no VARBINARY OLAP field type or
native VARBINARY full-key encoder in this PR. Native VARBINARY table/key
definitions are rejected, so no new non-memcomparable primary index can be
written.
--
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]
