lets-order-some-fries opened a new pull request, #67037:
URL: https://github.com/apache/doris/pull/67037
### What problem does this PR solve?
Related PR: #28685
Problem Summary:
`from_second` and `from_millisecond` widen their argument to microseconds
**before** the range check runs, so the multiplication can wrap and land back
inside the accepted range.
`fromSecond` computes `second.getValue() * 1000 * 1000` and hands the result
to `fromMicroSecond(long, int)`, whose guard is `microSecond < 0 || microSecond
> 253402271999999999L`. The guard therefore only ever sees the wrapped product:
| Expression | Product | Wraps to | Folds to |
|---|---|---|---|
| `from_second(18446744073710)` | `18446744073710 * 1000000` | `448384` |
`1970-01-01 00:00:00.448384` |
| `from_millisecond(18446744073709552)` | `18446744073709552 * 1000` | `384`
| `1970-01-01 00:00:00.000384` |
Both wrapped values sit inside `[0, 253402271999999999]`, so the guard
passes.
`from_microsecond` passes its argument through unmultiplied and is
unaffected, which is why only the two widening wrappers are wrong.
These functions are registered for FE constant folding, so this is the path
a literal argument takes. The BE divides rather than multiplies
(`from_unixtime(value / Impl::ratio, ...)`) and rejects the same value, so
today the same expression errors over a column but folds to a bogus datetime
over a literal. #28685 fixed the BE side of this family in 2023 and did not
touch the FE fold path.
### What is changed and how does it work?
The widening now goes through a small `toMicroSecond` helper that uses
`Math.multiplyExact` and converts the resulting `ArithmeticException` into the
same `AnalysisException` the range check already raises. An overflowing literal
now reports "out of range" instead of folding.
In-range arguments are unaffected, and negative arguments are still rejected
by the existing guard — both covered by the new tests.
### Release note
Fix `from_second` and `from_millisecond` returning a wrong datetime instead
of an out-of-range error for a literal argument large enough to overflow when
converted to microseconds.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
Added three cases to `DateTimeExtractAndTransformTest`: overflowing
arguments are rejected, the pre-existing out-of-range and negative checks still
reject, and in-range arguments still fold.
```
mvn -f fe/pom.xml -pl :fe-core -am test
-Dtest=DateTimeExtractAndTransformTest
cd fe && mvn clean checkstyle:check -> BUILD SUCCESS
```
I confirmed the new tests fail without the
`DateTimeExtractAndTransform.java` change, so they genuinely cover the defect.
**Note on one pre-existing test failure, unrelated to this PR.**
`testFromUnixTimeOutOfRangeThrows` already fails on a clean checkout of master
at `203923ca` — I verified this by stashing my changes and re-running (7 tests,
1 failure), and it fails under `UTC` as well, so it is not time-zone related.
The cause looks like the value chosen by the test rather than a defect in
`from_unixtime`: `253402272000000000` microseconds is `9999-12-31 16:00:00`
UTC, which is inside the supported range, so `datetime.checkRange()` is
correctly false and nothing is thrown. I have deliberately left it alone since
it is unrelated to this change — happy to send a separate PR correcting the
boundary value if you would like.
- Behavior changed:
- [ ] No.
- [x] Yes. <!-- Explain the behavior change -->
An overflowing literal now raises an out-of-range `AnalysisException`
instead of folding to a wrong datetime. Nothing changes for in-range arguments.
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
--
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]
