fix temporal part 2#25899
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
Merge Queue Status
This pull request spent 13 minutes 1 second in the queue, with no time running CI. ReasonThe pull request can't be updated
HintYou should update or rebase your pull request manually. If you do, this pull request will automatically be requeued once the queue conditions match again. Requeued — the merge queue status continues in this comment ↓. |
gouhongshen
left a comment
There was a problem hiding this comment.
Codex automated review
Zero temporal values are still misprocessed by two calendar-arithmetic paths introduced by the new sentinel representation.
P1 - Return NULL for zero DATETIME in CONVERT_TZ (pkg/sql/plan/function/func_binary.go:1254)
ZeroDatetime is now -1, but this path converts it to a Go time and clamps it to 0001-01-01 00:00:00, then appends that non-NULL string. Consequently, convert_tz(cast('0000-00-00 00:00:00' as datetime), '+00:00', '+01:00') returns a fabricated minimum date instead of NULL. CONVERT_TZ requires a complete date (as does the PR's zero-temporal policy); add a zero-temporal check before conversion and cover it in the BVT.
P1 - Do not perform ADDTIME/SUBTIME/TIMEDIFF arithmetic on the zero sentinel (pkg/sql/plan/function/func_binary.go:3221)
A zero DATETIME now reaches this existing arithmetic unguarded. For example, ADDTIME(cast('0000-00-00 00:00:00' as datetime), '00:00:01') computes -1 + 1000000 and returns 0001-01-01 00:00:00.999999 rather than NULL. The corresponding SUBTIME paths and both TIMEDIFF paths likewise subtract the sentinel and produce a fabricated or saturated non-NULL result. MySQL treats zero-date inputs to these date-arithmetic operations as NULL; guard ZeroDatetime/ZeroTimestamp before the arithmetic and add typed and string-input coverage.
gouhongshen
left a comment
There was a problem hiding this comment.
Codex automated review
The prior binary-protocol Date(0), DATE_FORMAT (including %x), ISCP drop_at, CONVERT_TZ, and ADDTIME/SUBTIME/TIMEDIFF blockers are fixed; none was merely withdrawn, risk-accepted, or remains blocking. Two newly exposed zero-temporal truncation paths still require correction.
P1 - Guard zero temporals before DATE_TRUNC calendar reconstruction (pkg/sql/plan/function/func_binary.go:12550)
Previously, zero temporal strings were rejected, so DATE_TRUNC could not receive a distinct zero value. In permissive mode this PR now produces ZeroDatetime(-1), but this call passes it directly to dateTruncCore. For month/day/sub-day units, the zero accessors yield month 0 and line 12754 calls DatetimeFromClock; DateFromCalendar then indexes daysBefore[month-1], where uint8 underflow produces index 255 and an index-out-of-range panic. DATE and calendar-unit TIMESTAMP overloads reach the same core. Repro: SET sql_mode=''; SELECT date_trunc('month', CAST('0000-00-00 00:00:00' AS DATETIME));. Handle the sentinel consistently before reconstruction—complete-calendar functions elsewhere return NULL—and add coverage for every temporal overload.
P2 - Do not alias zero time-window keys to the valid minimum datetime (pkg/sql/plan/function/func_binary.go:1721)
This PR makes ZeroDatetime equal -1 while preserving DatetimeEpoch(0) as the valid 0001-01-01 minimum. For every positive bucket width t, the current arithmetic computes -1 - (-1 % t) == 0, silently converting a stored zero datetime into that valid minimum. QueryBuilder injects this function for INTERVAL(ts, ...), and permissive mode permits zero DATETIME/TIMESTAMP primary keys, so a time-window query reports incorrect _wstart/_wend values and violates the PR's explicit zero-versus-minimum distinction. Enforce an explicit zero-key policy before modulo arithmetic and cover zero-valued time-window keys.
|
|
gouhongshen
left a comment
There was a problem hiding this comment.
Codex automated review
All previously blocking topics are fixed: binary-protocol zero dates, DATE_FORMAT sentinels, ISCP zero timestamps, CONVERT_TZ, zero-time arithmetic, DATE_TRUNC panics, and time-window sentinel aliasing. None were withdrawn or accepted as non-blocking, and none remain blocking. One new zero-date correctness regression remains.
P1 - Guard zero dates before extracting the week (pkg/sql/plan/function/func_binary.go:6265)
This PR makes 0000-00-00 a reachable Date(-1), but EXTRACT(WEEK ...) passes it directly to WeekOfYear2(). Its negative-date calendar arithmetic wraps the day-of-year and returns 146, outside MySQL's valid week range. The datetime path repeats this at line 6579. Thus SET sql_mode=''; SELECT EXTRACT(WEEK FROM CAST('0000-00-00' AS DATE)); produces a concrete invalid result; before this PR the cast was rejected. Handle ZeroDate/ZeroDatetime before WeekOfYear2() and return the MySQL-compatible zero result, with DATE, DATETIME, TIMESTAMP, and string-input regression tests.
• 针对 reviewer comment,完成了以下修改:
|
gouhongshen
left a comment
There was a problem hiding this comment.
Codex automated review
The prior blockers are fixed, including the latest EXTRACT(WEEK) issue; none was withdrawn or accepted as non-blocking. The new guards handle zero DATE, DATETIME, TIMESTAMP, and string inputs while preserving valid 0001-01-01 behavior. The merge adds no conflict-resolution delta.
Merge Queue Status
This pull request spent 2 hours 45 minutes 9 seconds in the queue, with no time running CI. Waiting for
All conditions
ReasonThe merge conditions cannot be satisfied due to failing checks Failing checks: HintYou may have to fix your CI before adding the pull request to the queue again. Tick the box to put this pull request back in the merge queue (same as
|
What type of PR is this?
Which issue(s) this PR fixes:
issue #25228
#25229
#25263
What this PR does / why we need it:
相对 main 的修改主要是 MySQL temporal 兼容:
此次合并 main 已完成,生成的 pipeline.pb.go 同时保留了 temporal 的 reject_zero_temporal 字段和 main 的 Iceberg protobuf 定义。