github-actions[bot] commented on code in PR #66333:
URL: https://github.com/apache/doris/pull/66333#discussion_r3715257654
##########
fe/fe-catalog/src/main/java/org/apache/doris/analysis/DateLiteral.java:
##########
@@ -331,7 +351,7 @@ public boolean isMinValue() {
int scale = ((ScalarType) getType()).getScalarScale();
return year == 0 && month == 1 && day == 1
&& hour == 0 && minute == 0 && second == 0
- && microsecond / SCALE_FACTORS[scale] == 0;
+ && getNanosecond() / NANO_SCALE_FACTORS[scale] == 0;
Review Comment:
[P1] Recognize the nanosecond minimum sentinel
For scales 7..9, `createInfinity(..., false)` now reaches `setMinValue()`
and stores the scale-adjusted Int64 boundary near 1677, but this predicate
still recognizes only the old year-0 sentinel. As a result
`PartitionItemToRange` does not translate the first range's lower sentinel to
NULL, and `OneRangePartitionEvaluator` marks that range as unable to contain
NULL, so `IS NULL` pruning can skip the partition that owns null rows. Please
make `isMinValue()` use the same scale-specific boundary as `setMinValue()` and
cover a nullable `DATETIMEV2(9)` first range with an `IS NULL` query.
##########
be/src/exprs/function/function_other_types_to_date.cpp:
##########
@@ -1492,9 +1518,11 @@ void register_function_timestamp(SimpleFunctionFactory&
factory) {
factory.register_function<FromDays>();
factory.register_function<FunctionDateTruncDateV2>();
factory.register_function<FunctionDateTruncDatetimeV2>();
+ factory.register_function<FunctionDateTruncDatetimeV2Nano>();
Review Comment:
[P1] Propagate nano `date_trunc` boundary failures
The nano truncation helper can return false when the truncated civil time no
longer fits the signed epoch-nanosecond range, but the registered `date_trunc`
implementation ignores that result. For example, truncating the accepted
minimum `1677-09-21 00:12:43.145224192` to SECOND (or any coarser unit) fails
conversion and then silently returns the original, untruncated value. Please
propagate the established out-of-range/null result instead of storing `dt`
after a failed truncation, and add lower-bound coverage for both argument
orders.
##########
be/src/storage/olap_common.h:
##########
@@ -167,6 +167,7 @@ enum class FieldType {
OLAP_FIELD_TYPE_IPV4 = 38,
OLAP_FIELD_TYPE_IPV6 = 39,
OLAP_FIELD_TYPE_TIMESTAMPTZ = 40,
+ OLAP_FIELD_TYPE_DATETIMEV2_NANO = 41,
Review Comment:
[P1] Fence persisted nano rowsets from older BEs
This new field type is written as schema name `DATETIMEV2_NANO` and numeric
type 41 in segment footers, but older BEs know neither mapping. During a
rolling upgrade (especially with shared cloud rowsets), an old reader can
therefore be assigned a newly written tablet and fail to open it; the cloud
schema-name normalization does not rewrite segment footers or gate compute
groups. Please add a minimum-reader capability fence before exposing/writing
scales 7..9, or use a backward-readable versioned encoding, and cover a
mixed-version reader.
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/DateLiteralUtils.java:
##########
@@ -247,15 +247,18 @@ public static DateLiteral createDateLiteral(String s,
@Nullable Type type) throw
DateLiteral result;
if (type.isDate() || type.isDateV2()) {
result = new DateLiteral(year, month, day, type);
- } else if (microsecond != 0 && (type.isDatetimeV2() ||
type.isTimeStampTz())) {
- result = new DateLiteral(year, month, day, hour, minute,
second, microsecond, type);
+ } else if (nanosecond != 0 && (type.isDatetimeV2() ||
type.isTimeStampTz())) {
+ int scale = ((ScalarType) type).getScalarScale();
+ long fractionalSecond = type.isDatetimeV2() && scale > 6 ?
nanosecond : microsecond;
Review Comment:
[P1] Normalize explicit-scale catalog literals before storing them
With an explicit `DATETIMEV2(7)`, `.12345675` is parsed as `123456750` ns
and passed through unchanged. It prints as `.1234567`, but FE
equality/hash/order still retain the hidden 50 ns, while Nereids and BE round
the same input to `.1234568`. LIST partition values use this legacy path, so
the catalog boundary can disagree with inserted rows; scale 9 also cannot
consume a tenth guard digit here. Please round with carry to the target scale
before constructing the literal, retain the guard digit, and add scale-7/8/9
list-partition cases.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/SimplifyComparisonPredicate.java:
##########
@@ -290,15 +290,17 @@ private static Expression
processDateTimeLikeComparisonPredicateDateLiteral(
lowBound = new DateTimeLiteral(right.getYear(), right.getMonth(),
right.getDay(), 0, 0, 0);
upBound = new DateTimeLiteral(right.getYear(), right.getMonth(),
right.getDay(), 23, 59, 59);
} else {
- long upMicroSecond = 0;
+ long upperFraction = 0;
for (int i = 0; i < ((DateTimeV2Type) leftType).getScale(); i++) {
- upMicroSecond = 10 * upMicroSecond + 9;
+ upperFraction = 10 * upperFraction + 9;
}
- upMicroSecond *= (int) Math.pow(10, 6 - ((DateTimeV2Type)
leftType).getScale());
+ int scale = ((DateTimeV2Type) leftType).getScale();
+ int fractionalWidth = scale > 6 ? 9 : 6;
+ upperFraction *= (long) Math.pow(10, fractionalWidth - scale);
Review Comment:
[P1] Keep boundary-date rewrites inside the nano range
For `CAST(dt9 AS DATEV2) = DATEV2 '1677-09-21'`, this rewrite constructs
scale-9 midnight, which the new literal validation rejects because the
representable day starts at `00:12:43.145224192`. The upper-bound date has the
symmetric problem: `2262-04-11 23:59:59.999999999` is after the nano maximum.
Both dates contain valid rows, but planning throws while synthesizing the
full-day bounds. Please clamp the generated bounds to the type's representable
min/max (or leave the cast predicate unreduced) and cover comparisons on both
boundary dates.
##########
be/test/core/data_type/data_type_datetime_v2_nano_test.cpp:
##########
@@ -0,0 +1,713 @@
+// 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.
+
+#include <arrow/api.h>
+#include <cctz/time_zone.h>
+#include <gtest/gtest.h>
+
+#include <array>
+#include <cstring>
+#include <limits>
+#include <memory>
+#include <orc/Vector.hh>
+#include <string>
+#include <vector>
+
+#include "core/assert_cast.h"
+#include "core/column/column_const.h"
+#include "core/column/column_nullable.h"
+#include "core/column/column_string.h"
+#include "core/data_type/data_type_date_or_datetime_v2.h"
+#include "core/data_type_serde/data_type_datetimev2_nano_serde.h"
+#include "core/data_type_serde/data_type_serde.h"
+#include "core/string_buffer.hpp"
+#include "core/value/vdatetime_value.h"
+#include "util/jsonb_utils.h"
+#include "util/jsonb_writer.h"
+#include "util/mysql_row_buffer.h"
+#include "util/slice.h"
+#include "util/timezone_utils.h"
+
+namespace doris {
+
+TEST(DataTypeDateTimeV2NanoTest, Int64EpochRangeAndOrdering) {
+ const DateTimeV2NanoValue epoch(0);
+ const DateTimeV2NanoValue before_epoch(-1);
+ const DateTimeV2NanoValue minimum(std::numeric_limits<int64_t>::min());
+ const DateTimeV2NanoValue maximum(std::numeric_limits<int64_t>::max());
+
+ EXPECT_EQ(epoch.to_string(9), "1970-01-01 00:00:00.000000000");
+ EXPECT_EQ(before_epoch.to_string(9), "1969-12-31 23:59:59.999999999");
+ EXPECT_EQ(minimum.to_string(9), "1677-09-21 00:12:43.145224192");
+ EXPECT_EQ(maximum.to_string(9), "2262-04-11 23:47:16.854775807");
+ EXPECT_LT(minimum, before_epoch);
+ EXPECT_LT(before_epoch, epoch);
+ EXPECT_LT(epoch, maximum);
+}
+
+TEST(DataTypeDateTimeV2NanoTest,
NegativeEpochUsesFloorSecondAndNormalizedFraction) {
+ struct TestCase {
+ int64_t epoch_nanos;
+ int64_t epoch_seconds;
+ uint32_t nanosecond;
+ };
+ const std::vector<TestCase> cases = {
+ {-1000000001, -2, 999999999},
+ {-1000000000, -1, 0},
+ {-999999999, -1, 1},
+ {-1, -1, 999999999},
+ {0, 0, 0},
+ {1, 0, 1},
+ };
+
+ for (const auto& test_case : cases) {
+ const DateTimeV2NanoValue value(test_case.epoch_nanos);
+ EXPECT_EQ(value.epoch_seconds(), test_case.epoch_seconds);
+ EXPECT_EQ(value.nanosecond(), test_case.nanosecond);
+ EXPECT_EQ(static_cast<__int128>(value.epoch_seconds()) *
+ DateTimeV2NanoValue::NANOS_PER_SECOND +
+ value.nanosecond(),
+ test_case.epoch_nanos);
+ }
+}
+
+TEST(DataTypeDateTimeV2NanoTest, ParseAndRoundToDeclaredScale) {
+ int64_t value = 0;
+
+ ASSERT_TRUE(parse_datetimev2_nano(StringRef("1970-01-01
00:00:00.12345675"), 7, &value).ok());
+ EXPECT_EQ(DateTimeV2NanoValue(value).to_string(7), "1970-01-01
00:00:00.1234568");
+
+ ASSERT_TRUE(parse_datetimev2_nano(StringRef("1969-12-31
23:59:59.999999999"), 9, &value).ok());
+ EXPECT_EQ(value, -1);
+
+ ASSERT_TRUE(parse_datetimev2_nano(StringRef("1970-01-01
00:00:00.999999995"), 8, &value).ok());
+ EXPECT_EQ(DateTimeV2NanoValue(value).to_string(8), "1970-01-01
00:00:01.00000000");
+}
+
+TEST(DataTypeDateTimeV2NanoTest, ParseTimezoneSuffixInSessionTimezone) {
+ TimezoneUtils::load_timezones_to_cache();
+ cctz::time_zone shanghai;
+ ASSERT_TRUE(cctz::load_time_zone("Asia/Shanghai", &shanghai));
+
+ int64_t value = 0;
+ auto status =
parse_datetimev2_nano(StringRef("2023-08-17T01:41:18.123456789Z"), 9, &value,
+ &shanghai);
+ ASSERT_TRUE(status.ok()) << status.to_string();
+ EXPECT_EQ(DateTimeV2NanoValue(value).to_string(9), "2023-08-17
09:41:18.123456789");
+
+
ASSERT_TRUE(parse_datetimev2_nano(StringRef("2023-08-17T01:41:18.123456789America/Los_Angeles"),
+ 9, &value, &shanghai)
+ .ok());
+ EXPECT_EQ(DateTimeV2NanoValue(value).to_string(9), "2023-08-17
16:41:18.123456789");
+
+
EXPECT_FALSE(parse_datetimev2_nano(StringRef("1677-09-21T00:12:43.145224192+14:00"),
9, &value,
+ &shanghai)
+ .ok());
+
EXPECT_FALSE(parse_datetimev2_nano(StringRef("2262-04-11T23:47:16.854775807-01:00"),
9, &value,
+ &shanghai)
+ .ok());
+}
+
+TEST(DataTypeDateTimeV2NanoTest,
ParseAcceptsAllNanoScalesAndRejectsMalformedValues) {
+ struct ValidCase {
+ int scale;
+ const char* input;
+ const char* expected;
+ };
+ const std::vector<ValidCase> valid_cases = {
+ {7, "2024-02-29 12:34:56.12345674", "2024-02-29 12:34:56.1234567"},
+ {7, "2024-02-29 12:34:56.12345675", "2024-02-29 12:34:56.1234568"},
+ {8, "2024-02-29 12:34:56.123456784", "2024-02-29
12:34:56.12345678"},
+ {8, "2024-02-29 12:34:56.123456785", "2024-02-29
12:34:56.12345679"},
+ {9, "2024-02-29 12:34:56.123456789", "2024-02-29
12:34:56.123456789"},
+ {9, "2024-02-29 12:34:56", "2024-02-29 12:34:56.000000000"},
+ };
+
+ for (const auto& test_case : valid_cases) {
+ int64_t value = 0;
+ ASSERT_TRUE(parse_datetimev2_nano(StringRef(test_case.input),
test_case.scale, &value).ok())
+ << test_case.input;
+ EXPECT_EQ(DateTimeV2NanoValue(value).to_string(test_case.scale),
test_case.expected);
+ }
+
+ const std::vector<const char*> invalid_values = {
+ "", "not-a-date", "2023-02-29
00:00:00.000000000",
+ "2024-13-01", "2024-01-01 24:00:00", "2024-01-01
00:00:00.trailing",
+ };
+ for (const char* input : invalid_values) {
+ int64_t value = 0;
+ EXPECT_FALSE(parse_datetimev2_nano(StringRef(input), 9, &value).ok())
<< input;
+ }
+}
+
+TEST(DataTypeDateTimeV2NanoTest, RejectValuesOutsideEpochRange) {
+ int64_t value = 0;
+ EXPECT_FALSE(parse_datetimev2_nano(StringRef("0000-01-01
00:00:00.000000000"), 9, &value).ok());
+ EXPECT_FALSE(parse_datetimev2_nano(StringRef("1677-09-21
00:12:43.145224191"), 9, &value).ok());
+ EXPECT_FALSE(parse_datetimev2_nano(StringRef("2262-04-11
23:47:16.854775808"), 9, &value).ok());
+ EXPECT_FALSE(parse_datetimev2_nano(StringRef("9999-12-31
23:59:59.999999999"), 9, &value).ok());
+}
+
+TEST(DataTypeDateTimeV2NanoTest, CivilRoundTripPreservesSubMicrosecondDigits) {
+ DateV2Value<DateTimeV2ValueType> civil;
+ civil.unchecked_set_time(2024, 2, 29, 23, 59, 58, 123456);
+
+ DateTimeV2NanoValue value;
+ ASSERT_TRUE(value.from_datetime(civil, 789));
+ EXPECT_EQ(value.to_string(9), "2024-02-29 23:59:58.123456789");
+ EXPECT_EQ(value.year(), 2024);
+ EXPECT_EQ(value.month(), 2);
+ EXPECT_EQ(value.day(), 29);
+ EXPECT_EQ(value.hour(), 23);
+ EXPECT_EQ(value.minute(), 59);
+ EXPECT_EQ(value.second(), 58);
+ EXPECT_EQ(value.microsecond(), 123456);
+ EXPECT_EQ(value.nanosecond_remainder(), 789);
+ EXPECT_EQ(value.to_datetime().to_date_int_val(), civil.to_date_int_val());
+}
+
+TEST(DataTypeDateTimeV2NanoTest,
CalendarArithmeticPreservesSubMicrosecondDigits) {
+ int64_t raw_value = 0;
+ ASSERT_TRUE(
+ parse_datetimev2_nano(StringRef("2024-01-31 23:59:59.123456789"),
9, &raw_value).ok());
+ DateTimeV2NanoValue value(raw_value);
+
+
ASSERT_TRUE(value.date_add_interval<TimeUnit::MONTH>(TimeInterval(TimeUnit::MONTH,
1, false)));
+ EXPECT_EQ(value.to_string(9), "2024-02-29 23:59:59.123456789");
+
+ ASSERT_TRUE(
+
value.date_add_interval<TimeUnit::SECOND>(TimeInterval(TimeUnit::SECOND, 1,
false)));
+ EXPECT_EQ(value.to_string(9), "2024-03-01 00:00:00.123456789");
+}
+
+TEST(DataTypeDateTimeV2NanoTest, DiffTruncatesIncompleteUnitsTowardZero) {
+ int64_t lhs_raw = 0;
+ int64_t rhs_raw = 0;
+ ASSERT_TRUE(
+ parse_datetimev2_nano(StringRef("1970-01-01 00:00:00.000000001"),
9, &lhs_raw).ok());
+ ASSERT_TRUE(
+ parse_datetimev2_nano(StringRef("1970-01-01 00:00:01.999999999"),
9, &rhs_raw).ok());
+ const DateTimeV2NanoValue lhs(lhs_raw);
+ const DateTimeV2NanoValue rhs(rhs_raw);
+
+ EXPECT_EQ(datetime_diff<TimeUnit::SECOND>(lhs, rhs), 1);
+ EXPECT_EQ(datetime_diff<TimeUnit::SECOND>(rhs, lhs), -1);
+ EXPECT_EQ(datetime_diff<TimeUnit::MILLISECOND>(lhs, rhs), 1999);
+ EXPECT_EQ(datetime_diff<TimeUnit::MILLISECOND>(rhs, lhs), -1999);
+ EXPECT_EQ(lhs.datetime_diff_in_microseconds(rhs), -1999999);
+ EXPECT_EQ(rhs.datetime_diff_in_microseconds(lhs), 1999999);
+}
+
+TEST(DataTypeDateTimeV2NanoTest,
FactoryKeepsLegacyAndNanoPhysicalTypesSeparate) {
+ const auto microseconds = create_datetimev2(6);
+ const auto nanoseconds = create_datetimev2(9);
+
+ EXPECT_EQ(microseconds->get_primitive_type(), TYPE_DATETIMEV2);
+ EXPECT_EQ(nanoseconds->get_primitive_type(), TYPE_DATETIMEV2_NANO);
+ EXPECT_EQ(nanoseconds->get_storage_field_type(),
FieldType::OLAP_FIELD_TYPE_DATETIMEV2_NANO);
+ EXPECT_EQ(nanoseconds->get_scale(), 9);
+ EXPECT_EQ(microseconds->get_family_name(), "DateTimeV2");
+ EXPECT_EQ(nanoseconds->get_family_name(), "DateTimeV2Nano");
+}
+
+TEST(DataTypeDateTimeV2NanoTest, SerDeRoundTripsTextProtobufAndBinary) {
+ const auto type = std::make_shared<DataTypeDateTimeV2Nano>(9);
+ const auto serde = type->get_serde();
+ auto source = type->create_column();
+ DataTypeSerDe::FormatOptions options;
+ const std::vector<std::string> inputs = {
+ "1677-09-21 00:12:43.145224192", "1969-12-31 23:59:59.999999999",
+ "1970-01-01 00:00:00.000000000", "2024-02-29 12:34:56.123456789",
+ "2262-04-11 23:47:16.854775807",
+ };
+ for (const auto& input : inputs) {
+ StringRef ref(input);
+ ASSERT_TRUE(serde->from_string(ref, *source, options).ok()) << input;
+ }
+ const auto& source_data = assert_cast<const
ColumnDateTimeV2Nano&>(*source).get_data();
+
+ PValues protobuf_values;
+ ASSERT_TRUE(serde->write_column_to_pb(*source, protobuf_values, 0,
source->size()).ok());
+ auto protobuf_result = type->create_column();
+ ASSERT_TRUE(serde->read_column_from_pb(*protobuf_result,
protobuf_values).ok());
+ const auto& protobuf_data =
+ assert_cast<const
ColumnDateTimeV2Nano&>(*protobuf_result).get_data();
+ EXPECT_EQ(protobuf_data, source_data);
+
+ ColumnString::Chars binary;
+ std::vector<size_t> offsets = {0};
+ for (size_t row = 0; row < source->size(); ++row) {
+ serde->write_one_cell_to_binary(*source, binary, row);
+ offsets.push_back(binary.size());
+ }
+ constexpr size_t bytes_per_row = sizeof(uint8_t) + sizeof(uint8_t) +
sizeof(int64_t);
+ ASSERT_EQ(binary.size(), source->size() * bytes_per_row);
+ auto binary_result = ColumnNullable::create(type->create_column(),
ColumnUInt8::create());
+ for (size_t row = 0; row < source->size(); ++row) {
+ const uint8_t* begin = binary.data() + offsets[row];
+ const uint8_t* end =
DataTypeSerDe::deserialize_binary_to_column(begin, *binary_result);
+ EXPECT_EQ(end - begin, bytes_per_row);
+ }
+ const auto& binary_data =
+ assert_cast<const
ColumnDateTimeV2Nano&>(binary_result->get_nested_column()).get_data();
+ EXPECT_EQ(binary_data, source_data);
+}
+
+TEST(DataTypeDateTimeV2NanoTest, SerDeRoundTripsArrowNanosecondsBeforeEpoch) {
+ const auto type = std::make_shared<DataTypeDateTimeV2Nano>(9);
+ const auto serde = type->get_serde();
+ auto source = type->create_column();
+ auto& source_data = assert_cast<ColumnDateTimeV2Nano&>(*source).get_data();
+ source_data.push_back(DateTimeV2NanoValue(-1));
+ source_data.push_back(DateTimeV2NanoValue(0));
+ source_data.push_back(DateTimeV2NanoValue(1234567890));
+
+ arrow::TimestampBuilder builder(arrow::timestamp(arrow::TimeUnit::NANO),
+ arrow::default_memory_pool());
+ ASSERT_TRUE(serde->write_column_to_arrow(*source, nullptr, &builder, 0,
source->size(),
Review Comment:
[P1] Do not assert success from the current SerDe stubs
`write_column_to_arrow` currently returns `NotSupported` unconditionally, so
this first assertion fails immediately; the later Arrow read and ORC write
assertions have the same contradiction. This file is picked up by the recursive
BE test target, making the current head deterministically red even before
production schema reachability is considered. Please either land the
implementations with these round-trip tests or align the tests with the
deferred `NotSupported` contract until that support lands.
--
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]
