Gabriel39 commented on code in PR #68301:
URL: https://github.com/apache/doris/pull/68301#discussion_r4060628583
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/util/TypeCoercionUtils.java:
##########
@@ -162,6 +164,10 @@ public class TypeCoercionUtils {
);
private static final Logger LOG =
LogManager.getLogger(TypeCoercionUtils.class);
+ private static final Set<String> UNSUPPORTED_VARBINARY_COLLECTIONS =
ImmutableSet.of(
+ "array_contains", "array_position", "countequal",
"array_distinct", "array_remove",
+ "array_enumerate_uniq", "array_contains_all", "arrays_overlap",
"array_union",
+ "array_except", "array_intersect", "collect_set");
Review Comment:
Already addressed in 44eef01c38d2ee094290cfc1db07535c76ec2274:
function-specific VARBINARY legality checks live in the functions and the
TypeCoercionUtils blacklist was removed. This follow-up retains that
arrangement.
##########
be/src/exec/common/hash_table/hash_key_type.h:
##########
@@ -102,6 +102,13 @@ inline HashKeyType get_hash_key_type_fixed(const
std::vector<DataTypePtr>& data_
}
inline HashKeyType get_hash_key_type(const std::vector<DataTypePtr>&
data_types) {
+ // Reject binary before the multi-key serialization fallback can enable
joins or grouping.
+ for (const auto& type : data_types) {
+ if (type->get_primitive_type() == TYPE_VARBINARY) {
Review Comment:
The rejection is intentional for this change. Preserving previously
reachable serialized hash consumers is a compatibility concern, so the common
VARBINARY hash restriction is retained.
##########
be/src/exec/sink/writer/iceberg/partition_transformers.cpp:
##########
@@ -46,6 +46,12 @@ const std::chrono::sys_days
PartitionColumnTransformUtils::EPOCH = std::chrono::
std::unique_ptr<PartitionColumnTransform> PartitionColumnTransforms::create(
const doris::iceberg::PartitionField& field, const DataTypePtr&
source_type) {
auto& transform = field.transform();
+ // Identity/void only carry values; computed binary partition transforms
are unsupported.
+ if (source_type->get_primitive_type() == TYPE_VARBINARY && transform !=
"identity" &&
Review Comment:
The binary identity extraction/commit path was already incomplete before
this PR. This PR does not add binary partition-writing support; completing that
pipeline or redesigning its rejection boundary is outside this follow-up.
##########
be/src/core/value/timestamptz_value.cpp:
##########
@@ -38,6 +39,13 @@ std::string TimestampTzValue::to_string(const
cctz::time_zone& tz, int scale) co
auto lookup_result = tz.lookup(cur_tz_time);
cctz::civil_second civ = lookup_result.cs;
+ // UTC storage bounds do not guarantee a representable session-local year.
Reject
+ // overflow before DateTimeV2 formatting could produce an offset-only wire
value.
+ if (civ.year() < 0 || civ.year() > 9999) {
Review Comment:
Fixed in 0765475366abe60678176acbbb82ef7d34366bec. Both remaining
TIMESTAMP_NS error paths now format the valid UTC value instead of calling the
session-local formatter again. ASAN tests cover both year endpoints, strict
errors/non-strict NULL results, and both comparison operand orders; all 31
focused BE tests passed.
--
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]
