Skip to content

fix temporal part 2#25899

Merged
XuPeng-SH merged 53 commits into
matrixorigin:mainfrom
daviszhen:0716-fix-temporal2
Jul 24, 2026
Merged

fix temporal part 2#25899
XuPeng-SH merged 53 commits into
matrixorigin:mainfrom
daviszhen:0716-fix-temporal2

Conversation

@daviszhen

@daviszhen daviszhen commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

What type of PR is this?

  • API-change
  • BUG
  • Improvement
  • Documentation
  • Feature
  • Test and CI
  • Code Refactoring

Which issue(s) this PR fixes:

issue #25228

#25229

#25263

What this PR does / why we need it:

相对 main 的修改主要是 MySQL temporal 兼容:

  • 支持 DATE、DATETIME、TIMESTAMP 的零值 sentinel,并区分它与真实最小日期 0001-01-01。
  • 完善严格模式与 NO_ZERO_DATE 校验,覆盖普通 DML、multi_update、preinsert、外表/LOAD DATA 等写入路径。
  • 修复 temporal 与数值的转换:DATE/DATETIME/TIMESTAMP 使用 MySQL packed numeric 表示,补齐 DECIMAL -> temporal 等回写转换。
  • 修正 temporal 比较与表达式推导,包括 DATE 与整数比较、TIME 与紧凑字符串比较,以及常量折叠和 Binder 路径。
  • 零日期参与日期函数时按 MySQL 语义返回 NULL 或字段零值;DATE_FORMAT 不再伪造星期/月或在月份为零时报错。
  • 修复 MySQL binary protocol:真实 0001-01-01 不再被编码为 0000-00-00。
  • ISCP 将业务零 TIMESTAMP 归一化,避免误作 drop_at 状态 sentinel。
  • SQL mode 在远程执行、编译和序列化路径中传递,避免节点间语义不一致。
  • 补充 container、cast、函数、DML、external、process 等单测,以及 temporal 兼容 BVT 场景。

此次合并 main 已完成,生成的 pipeline.pb.go 同时保留了 temporal 的 reject_zero_temporal 字段和 main 的 Iceberg protobuf 定义。

@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@mergify

mergify Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Merge Queue Status

  • Entered queue2026-07-23 10:34 UTC · Rule: main · triggered by rule Automatic queue on approval for main
  • 🚫 Left the queue2026-07-23 10:47 UTC · at f4fb405c189d52baf1486e53f12a19528734d705

This pull request spent 13 minutes 1 second in the queue, with no time running CI.

Reason

The pull request can't be updated

merge conflict between base and head

Hint

You should update or rebase your pull request manually. If you do, this pull request will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue, you can requeue the pull request, without updating it, by posting a @mergifyio queue comment.

Requeued — the merge queue status continues in this comment ↓.

@gouhongshen gouhongshen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 gouhongshen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@daviszhen

daviszhen commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

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.

  • 修复 binary protocol:区分 zero temporal sentinel 与合法最小日期 0001-01-01。
  • 完善 DATE_FORMAT:zero temporal 的名称、星期格式返回 NULL,并修正 %x 为 0001。
  • 修复 ISCP:将 zero drop_at 归一化,避免破坏 active/drop 状态语义。
  • 修复时间运算:CONVERT_TZ、ADDTIME、SUBTIME、TIMEDIFF 遇到 zero temporal 时返回 NULL,不再产生伪造日期。
  • 修复 DATE_TRUNC:在日历重建前拦截 DATE、DATETIME、TIMESTAMP 零值,避免 panic。
  • 修复 TimeWin:zero key 使用独立状态,不再与合法的 0001-01-01 混淆,并补全 Reset/复用路径。
  • 补充对应 UT、race 测试及 BVT/result,覆盖 zero 与合法最小值、批量中间零值、typed/string 输入和窗口边界。

@daviszhen

Copy link
Copy Markdown
Contributor Author

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.

  • 修复 binary protocol:区分 zero temporal sentinel 与合法最小日期 0001-01-01。
  • 完善 DATE_FORMAT:zero temporal 的名称、星期格式返回 NULL,并修正 %x 为 0001。
  • 修复 ISCP:将 zero drop_at 归一化,避免破坏 active/drop 状态语义。
  • 修复时间运算:CONVERT_TZ、ADDTIME、SUBTIME、TIMEDIFF 遇到 zero temporal 时返回 NULL,不再产生伪造日期。
  • 修复 DATE_TRUNC:在日历重建前拦截 DATE、DATETIME、TIMESTAMP 零值,避免 panic。
  • 修复 TimeWin:zero key 使用独立状态,不再与合法的 0001-01-01 混淆,并补全 Reset/复用路径。
  • 补充对应 UT、race 测试及 BVT/result,覆盖 zero 与合法最小值、批量中间零值、typed/string 输入和窗口边界。

@gouhongshen gouhongshen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@daviszhen

Copy link
Copy Markdown
Contributor Author

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,完成了以下修改:

  • 在 EXTRACT(WEEK) 的 DATE 路径中拦截 ZeroDate,直接返回 0,避免 WeekOfYear2() 溢出得到 146。
  • DATETIME 路径拦截 ZeroDatetime;TIMESTAMP 和字符串输入复用该保护。
  • 补充 DATE、DATETIME、TIMESTAMP、string 四种输入的 UT。
  • 补充 BVT,覆盖 zero date/datetime、两种 zero string,以及合法最小值 0001-01-01 对照。
  • 更新对应 .result,确认 zero 返回 0/00,合法最小值仍返回正常周数。

@gouhongshen gouhongshen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@mergify

mergify Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Merge Queue Status

  • Entered queue2026-07-24 09:30 UTC · Rule: main · triggered by rule Automatic queue on approval for main
  • Checks failed · in-place
  • 🚫 Left the queue2026-07-24 12:15 UTC · at 828c7c87449aa52e1de4b51c75d866044a44a0ce

This pull request spent 2 hours 45 minutes 9 seconds in the queue, with no time running CI.

Waiting for
  • any of: [🛡 GitHub branch protection]
    • check-neutral = Matrixone CI / UT Test on Ubuntu/x86
    • check-skipped = Matrixone CI / UT Test on Ubuntu/x86
    • check-success = Matrixone CI / UT Test on Ubuntu/x86
  • any of: [🛡 GitHub branch protection]
    • check-neutral = Matrixone Utils CI / Coverage
    • check-skipped = Matrixone Utils CI / Coverage
    • check-success = Matrixone Utils CI / Coverage
All conditions
  • any of [🛡 GitHub branch protection]:
    • check-neutral = Matrixone CI / UT Test on Ubuntu/x86
    • check-skipped = Matrixone CI / UT Test on Ubuntu/x86
    • check-success = Matrixone CI / UT Test on Ubuntu/x86
  • any of [🛡 GitHub branch protection]:
    • check-neutral = Matrixone Utils CI / Coverage
    • check-skipped = Matrixone Utils CI / Coverage
    • check-success = Matrixone Utils CI / Coverage
  • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
  • github-review-approved [🛡 GitHub branch protection] (documentation)
  • github-review-decision = APPROVED [🛡 GitHub branch protection] (documentation)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Compose CI / multi cn e2e bvt test docker compose(PESSIMISTIC)
    • check-neutral = Matrixone Compose CI / multi cn e2e bvt test docker compose(PESSIMISTIC)
    • check-skipped = Matrixone Compose CI / multi cn e2e bvt test docker compose(PESSIMISTIC)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
    • check-neutral = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
    • check-skipped = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
    • check-neutral = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
    • check-skipped = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone CI / SCA Test on Linux/arm64
    • check-neutral = Matrixone CI / SCA Test on Linux/arm64
    • check-skipped = Matrixone CI / SCA Test on Linux/arm64

Reason

The merge conditions cannot be satisfied due to failing checks

Failing checks:

Hint

You may have to fix your CI before adding the pull request to the queue again.
If you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio queue comment.

Tick the box to put this pull request back in the merge queue (same as @mergifyio queue).

  • Requeue this pull request

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug Something isn't working size/XL Denotes a PR that changes [1000, 1999] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants