Gabriel39 commented on code in PR #67784:
URL: https://github.com/apache/doris/pull/67784#discussion_r4056549042
##########
be/src/core/field.cpp:
##########
@@ -90,6 +90,34 @@ bool decimal_less_or_equal(Decimal128V3 x, Decimal128V3 y,
UInt32 xs, UInt32 ys)
return dec_less_or_equal<TYPE_DECIMAL128I>(x, y, xs, ys);
}
+namespace {
+// Fields outlive decoder pages and source columns (for example, zone-map
bounds).
+// Keep the view first for Field::get(), and fit ownership into the existing
Field storage.
+struct OwnedBinaryField {
+ StringView view;
+ char* bytes = nullptr;
+
+ explicit OwnedBinaryField(const StringView& value) : view(value) {
+ if (!value.isInline()) {
Review Comment:
Addressed in 19bb5f231e.
Simplified OwnedBinaryField to copy into owned bytes and initialize the view
afterwards. Added coverage for empty, 1-, 12-, 13-, and 64-byte values, source
mutation, copying, moving, and assignment. Removing only the initializer would
lose inline values, so the constructor now handles both representations
uniformly.
##########
be/src/exec/sink/writer/iceberg/partition_transformers.cpp:
##########
@@ -115,10 +118,12 @@ std::unique_ptr<PartitionColumnTransform>
PartitionColumnTransforms::create(
return
std::make_unique<DateBucketPartitionColumnTransform>(source_type,
parsed_width);
}
- case TYPE_DATETIMEV2: {
- return
std::make_unique<TimestampBucketPartitionColumnTransform>(source_type,
-
parsed_width);
- }
+ case TYPE_TIMESTAMPTZ:
+ return
std::make_unique<TimestampBucketPartitionColumnTransform<TYPE_TIMESTAMPTZ>>(
+ source_type, parsed_width);
+ case TYPE_DATETIMEV2:
+ return
std::make_unique<TimestampBucketPartitionColumnTransform<>>(source_type,
Review Comment:
Addressed in 19bb5f231e.
Made DATETIMEV2 explicit at all bucket/year/month/day/hour transform call
sites and in the tests; no timestamp transform relies on a default template
argument.
##########
be/src/exec/sink/writer/iceberg/partition_transformers.h:
##########
@@ -89,13 +94,14 @@ class PartitionColumnTransformUtils {
}
static std::string human_hour(int hour_ordinal) {
- int day_value = hour_ordinal / 24;
- int housr_value = hour_ordinal % 24;
+ // Iceberg ordinals floor toward negative infinity, including the hour
before the epoch.
+ int day_value = hour_ordinal / 24 - (hour_ordinal % 24 < 0);
Review Comment:
Addressed in 19bb5f231e.
The existing timestamp transform test covers epoch -1 microsecond, both
timestamp types, NULL, and the repeated DST hour. Added explicit human_hour
cases for -25, -24, -1, 0, 23, and 24 to cover negative remainders and exact
day boundaries.
##########
be/src/exec/sink/writer/iceberg/partition_transformers.h:
##########
@@ -640,6 +679,7 @@ class DateBucketPartitionColumnTransform : public
PartitionColumnTransform {
DataTypePtr _target_type;
};
+template <PrimitiveType P = TYPE_DATETIMEV2>
Review Comment:
Addressed in 19bb5f231e.
Removed the default primitive type from all five timestamp transform
templates. Production and test call sites now explicitly select TYPE_DATETIMEV2
or TYPE_TIMESTAMPTZ.
--
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]
