github-actions[bot] commented on code in PR #66761:
URL: https://github.com/apache/doris/pull/66761#discussion_r3855691938
##########
be/src/exprs/aggregate/aggregate_function_window_funnel.cpp:
##########
@@ -39,6 +39,9 @@ AggregateFunctionPtr
create_aggregate_function_window_funnel(const std::string&
if (argument_types[2]->get_primitive_type() == TYPE_DATETIMEV2) {
return
creator_without_type::create<AggregateFunctionWindowFunnel<TYPE_DATETIMEV2>>(
argument_types, result_is_nullable, attr);
+ } else if (argument_types[2]->get_primitive_type() == TYPE_TIMESTAMP_NS) {
Review Comment:
[P1] Encode pre-epoch states without a signed left shift. This new
instantiation makes `WindowFunnelState::write()` pass negative epoch
nanoseconds to `write_var_int(Int64)`, whose zig-zag expression computes `x <<
1`; left-shifting a negative signed value is undefined behavior. The added
`PreservesNegativeTimestampDuringSerialization` test feeds `-1` through this
path, and the full BE ASAN profile enables UBSAN (the default ASAN_UT profile
does not), while distributed `window_funnel` can otherwise serialize undefined
state for any pre-1970 value. Perform the zig-zag transform in the unsigned
domain (or use fixed-width signed serialization as v2 does) and cover both
Int64 endpoints.
##########
gensrc/thrift/Types.thrift:
##########
@@ -102,7 +102,8 @@ enum TPrimitiveType {
UINT64 = 41, // only used in BE to represent offsets
FIXED_LENGTH_OBJECT = 42 // only used in BE to represent fixed-length object
VARBINARY = 43, // represent varbinary type
- TIMESTAMPTZ = 44 // timestamp with time zone
+ TIMESTAMPTZ = 44, // timestamp with time zone
+ TIMESTAMP_NS = 45 // signed nanoseconds since the Unix epoch
Review Comment:
[P1] Gate this type on mixed-version clusters. TIMESTAMP_NS is emitted
unconditionally, but both FE and BE still advertise maximum execution version
13. During a rolling upgrade, a new FE can therefore schedule a TIMESTAMP_NS
fragment on an old BE under version 13; that BE preserves enum value 45 and
then hits the `thrift_to_type` default `CHECK(false)`, aborting instead of
rejecting the query. PBlock exchange has the same compatibility gap with
protobuf type 43. Add a new capability/version gate and reject use until every
participating BE supports it, with mixed-version descriptor and block-exchange
tests.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/PredicateRewriteForPartitionPrune.java:
##########
@@ -45,13 +52,30 @@ public static Expression rewrite(Expression expression,
return expression.accept(rewriter, cascadesContext);
}
- /* F: a DateTime or DateTimeV2 column
+ /* F: a DateTime, DateTimeV2, or TimeStampNs column
* Date(F) in (2020-01-02, 2020-01-01) =>
* (2020-01-01 24:00:00 >= F >= 2020-01-01 00:00:00)
* or (2020-01-02 24:00:00 >= F >= 2020-01-02 00:00:00)
*/
@Override
public Expression visitInPredicate(InPredicate in, CascadesContext
context) {
+ if (isDateTimeV2ToTimeStampNsCast(in.getCompareExpr())) {
Review Comment:
[P1] Keep this cast when pruning can hide its failure. DATETIMEV2 covers
years 0000-9999, but its cast to TIMESTAMP_NS fails outside the signed
epoch-nanosecond range when `enable_strict_cast=true`. Rewriting `CAST(dt AS
TIMESTAMP_NS) = ...` to raw `dt = ...` can prune a partition containing
`9999-12-31` before the required cast error is evaluated; for an option that is
not representable at the DATETIMEV2 scale, returning FALSE can prune every
partition the same way. Retain the casted predicate unless the source partition
range is proven representable, and add strict-cast equality/IN tests with an
out-of-range partition.
--
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]
