OIiveirra commented on code in PR #69577:
URL: https://github.com/apache/doris/pull/69577#discussion_r4367548794
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PushDownFilterThroughProject.java:
##########
@@ -101,7 +101,7 @@ private static Plan pushDownFilterThroughLimitProject(
Map<Slot, Expression> childAlias = project.getAliasToProducer();
// split the conjuncts by child's output
Pair<Set<Expression>, Set<Expression>> splitConjuncts =
- splitConjunctsByChildOutput(filter.getConjuncts(),
childOutputs, childAlias);
+ splitConjunctsByChildOutput(filter.getConjuncts(),
childOutputs, childAlias, limit);
Review Comment:
Fixed in 4a13e99a26a589c870d88aec8cc43c81f0c5cb52.
The Limit/Project compensation now bypasses only the date-arithmetic
protection check: it restores `Project -> Filter -> Limit`, so both the
expanded predicate and the projection evaluate after the enclosing OFFSET and
all lower rejecting operators. Existing volatile-expression and Project-level
guards are retained; ordinary Filter/Project pushdown remains protected.
Reproduced the original overflow before the fix with OFFSET 1 and 100.
Planner coverage now checks offsets 0/1/100, with and without a lower Filter.
Native regression verifies empty results for offsets 1/100 and preserves the
real overflow at offset 0. Final branch validation passed: 17 FE unit tests, 3
targeted native P0 suites and 10 direct SQL checks, with no failures or skips.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/util/ExpressionUtils.java:
##########
@@ -133,6 +147,39 @@ public class ExpressionUtils {
// folding the conjunct). We restrict MAX_MARK_SLOT_COUNT to 4 to bound
this cost.
private static final int MAX_MARK_SLOT_COUNT = 4;
+ /**
+ * Date arithmetic can throw for values outside its valid range. Keep it
above a lower
+ * condition, join, limit, rejecting set operation, generate, or CTE
consumer so rejected
+ * rows are not evaluated early.
+ * A date add/subtract with literal zero is an identity and remains safe
to push through.
+ */
+ public static boolean requiresDateArithmeticProtection(Expression
expression, Plan input) {
+ return expression.anyMatch(expr -> isUnsafeDateArithmetic((Expression)
expr))
+ && input.anyMatch(plan -> plan instanceof Filter || plan
instanceof Join || plan instanceof Limit
+ || plan instanceof LogicalExcept || plan instanceof
PhysicalExcept
+ || plan instanceof LogicalIntersect || plan instanceof
PhysicalIntersect
+ || plan instanceof Generate || plan instanceof
LogicalCTEConsumer
+ || plan instanceof PhysicalCTEConsumer);
+ }
+
+ private static boolean isUnsafeDateArithmetic(Expression expression) {
+ if (expression instanceof DateCeilFloorMonotonic) {
+ return true;
+ }
+ if (!(expression instanceof DateAddSubMonotonic)) {
Review Comment:
Fixed in 4a13e99a26a589c870d88aec8cc43c81f0c5cb52.
The existing classifier now includes `TimestampArithmetic`, treating child 0
as the date operand and applying the same integer-zero exemption to its
interval operand. Logical and physical pushdown share this check; no parser,
backend or global expression-trait changes were added.
Reproduced the EXCEPT overflow before the fix for `dt + INTERVAL 5 WEEK` and
`dt - INTERVAL -5 WEEK`. Added logical/physical planner coverage and native
regression for interval addition/subtraction, both zero-interval spellings, and
participating-row overflow. Both former failures now return February 5; zero
offsets and real-overflow controls retain their expected behavior. Final branch
validation passed: 17 FE unit tests, 3 targeted native P0 suites and 10 direct
SQL checks, with no failures or skips.
--
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]
