github-actions[bot] commented on code in PR #66761:
URL: https://github.com/apache/doris/pull/66761#discussion_r3865318759
##########
be/src/exprs/function/function_date_or_datetime_computation.cpp:
##########
@@ -88,6 +88,35 @@ using FunctionDatetimeAddWeeks =
FunctionDateOrDateTimeComputation<AddWeeksImpl<
using FunctionDatetimeAddMonths =
FunctionDateOrDateTimeComputation<AddMonthsImpl<TYPE_DATETIMEV2>>;
using FunctionDatetimeAddYears =
FunctionDateOrDateTimeComputation<AddYearsImpl<TYPE_DATETIMEV2>>;
+#define TIMESTAMP_NS_COMPUTATION_ALIAS(NAME, IMPL) \
+ using FunctionTimeStampNs##NAME =
FunctionDateOrDateTimeComputation<IMPL<TYPE_TIMESTAMP_NS>>
+
+TIMESTAMP_NS_COMPUTATION_ALIAS(AddMicroseconds, AddMicrosecondsImpl);
+using FunctionTimeStampNsAddNanoseconds =
FunctionDateOrDateTimeComputation<AddNanosecondsImpl>;
+TIMESTAMP_NS_COMPUTATION_ALIAS(AddMilliseconds, AddMillisecondsImpl);
+TIMESTAMP_NS_COMPUTATION_ALIAS(AddSeconds, AddSecondsImpl);
+TIMESTAMP_NS_COMPUTATION_ALIAS(AddMinutes, AddMinutesImpl);
+TIMESTAMP_NS_COMPUTATION_ALIAS(AddHours, AddHoursImpl);
+TIMESTAMP_NS_COMPUTATION_ALIAS(AddDays, AddDaysImpl);
+TIMESTAMP_NS_COMPUTATION_ALIAS(AddWeeks, AddWeeksImpl);
+TIMESTAMP_NS_COMPUTATION_ALIAS(AddMonths, AddMonthsImpl);
+TIMESTAMP_NS_COMPUTATION_ALIAS(AddYears, AddYearsImpl);
+TIMESTAMP_NS_COMPUTATION_ALIAS(AddQuarters, AddQuartersImpl);
Review Comment:
[P1] Check interval arithmetic before exposing these aliases
This alias reaches `AddQuartersImpl`, which evaluates `3 * delta` in signed
`Int32`. For example, `quarters_add(cast('1970-01-01 00:00:00.000000000' as
timestamp_ns), 1431655765)` overflows that product; the folded path can turn it
into `-1` and return December 1969 instead of rejecting roughly 4.29 billion
requested months. The other newly exposed aliases have the same endpoint
family: `date_time_add` calls `std::abs(INT*_MIN)`, and generic subtraction
first evaluates `-delta`, both undefined for the signed minimum in C++. Please
compute sign/magnitude and products in a checked wider or unsigned domain and
add folded plus runtime tests for the INT/BIGINT extrema.
##########
be/src/exprs/function/function_date_or_datetime_computation.h:
##########
@@ -1871,7 +2172,16 @@ class AddTimeImplBase {
const auto& arg1 = left_data[index_check_const(i,
cols_info[0].is_const)];
const auto& arg2 = right_data[index_check_const(i,
cols_info[1].is_const)];
- if constexpr (PType == TYPE_DATETIMEV2 || PType ==
TYPE_TIMESTAMPTZ) {
+ if constexpr (PType == TYPE_TIMESTAMP_NS) {
+ auto result = arg1;
+ auto tv2 = static_cast<TimeValue::TimeType>(arg2);
+ TimeInterval interval(TimeUnit::MICROSECOND, tv2, IsNegative);
Review Comment:
[P1] Preserve the sign of TIME operands
`TIMEV2` carries a signed microsecond count, but this constructor takes an
unsigned `uint64_t` magnitude and `IsNegative` reflects only whether the
function is `sub_time`. Consequently a legal call such as `add_time(ts_col,
cast('-00:00:01' as time))` converts the negative value outside the destination
range instead of subtracting one second. FE folding handles the signed value
directly, so the constant form can disagree with the equivalent column form.
Please derive a checked magnitude and combine the operand sign with
`IsNegative`, with folded/runtime tests for negative TIME operands in both
functions.
--
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]
