github-actions[bot] commented on code in PR #66761:
URL: https://github.com/apache/doris/pull/66761#discussion_r3845618683
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/util/TypeCoercionUtils.java:
##########
@@ -1606,6 +1806,186 @@ public static Optional<DataType>
findWiderTypeForTwoByVariable(DataType left, Da
}
}
+ /** Find a common type for two value-producing expressions using exact
literal conversions. */
+ public static Optional<DataType> findWiderTypeForTwoByVariable(Expression
left, Expression right,
+ boolean overflowToDouble, boolean stringIsHighPriority) {
+ return findWiderCommonTypeForExpressionsByVariable(
+ ImmutableList.of(left, right), overflowToDouble,
stringIsHighPriority, false);
+ }
+
+ /** Find a common type using literal values when TIMESTAMP_NS and another
date-like type mix. */
+ public static Optional<DataType>
findWiderCommonTypeForExpressionsByVariable(
+ List<? extends Expression> expressions, boolean overflowToDouble,
+ boolean stringIsHighPriority, boolean comparison) {
+ Optional<List<DataType>> exactTypes =
replaceExactTimeStampNsAndDateLikeTypes(expressions);
+ if (!exactTypes.isPresent()) {
+ return Optional.empty();
+ }
+ if (GlobalVariable.enableNewTypeCoercionBehavior) {
+ return findWiderCommonType(exactTypes.get(), overflowToDouble,
stringIsHighPriority);
+ }
+ return comparison
+ ? findWiderCommonTypeForComparison(exactTypes.get(), true)
+ : findWiderCommonTypeForCaseWhen(exactTypes.get());
+ }
+
+ /** Find an indexed-ANY common type while retaining the expression for
each nested type. */
+ public static Optional<DataType> findWiderCommonTypeForIndexedAny(
+ List<DataType> dataTypes, List<? extends Expression> expressions) {
+ Preconditions.checkArgument(dataTypes.size() == expressions.size());
+ Optional<List<DataType>> exactTypes =
replaceExactTimeStampNsAndDateLikeTypes(dataTypes, expressions);
+ if (!exactTypes.isPresent()) {
+ return Optional.empty();
+ }
+ return GlobalVariable.enableNewTypeCoercionBehavior
+ ? findWiderCommonType(exactTypes.get(), false, true)
+ : findWiderCommonTypeForComparison(exactTypes.get());
+ }
+
+ /** Whether the two scalar types are the mixed temporal pair with no total
common type. */
+ public static boolean isTimeStampNsAndDateTimeV2Pair(DataType left,
DataType right) {
+ return left instanceof TimeStampNsType && right instanceof
DateTimeV2Type
+ || left instanceof DateTimeV2Type && right instanceof
TimeStampNsType;
+ }
+
+ private static boolean isTimeStampNsAndDateLikePair(DataType left,
DataType right) {
+ return (left instanceof TimeStampNsType && right.isDateLikeType()
+ && !(right instanceof TimeStampNsType))
+ || (right instanceof TimeStampNsType && left.isDateLikeType()
+ && !(left instanceof TimeStampNsType));
+ }
+
+ private static Optional<DataType>
findExactCommonTypeForTimeStampNsAndDateLike(
+ List<? extends Expression> expressions) {
+ Optional<List<DataType>> exactTypes =
replaceExactTimeStampNsAndDateLikeTypes(expressions);
+ if (!exactTypes.isPresent()) {
+ return Optional.empty();
+ }
+ DataType first = exactTypes.get().get(0);
+ return exactTypes.get().stream().allMatch(first::equals) ?
Optional.of(first) : Optional.empty();
+ }
+
+ private static Optional<List<DataType>>
replaceExactTimeStampNsAndDateLikeTypes(
+ List<? extends Expression> expressions) {
+ return replaceExactTimeStampNsAndDateLikeTypes(
+
expressions.stream().map(Expression::getDataType).collect(Collectors.toList()),
expressions);
+ }
+
+ private static Optional<List<DataType>>
replaceExactTimeStampNsAndDateLikeTypes(
+ List<DataType> dataTypes, List<? extends Expression> expressions) {
+ boolean containsTimeStampNs = dataTypes.stream()
Review Comment:
[P2] Apply exact temporal coercion inside nested result types
`replaceExactTimeStampNsAndDateLikeTypes()` only recognizes scalar outer
types. For example, a `CASE` whose branches are `array(cast('2024-01-02
03:04:05.123456789' as timestamp_ns))` and `array(cast('2024-01-02
03:04:05.123456' as datetimev2(6)))` reaches this helper as two `ArrayType`s,
so it skips the value-aware conversion; recursive widening later sees only the
leaf types, finds no total TIMESTAMP_NS/DATETIMEV2 common domain, and rejects
an exactly representable result that the scalar CASE path accepts. This is
independent of indexed-ANY signature resolution. Please carry the corresponding
nested expressions while widening ARRAY/MAP/STRUCT results, with
CASE/IF/COALESCE coverage.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/DateTimeWithPrecision.java:
##########
@@ -46,7 +50,14 @@ public FunctionSignature computeSignature(FunctionSignature
signature) {
// searching in FunctionSet. So we adjust the return type by hand
here.
if (getArgument(0) instanceof IntegerLikeLiteral) {
IntegerLikeLiteral integerLikeLiteral = (IntegerLikeLiteral)
getArgument(0);
- signature =
signature.withReturnType(DateTimeV2Type.of(integerLikeLiteral.getIntValue()));
+ int precision = integerLikeLiteral.getIntValue();
+ if (precision < 0 || precision > TimeStampNsType.SCALE) {
+ throw new AnalysisException("Precision of " +
getName().toUpperCase(Locale.ROOT)
+ + " must be between 0 and "
+ + TimeStampNsType.SCALE + ". Precision was set to:
" + precision);
+ }
+ signature = signature.withReturnType(precision >
DateTimeV2Type.MAX_SCALE
Review Comment:
[P1] Make nonliteral `now` precision obey the nanosecond contract
The TIMESTAMP_NS return type and 0..9 validation run only for literals;
`now(integer_column)` is still accepted and falls through to DATETIMEV2(6). The
generated `N.groovy` query exercises this form, and BE dispatches by that fixed
result carrier: row values 7-9 enter the DATETIMEV2 path, whose `from_unixtime`
clamps the scale to 6, while values above 9 also bypass the new analysis error.
Thus the same precision value preserves nanoseconds as `now(7)` but silently
loses them as `now(kint)`. Please either require a constant scale as
`utc_timestamp` does, or give this form a stable nanosecond carrier plus
runtime range validation, with slot-value tests for 6, 7, 9, and 10.
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/util/StatisticsUtil.java:
##########
@@ -313,6 +314,7 @@ public static double convertToDouble(Type type, String
columnValue) throws Analy
return literal.getDouble();
case DATETIMEV2:
+ case TIMESTAMP_NS:
Review Comment:
[P2] Use one fractional encoding for TIMESTAMP_NS statistics
This arm parses TIMESTAMP_NS min/max as `DateTimeLiteral`, whose
`getDouble()` drops the fractional second, but predicate constants are
`TimeStampNsLiteral`s and add `nanosecond / 1e9`. For a one-value column
containing `2024-02-29 12:34:56.123456789`, ANALYZE stores max
`20240229123456.0`, while the matching literal is represented as
`20240229123456.125`; `estimateColumnEqualToConstant()` therefore treats the
literal as above max and assigns zero selectivity. Please encode collected
bounds with the same TIMESTAMP_NS literal, or conservatively invalidate
fractional ranges, and cover equality and range estimates for distinct
fractions within one second.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/executable/DateTimeAcquire.java:
##########
@@ -40,26 +44,50 @@ public class DateTimeAcquire {
*/
@ExecFunction(name = "now")
public static Expression now() {
- return
DateTimeV2Literal.fromJavaDateType(LocalDateTime.now(DateUtils.getTimeZone()),
0);
+ return DateTimeV2Literal.fromJavaDateType(currentDateTime(), 0);
}
@ExecFunction(name = "now")
public static Expression now(IntegerLiteral precision) {
- return
DateTimeV2Literal.fromJavaDateType(LocalDateTime.now(DateUtils.getTimeZone()),
- precision.getValue());
+ return currentTimestamp(precision.getValue());
}
/**
* date acquire function: current_timestamp
*/
@ExecFunction(name = "current_timestamp")
public static Expression currentTimestamp() {
- return
DateTimeV2Literal.fromJavaDateType(LocalDateTime.now(DateUtils.getTimeZone()),
0);
+ return DateTimeV2Literal.fromJavaDateType(currentDateTime(), 0);
}
@ExecFunction(name = "current_timestamp")
public static Expression currentTimestamp(IntegerLiteral precision) {
- return
DateTimeV2Literal.fromJavaDateType(LocalDateTime.now(DateUtils.getTimeZone()),
precision.getValue());
+ return currentTimestamp(precision.getValue());
+ }
+
+ private static Expression currentTimestamp(int precision) {
+ return currentTimestamp(precision, DateUtils.getTimeZone());
+ }
+
+ private static Expression currentTimestamp(int precision, ZoneId zoneId) {
+ LocalDateTime dateTime = currentDateTime(zoneId);
+ if (precision <= DateTimeV2Type.MAX_SCALE) {
+ return DateTimeV2Literal.fromJavaDateType(dateTime, precision);
+ }
+ int factor = (int) Math.pow(10, DateUtils.NANOSECOND_SCALE -
precision);
+ return TimeStampNsLiteral.fromJavaDateType(
+ dateTime.withNano(dateTime.getNano() / factor * factor));
+ }
+
+ private static LocalDateTime currentDateTime() {
+ return currentDateTime(DateUtils.getTimeZone());
+ }
+
+ private static LocalDateTime currentDateTime(ZoneId zoneId) {
+ ConnectContext connectContext = ConnectContext.get();
+ // Executable functions are also invoked by evaluators without a
session context.
+ Instant currentTime = connectContext == null ? Instant.now() :
connectContext.getStartTimeInstant();
Review Comment:
[P2] Fold every current-time sibling from the statement snapshot
The new `currentDateTime` helper makes
`now`/`current_timestamp`/`utc_timestamp` use
`ConnectContext.getStartTimeInstant()`, but the `current_time`/`curtime`
evaluators still call `LocalDateTime.now()`; `current_date` and no-argument
`unix_timestamp` have the same live-clock pattern. Their BE implementations
read the RuntimeState query globals instead. After a planning delay, FE folding
can therefore make `select now(6), current_time(6)` observe two instants, while
disabling folding makes both statement-stable. Please route all FE
current-date/time/epoch acquisition through this helper and add an
injected-start-instant fold/runtime parity test for the sibling names.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/types/TimeStampNsType.java:
##########
@@ -0,0 +1,99 @@
+// 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.nereids.types;
+
+import org.apache.doris.catalog.ScalarType;
+import org.apache.doris.catalog.Type;
+import org.apache.doris.nereids.trees.expressions.literal.StringLikeLiteral;
+import org.apache.doris.nereids.types.coercion.CharacterType;
+import org.apache.doris.nereids.types.coercion.DateLikeType;
+import org.apache.doris.nereids.types.coercion.ScaleTimeType;
+
+import java.time.DateTimeException;
+import java.time.LocalDateTime;
+import java.time.temporal.ChronoUnit;
+
+/**
+ * Timestamp represented as signed nanoseconds since the Unix epoch.
+ *
+ * <p>TIMESTAMP_NS is date-like and has a fixed scale, but it is not a
DATETIMEV2 precision variant:
+ * the two types have different physical representations and coercion rules.
Keeping it directly under
+ * {@link DateLikeType} prevents generic DATETIMEV2 code from treating its
fixed nanosecond scale as a
+ * DATETIMEV2 scale.</p>
+ */
+public final class TimeStampNsType extends DateLikeType implements
ScaleTimeType {
+ public static final int SCALE = ScalarType.TIMESTAMP_NS_SCALE;
+ public static final TimeStampNsType INSTANCE = new TimeStampNsType();
+
+ private static final int WIDTH = 8;
+
+ private TimeStampNsType() {
+ }
+
+ @Override
+ public boolean isInjectiveCastTo(DataType target) {
+ return target instanceof TimeStampNsType || target instanceof
CharacterType;
+ }
+
+ @Override
+ public Type toCatalogDataType() {
+ return ScalarType.createTimeStampNsType();
+ }
+
+ @Override
+ public ScaleTimeType scaleTypeForType(DataType dataType) {
+ return INSTANCE;
+ }
+
+ @Override
+ public ScaleTimeType forTypeFromString(StringLikeLiteral str) {
+ return INSTANCE;
+ }
+
+ @Override
+ public int getScale() {
+ return SCALE;
+ }
+
+ @Override
+ public int width() {
+ return WIDTH;
+ }
+
+ @Override
+ public double rangeLength(double high, double low) {
+ if (high == low) {
+ return 0;
+ }
+ if (Double.isInfinite(high) || Double.isInfinite(low)) {
+ return Double.POSITIVE_INFINITY;
+ }
+ try {
+ LocalDateTime to = toLocalDateTime(high);
+ LocalDateTime from = toLocalDateTime(low);
+ return ChronoUnit.SECONDS.between(from, to);
Review Comment:
[P2] Preserve subsecond distance in TIMESTAMP_NS range estimates
`toLocalDateTime(double)` casts away the packed fractional remainder, and
`ChronoUnit.SECONDS` then makes every pair within one second have distance
zero. Once TIMESTAMP_NS bounds use their proper fractional encoding, a range
`.1` to `.9` filtered at `.5` sends `0 / 0` into `FilterEstimation`; the NaN
survives its local clamps and `Statistics.withSel()` normalizes it to
selectivity 1.0. Please measure a subsecond distance without overflowing the
full legal domain, or decline range scaling when the representation cannot
distinguish the endpoints, with same-second inequality-estimation tests.
--
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]
