github-actions[bot] commented on code in PR #66761:
URL: https://github.com/apache/doris/pull/66761#discussion_r3831741873
##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVPartitionExprDateTrunc.java:
##########
@@ -247,7 +247,8 @@ private String dateTimeToStr(DateTimeV2Literal literal,
if (partitionColumnType.isDate() || partitionColumnType.isDateV2()) {
return String.format(PartitionExprUtil.DATE_FORMATTER,
literal.getYear(), literal.getMonth(),
literal.getDay());
- } else if (partitionColumnType.isDatetime() ||
partitionColumnType.isDatetimeV2()) {
+ } else if (partitionColumnType.isDatetime() ||
partitionColumnType.isDatetimeV2()
+ || partitionColumnType.isTimeStampNs()) {
Review Comment:
[P1] Preserve valid TIMESTAMP_NS bounds through MTMV roll-up
Admitting TIMESTAMP_NS here still parses its endpoints as
`DateTimeV2Literal` and subtracts one whole second. A valid base range
`[2024-01-01 00:00:00.000000000, 2024-01-01 00:00:00.000000001)` is therefore
assigned an upper representative in the previous hour and rejected. Even after
preserving nanoseconds and using a one-nanosecond predecessor, natural hour
buckets at the signed endpoints generate `1677-09-21 00:00:00` below MIN or
`2262-04-12 00:00:00` above MAX, which downstream partition-key construction
rejects. Please keep TIMESTAMP_NS semantics through this calculation, clamp or
represent endpoint buckets safely, and add MTMV create/refresh coverage for
subsecond bounds and both signed endpoints.
##########
be/src/exprs/function/function_datetime_floor_ceil.cpp:
##########
@@ -739,6 +754,86 @@ struct DateTimeFloorCeilCore {
trivial_part_ts_arg = calc_arg.microsecond();
trivial_part_ts_res = calc_origin.microsecond();
}
+ } else if constexpr (std::is_same_v<DateValueType, TimeStampNsValue>) {
+ const auto nanos_since_midnight = [](const TimeStampNsValue&
value) {
+ return value.time_part_to_seconds() *
TimeStampNsValue::NANOS_PER_SECOND +
+ value.nanosecond();
+ };
+ const auto nanos_since_date = [&](const TimeStampNsValue& value,
uint8_t month,
+ uint8_t day) {
+ return (value.daynr() - calc_daynr(value.year(), month, day))
* HOUR_PER_DAY *
+ SECOND_PER_HOUR *
TimeStampNsValue::NANOS_PER_SECOND +
+ nanos_since_midnight(value);
+ };
+
+ if constexpr (Flag::Unit == YEAR) {
+ diff = ts_arg.year() - ts_origin.year();
+ const auto calendar_remainder = [&](const TimeStampNsValue&
value) {
+ return (static_cast<int64_t>(value.month()) * 32 +
value.day()) * HOUR_PER_DAY *
+ SECOND_PER_HOUR *
TimeStampNsValue::NANOS_PER_SECOND +
+ nanos_since_midnight(value);
+ };
+ trivial_part_ts_arg = calendar_remainder(ts_arg);
+ trivial_part_ts_res = calendar_remainder(ts_origin);
+ }
+ if constexpr (Flag::Unit == QUARTER) {
+ const int64_t total_months = (ts_arg.year() -
ts_origin.year()) * 12 +
+ ts_arg.month() -
ts_origin.month();
+ diff = total_months / 3;
+ const int64_t remaining_months = total_months % 3;
+ if (remaining_months != 0) {
+ trivial_part_ts_arg = remaining_months;
+ trivial_part_ts_res = 0;
+ } else {
+ trivial_part_ts_arg = nanos_since_date(ts_arg,
ts_arg.month(), 1);
+ trivial_part_ts_res = nanos_since_date(ts_origin,
ts_origin.month(), 1);
+ }
+ }
+ if constexpr (Flag::Unit == MONTH) {
+ diff = (ts_arg.year() - ts_origin.year()) * 12 +
+ (ts_arg.month() - ts_origin.month());
+ trivial_part_ts_arg = nanos_since_date(ts_arg, ts_arg.month(),
1);
Review Comment:
[P1] Compare floor/ceil with the clamped boundary
These remainders use the origin's nominal day, but the selected step is
materialized below with calendar clamping. For `month_floor(TIMESTAMP_NS
'2025-02-28 00:00:00.000000000', 1, TIMESTAMP_NS '2025-01-31
00:00:00.000000000')`, this decrements the one-month delta and returns January
31 even though `origin + 1 month` is exactly February 28. Likewise, ceil one
nanosecond after that boundary returns February 28 midnight, earlier than its
input. YEAR/QUARTER and FE folding mirror the same issue. This is distinct from
the earlier origin-relative quarter-coordinate fix: the chosen calendar
boundary itself clamps. Please compare against the actual clamped candidate
boundary and cover exact/just-after Jan-31 and leap-day cases in folded and
runtime paths.
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/PartitionKey.java:
##########
@@ -358,12 +362,19 @@ public PartitionKey successor() throws AnalysisException {
} else if (type == PrimitiveType.DATETIME) {
successorDateTime = successorDateTime.plusSeconds(1);
} else {
- int scale = Math.min(6, Math.max(0, ((ScalarType)
literal.getType()).getScalarScale()));
+ int scale = Math.min(ScalarType.MAX_DATETIMEV2_SCALE,
+ Math.max(0, ((ScalarType)
literal.getType()).getScalarScale()));
long nanoSeconds = BigInteger.TEN.pow(9 -
scale).longValue();
successorDateTime =
successorDateTime.plusNanos(nanoSeconds);
}
successor.pushColumn(new DateLiteral(successorDateTime,
literal.getType()), type);
return successor;
+ case TIMESTAMP_NS:
+ org.apache.doris.analysis.TimeStampNsLiteral timestampNsLiteral
+ = (org.apache.doris.analysis.TimeStampNsLiteral)
literal;
+ successor.pushColumn(new
org.apache.doris.analysis.TimeStampNsLiteral(
Review Comment:
[P2] Handle the TIMESTAMP_NS maximum successor explicitly
Query-cache normalization calls `successor()` for closed `=`/`<=` upper
bounds. At `2262-04-11 23:47:16.854775807`, this creates max + 1ns; the
unchecked literal reaches `PartitionKey.toString()`, where
`getRealValue().longValueExact()` throws on `2^63`. `QueryCacheNormalizer`
catches that and silently disables caching for these otherwise cacheable
queries. Please represent this successor as MAXVALUE/infinity (or special-case
the type maximum) and add maximum-bound normalization coverage.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/util/TypeCoercionUtils.java:
##########
@@ -234,6 +237,27 @@ public static Optional<DataType> implicitCast(DataType
input, DataType expected)
* Return Optional.empty() if we cannot do implicit cast.
*/
public static Optional<DataType> implicitCastPrimitive(DataType input,
DataType expected) {
+ // TIMESTAMP_NS has a different physical representation from
DATETIMEV2. Temporal and
+ // numeric inputs may be promoted to TIMESTAMP_NS now that the
corresponding casts exist,
+ // but never implicitly demote TIMESTAMP_NS to DATETIMEV2 because that
loses nanoseconds.
+ if (input instanceof TimeStampNsType || expected instanceof
TimeStampNsType) {
+ if (input.equals(expected) || expected instanceof AnyDataType) {
+ return Optional.of(input);
+ } else if (input instanceof NullType) {
+ return Optional.of(expected.defaultConcreteType());
+ } else if (expected instanceof TimeStampNsType
+ && (input instanceof NumericType || input.isDateLikeType()
+ || input instanceof TimeV2Type || input instanceof
CharacterType)) {
+ return Optional.of(expected);
+ } else if (input instanceof TimeStampNsType
+ && (expected instanceof FloatType || expected instanceof
DoubleType)) {
Review Comment:
[P1] Keep nanoseconds out of the DOUBLE width_bucket path
This generic implicit branch makes three TIMESTAMP_NS arguments select
`width_bucket`'s DOUBLE signature, but both cast implementations discard the
fractional second. For `width_bucket(TIMESTAMP_NS '1970-01-01
00:00:00.000000005', TIMESTAMP_NS '1970-01-01 00:00:00.000000000', TIMESTAMP_NS
'1970-01-01 00:00:00.000000010', 2)`, all three inputs become the same
packed-second value and the function returns bucket 3 instead of bucket 2.
Other numeric-only signatures inherit the same precision-loss root. Please add
an overflow-safe epoch-nanosecond path or reject this implicit signature, and
cover folded and runtime subsecond ranges.
##########
fe/fe-catalog/src/main/java/org/apache/doris/analysis/TimeStampNsLiteral.java:
##########
@@ -0,0 +1,372 @@
+// 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.
+
+package org.apache.doris.analysis;
+
+import org.apache.doris.catalog.PrimitiveType;
+import org.apache.doris.catalog.ScalarType;
+import org.apache.doris.catalog.Type;
+import org.apache.doris.common.AnalysisException;
+
+import com.google.common.base.Preconditions;
+import com.google.gson.annotations.SerializedName;
+
+import java.math.BigInteger;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.time.DateTimeException;
+import java.time.LocalDateTime;
+import java.time.Year;
+import java.time.ZoneOffset;
+
+/**
+ * Legacy literal for the fixed nanosecond-precision TIMESTAMP_NS type.
+ *
+ * <p>TIMESTAMP_NS owns its value and range logic instead of inheriting the
unrelated calendar
+ * encodings and microsecond rules in {@link DateLiteral}.</p>
+ */
+public final class TimeStampNsLiteral extends LiteralExpr {
+ private static final long NANOSECONDS_PER_SECOND = 1_000_000_000L;
+ private static final long MAX_NANOSECOND = NANOSECONDS_PER_SECOND - 1;
+ private static final int MIN_YEAR = 1677;
+ private static final int MAX_YEAR = 2262;
+ private static final LocalDateTime MIN_VALUE
+ = LocalDateTime.of(MIN_YEAR, 9, 21, 0, 12, 43, 145224192);
+ private static final LocalDateTime MAX_VALUE
+ = LocalDateTime.of(MAX_YEAR, 4, 11, 23, 47, 16, 854775807);
+
+ @SerializedName("y")
+ private long year;
+ @SerializedName("m")
+ private long month;
+ @SerializedName("d")
+ private long day;
+ @SerializedName("h")
+ private long hour;
+ @SerializedName("M")
+ private long minute;
+ @SerializedName("s")
+ private long second;
+ @SerializedName("ns")
+ private long nanosecond;
+
+ public TimeStampNsLiteral() {
+ type = Type.TIMESTAMP_NS;
+ nullable = false;
+ }
+
+ public TimeStampNsLiteral(boolean isMax) {
+ this(isMax ? MAX_VALUE : MIN_VALUE);
+ }
+
+ public TimeStampNsLiteral(long year, long month, long day, long hour, long
minute, long second,
+ long nanosecond) {
+ this();
+ this.year = year;
+ this.month = month;
+ this.day = day;
+ this.hour = hour;
+ this.minute = minute;
+ this.second = second;
+ this.nanosecond = nanosecond;
+ }
+
+ public TimeStampNsLiteral(LocalDateTime value) {
+ this(value.getYear(), value.getMonthValue(), value.getDayOfMonth(),
value.getHour(),
+ value.getMinute(), value.getSecond(), value.getNano());
+ }
+
+ private TimeStampNsLiteral(TimeStampNsLiteral other) {
+ super(other);
+ year = other.year;
+ month = other.month;
+ day = other.day;
+ hour = other.hour;
+ minute = other.minute;
+ second = other.second;
+ nanosecond = other.nanosecond;
+ type = Type.TIMESTAMP_NS;
+ }
+
+ public static TimeStampNsLiteral createMinValue() {
+ return new TimeStampNsLiteral(false);
+ }
+
+ @Override
+ public Expr clone() {
+ return new TimeStampNsLiteral(this);
+ }
+
+ @Override
+ public boolean isMinValue() {
Review Comment:
[P1] Distinguish the legal minimum from the unbounded sentinel
`PartitionKey.isMinValue()` delegates to this method, and query-cache
normalization skips advancing an open lower endpoint whenever it returns true.
Thus `ts > TIMESTAMP_NS '1677-09-21 00:12:43.145224192'` becomes `[MIN, MAX)`,
identical to `ts >= MIN`; both predicates are removed from the normalized
conjuncts and publish the same tablet ranges. If a row exists at MIN, cached
aggregates for the two queries can collide and return the wrong result. Please
track the infinity sentinel separately or advance an open legal MIN by one
nanosecond, with paired query-cache coverage.
--
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]
