github-actions[bot] commented on code in PR #67627:
URL: https://github.com/apache/doris/pull/67627#discussion_r4003967064
##########
fe/fe-type/src/main/java/org/apache/doris/catalog/PrimitiveType.java:
##########
@@ -46,6 +46,7 @@ public enum PrimitiveType {
DATETIME("DATETIME", 16, TPrimitiveType.DATETIME, true),
IPV4("IPV4", 4, TPrimitiveType.IPV4, true),
IPV6("IPV6", 16, TPrimitiveType.IPV6, true),
+ UUID("UUID", 16, TPrimitiveType.UUID, true),
Review Comment:
[P1] Fence UUID behind a new BE execution version — UUID adds new Thrift and
PBlock type IDs, but this branch still advertises max `be_exec_version = 14`,
which is also accepted by the immediately preceding TIMESTAMP_NS-capable BE.
During a rolling/smooth upgrade, the FE can therefore schedule a UUID
descriptor or exchange block to an old version-14 BE even though that BE has no
`TPrimitiveType.UUID`, `PGenericType.UUID`, or UUID data-type factory, so the
fragment fails instead of being rejected before scheduling. Please advance the
FE/BE execution version and gate `UUID` in `toThrift()` (with mixed-version
coverage), analogous to the existing TIMESTAMP_NS version-14 fence.
##########
regression-test/suites/datatype_p0/uuid/test_uuid_array_matrix.groovy:
##########
@@ -0,0 +1,95 @@
+// 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.
+
+// All constant/column masks, with FE folding, BE folding and folding disabled.
+suite("test_uuid_array_matrix", "p0") {
Review Comment:
[P2] Keep the UUID golden fixtures below the 1 MiB limit — These two new
golden files exceed the repository's 1 MiB large-file threshold
(`test_uuid_array_matrix.out` is 1,748,503 bytes and
`test_uuid_scalar_matrix.out` is 1,564,821 bytes), which is why the required
large-file check reaches its oversized-file warning path. Please keep the
coverage while reducing the oracle size—for example, split the matrices or
assert compact hashes/counts for the highly repetitive cases—so each added
fixture stays below the limit.
##########
be/src/format_v2/orc/orc_reader.cpp:
##########
@@ -1029,9 +1030,16 @@ DataTypePtr OrcReader::_convert_to_doris_type(const
::orc::Type& type) const {
data_type = std::make_shared<DataTypeFloat64>();
break;
case ::orc::TypeKind::STRING:
- case ::orc::TypeKind::BINARY:
data_type = std::make_shared<DataTypeString>();
break;
+ case ::orc::TypeKind::BINARY:
+ if (type.hasAttributeKey("doris.logical_type") &&
Review Comment:
[P1] Honor enable_mapping_varbinary in the V2 ORC UUID path — Schema
inference and the legacy ORC reader give `enable_mapping_varbinary` precedence
over the new `doris.logical_type=uuid` annotation, so with that option enabled
the FE/table slot is VARBINARY. `TableReader` does not pass the option to the
V2 ORC reader, and this branch instead exposes the same leaf as UUID; the
mapper then creates `CAST(UUID AS VARBINARY)`, which is unsupported because
VARBINARY casts only accept String. Please thread the option into V2 ORC and
apply it before the UUID annotation, and cover `enable_mapping_varbinary=true`
with both scanner generations.
--
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]
