jacktengg opened a new pull request, #70556:
URL: https://github.com/apache/doris/pull/70556
### Background
Nereids can move date arithmetic from a comparison's left operand to its
right operand. For example, days_sub(d, 1) < c becomes d < days_add(c, 1). This
can expose a column predicate for pruning.
### What problem does this PR solve?
Issue Number: None
Related PR: None
PR Type: Bugfix
Problem Summary:
The inverse operation can exceed the date type's range even when the
original operation is valid. For example, subtracting one day from an ordinary
date fails when compared with the maximum datetime. Check the inverse with FE
constant evaluation before accepting the rewrite. Keep the original comparison
when the inverse is not a valid date literal.
### Details
- **Change map:** 19 production diff lines, 78 FE test lines, 122 regression
test lines, and 184 generated result lines.
- **Frequency:** Comparisons whose inverse crosses a date boundary,
including a boundary supplied by a column.
- **Root cause:** The rule created an inverse function without checking its
result. Failed FE evaluation left that function for BE execution.
- **Why existing tests missed it:** Existing comparison tests used dates
inside the valid range.
- **Fix:** Coerce and evaluate the inverse on FE. Accept only a date
literal. Reuse that literal in the rewritten comparison.
- **Other paths and branches:** The shared check covers week, day, hour,
minute, and second addition and subtraction. It covers legacy date types,
DATEV2, DATETIMEV2, TIMESTAMPTZ, and TIMESTAMP_NS. SQL aliases use the same
bound functions. Month, quarter, year, smaller units, and compound intervals do
not use this inverse rule. Adjacent date-boundary inference already checks its
generated constants. No release branches were changed.
- **Behavior change:** Valid source arithmetic no longer fails because a
generated inverse exceeds the date range. A valid constant inverse still
permits the rewrite. A boundary supplied by a column retains the original
expression.
### Testing
All checks ran locally.
- Before the fix, the reported SQL failed with E-218. Equivalent DATE,
DATETIME(6), TIMESTAMPTZ, and TIMESTAMP_NS queries also failed.
- `bash run-fe-ut.sh --run
org.apache.doris.nereids.rules.expression.rules.SimplifyArithmeticComparisonRuleTest`:
4 tests passed.
- `DISABLE_BUILD_UI=ON FE_MAVEN_THREADS=4 bash build.sh --fe --output
/mnt/disk2/tengjianping/doris-master2/output/date-boundary-build`: passed,
including Checkstyle with zero violations.
- Generated the new result file with `run-regression-test.sh --run -d
nereids_rules_p0/expression -s test_date_comparison_boundary -forceGenOut`.
- Checked all 61 generated result blocks against their intended values.
- `run-regression-test.sh --run -d nereids_rules_p0 -s
test_date_comparison_boundary,test_simplify_arithmetic,test_simplify_comparison_predicate,test_date_arithmetic_rejecting_boundary,test_calendar_arithmetic_prune`:
all 5 suites passed.
- The reported SQL returned its original row after the FE restart. EXPLAIN
retained days_sub on the column.
### Notes for Reviewers
Self-review checkpoints:
- Goal and evidence: The original error is reproduced before the fix and
removed after it.
- Scope and reuse: One shared rule changes. Existing coercion and date
evaluation supply the range checks.
- Concurrency and locks: No shared mutable state, threads, or locks are
added.
- Lifecycle and initialization: No lifecycle or static initialization
behavior changes.
- Configuration: No configuration item is added.
- Compatibility: Function signatures and stored formats are unchanged.
- Thrift and Protobuf: No definitions change.
- Parallel paths: All ten date functions in the rearrangement map use the
check.
- Conditional checks: Comments explain the maximum-date failure and the
unknown column boundary.
- Test coverage: Tests cover both limits, both interval signs, six
comparison operators, NULL, aliases, nested operations, and BE folding.
- Test results: The regression suite uses one stored row. Its results are
generated by the standard script and verified on a second run.
- Observability: EXPLAIN shows the retained predicate. No new log or metric
is needed.
- Transactions and persistence: No transaction or journal behavior changes.
- Data writes and atomicity: Production write paths are unchanged.
- FE/BE variables: No new value crosses the FE/BE interface.
- Performance: The check runs locally during planning and reuses its folded
result. Unknown boundaries retain per-row date evaluation.
- Other findings: No additional issue was found in the changed paths.
### Release note
Fix date comparison errors caused by optimizer-generated arithmetic that
exceeds the date type's range.
### Check List (For Author)
- Behavior changed: Yes (see Details)
- Does this need documentation: No
- Pre-submission status: Ready
### AI Disclosure
- Tool: Codex (GPT-6)
- Generated: Code, tests, comments, and this commit description.
- Human review: Pending.
### Background
<!-- How the feature or module this PR touches works today, in two to four
plain sentences, for a reviewer who doesn't know it. Link the user doc if there
is one. Leave the problem and the fix to Problem Summary. -->
### What problem does this PR solve?
<!-- Issue Number: "close #N" if this PR resolves the issue, "#N" if it only
relates to it, "None" if there is none. A Feature needs an issue. -->
<!-- Related PR: "#N (relation)" for each PR a reviewer should know about,
such as "#123 (introduced the bug)", separated by commas. Write "None" if no
related PR is known. -->
<!-- PR Type: Bugfix, Performance, Feature, Enhancement, Refactor, Test,
Build/CI/Chore, Revert, Release/Dependency, or Other. -->
Issue Number:
Related PR:
PR Type:
Problem Summary:
<!-- Two to five sentences: the problem or need and who is affected; the
approach in one sentence; when the diff is larger than that suggests, what else
it had to handle, or that most of it is tests; and what existing users see
after an upgrade. -->
### Details
<!--
For a diff of more than about 300 changed lines, start with "- **Change
map:**": how many lines are production code, tests, generated files, and docs,
then the code in parts, each with the problem it handles and its size.
Then answer each field of your PR type on its own line, as "- **Field:**
answer", in this order:
Bugfix: Frequency, Root cause, Why existing tests missed it, Fix, Other
paths and branches
Performance: Why it was slow, Optimization, Results, Applicability and cost
Feature: Scope, Design, Usage, Default behavior, Compatibility
Enhancement: Before and after, Configs and defaults, Impact on existing
users
Refactor: Moves vs. logic changes, Behavior change, Compatibility,
Performance impact, Rollback
Test, adding tests: Scenarios, What existing tests miss, Fails when broken
Test, fixing a test: Failure, Cause, Stability evidence
Build/CI/Chore: Affected, Developer and release impact, License and
third-party, Rollback
Revert: Why not fix forward, Original author notified, Follow-up
Release/Dependency: Versions, Upstream changes, Compatibility, Rollback
Other: Why Other
"None" is a valid answer when it is true. Don't restate the diff.
-->
### Testing
<!-- The tests, suites, or commands that ran, what each shows, and where it
ran (locally or in CI). For a fix, the test that fails before it and passes
after. -->
### Notes for Reviewers
<!-- Optional; delete this section if there is nothing to say. Why each
non-obvious edit was made, what you are unsure about, and where to start
reading if Details has no Change map. -->
### Release note
<!-- One paragraph on what users will notice, or None. -->
### Check List (For Author)
<!-- Behavior changed: "No", or "Yes (see Details)" with a Behavior change
field in Details. -->
<!-- Does this need documentation: "No", or "Yes" with the
apache/doris-website PR or the plan. -->
<!-- Pre-submission status: "Ready"; "Partially verified: " and what is not
verified; or "Not ready: " and why, for a Draft PR. -->
- Behavior changed:
- Does this need documentation:
- Pre-submission status:
### AI Disclosure
<!-- Tool: the agent and model, or None. Generated: the parts it wrote
(code, tests, this description), or None. Human review: what you reviewed of
the generated parts, or "Not applicable" without AI tools. You may add "-
Prompts:" to summarize how you directed the agent. -->
- Tool:
- Generated:
- Human review:
--
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]
