feiniaofeiafei commented on code in PR #69828:
URL: https://github.com/apache/doris/pull/69828#discussion_r4454152852
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/OneRangePartitionEvaluator.java:
##########
@@ -1039,7 +1041,9 @@ private EvaluateRangeResult
computeMonotonicFunctionRange(EvaluateRangeResult re
Literal lower = span.hasLowerBound() ? span.lowerEndpoint().getValue()
: null;
Literal upper = span.hasUpperBound() &&
!(span.upperEndpoint().getValue() instanceof MaxLiteral)
? span.upperEndpoint().getValue() : null;
- if (!func.isMonotonic(lower, upper)) {
+ boolean monotonic = func.isMonotonic(lower, upper);
+ if (!monotonic && !(func instanceof CalendarAddSubMonotonic
Review Comment:
The approach of widening the result range to preserve partition pruning
makes sense. However, the current implementation introduces calendar-specific
handling into the generic computeMonotonicFunctionRange() method: an instanceof
CalendarAddSubMonotonic check allows evaluation to proceed, followed by a cast
and boundary adjustment in the !monotonic branch. This spreads
calendar-specific logic across the generic flow and could lead to more special
cases as other non-monotonic functions are supported.
I suggest introducing a separate capability for inferring a safe result
range, such as inferRange():
- Monotonic functions provide a default implementation that evaluates the
endpoints and orders the bounds according to whether the function is increasing
or decreasing.
- Calendar arithmetic functions provide their own implementation, widening
the bounds when necessary to cover all possible results.
- Functions return Optional.empty() when a safe range cannot be inferred,
allowing the pruner to handle them conservatively.
The partition pruner would then consume the inferred range without knowing
the specific function type. computeMonotonicFunctionRange() could also be
renamed to computeFunctionRange(). Endpoint constant folding and result
validation should be shared to avoid duplication.
Meanwhile, isMonotonic() should retain its independent semantics for
consumers such as runtime filter classification that actually require
monotonicity. Being able to infer a safe result range does not imply that the
function is monotonic.
--
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]
