Skip to content

fix temporal part 2#25899

Open
daviszhen wants to merge 23 commits into
matrixorigin:mainfrom
daviszhen:0716-fix-temporal2
Open

fix temporal part 2#25899
daviszhen wants to merge 23 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 →

@daviszhen

Copy link
Copy Markdown
Contributor Author

I still see several merge blockers on the latest head.

  1. pkg/frontend/mysql_protocol.go:3741 still aliases raw Date(0) to the MySQL zero-date wire encoding in the binary protocol. After this PR, Date(0) is the valid minimum date 0001-01-01, while the dedicated zero-date sentinel is types.ZeroDate (formatDateForMySQL already special-cases only types.ZeroDate at mysql_protocol.go:3980-3984). So text mode preserves 0001-01-01, but prepared statements / binary-protocol clients serialize that same real value as the zero date. appendDate needs to check value == types.ZeroDate, not int32(value) == 0.
  2. DATE_FORMAT was left out of the zero-temporal closure. DateFormat still feeds zero temporals straight into makeDateFormat (pkg/sql/plan/function/func_binary.go:3623-3633), and makeDateFormat then calls DayOfWeek(), Week(), YearWeek(), DayOfYear(), etc., with no zero guard (func_binary.go:3925-4045). %b/%M even raise an error on month 0. But this same PR makes the corresponding scalar functions on zero temporals return NULL, and the new BVT keeps 0001-01-01 separate from the zero sentinel. Right now DAYNAME(zero) becomes NULL, while DATE_FORMAT(zero,'%W') fabricates a weekday and DATE_FORMAT(zero,'%M') errors. DATE_FORMAT needs the same zero-temporal semantics as the direct date-name/week functions.
  3. The new valid ZeroTimestamp breaks mo_iscp_log.drop_at's old raw-zero sentinel contract. ValidTimestamp now accepts types.ZeroTimestamp (pkg/container/types/timestamp.go:293-294), and the new temporal BVTs make zero TIMESTAMP values storable. ISCP replay reads any non-null drop_at directly from storage (pkg/iscp/executor.go:746-759, 846-852), then TableEntry still treats only dropAt == 0 as “not dropped” and uses dropAt != 0 for candidate exclusion / GC (pkg/iscp/table_entry.go:89-90, 100-101, 120-121, 151-152). A stored 0000-00-00 00:00:00 therefore looks dropped forever, and its GC age check goes through UnixTimestamp() == -1, which is then cast to uint64. Please stop using raw numeric zero as the unset sentinel here, or normalize zero TIMESTAMPs away before they enter ISCP state.

相对 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 定义。

@daviszhen

Copy link
Copy Markdown
Contributor Author

Codex automated review

Loaded all existing reviews, issue comments, and inline threads. Three blocking regressions remain on the checked-out head.

P1 - Binary protocol serializes 0001-01-01 as a zero date (pkg/frontend/mysql_protocol.go:3742)

Date(0) is now the valid minimum date (0001-01-01), while the zero-date sentinel is types.ZeroDate (-1). appendDate still tests int32(value) == 0 and emits a zero-length MySQL date, so prepared/binary-protocol result sets return 0001-01-01 as 0000-00-00. The text path already checks d == types.ZeroDate. Test the sentinel explicitly and add binary-protocol coverage for both values.

P1 - DATE_FORMAT has no zero-temporal handling (pkg/sql/plan/function/func_binary.go:3623)

DateFormat dispatches every non-NULL temporal value to makeDateFormat, including the newly supported zero sentinel. A zero datetime has month 0, so %b/%M fail the formatter's month validation, while other specifiers can produce fabricated values. Adjacent zero-temporal scalar functions in this change return NULL instead. Handle zero temporal values before formatting and add regression cases.

P1 - Zero TIMESTAMP corrupts ISCP drop-state semantics (pkg/container/types/timestamp.go:294)

ValidTimestamp now accepts ZeroTimestamp (-1), but ISCP still uses raw numeric drop_at == 0 as its active-state sentinel. pkg/iscp/executor.go loads any non-NULL drop_at unchanged, and pkg/iscp/table_entry.go treats it as dropped and converts it to an unsigned Unix timestamp during GC. A stored zero timestamp is therefore immediately hidden and can be selected for deletion. ISCP metadata needs an explicit zero-timestamp policy/normalization before globally accepting this value.

相对 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 定义。

@aunjgr aunjgr 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.

Reviewed the latest head independently across the temporal sentinel closure. Binary and text protocol paths distinguish ZeroDate from valid Date(0); zero DATE/DATETIME/TIMESTAMP handling is propagated through arithmetic, formatting, scalar functions, casts/load paths, and process serialization; ISCP normalizes a non-NULL zero timestamp away from its raw-zero active-job sentinel before state admission. I found no remaining merge-blocking correctness or lifecycle issue in the current code. The Ubuntu UT and coverage checks are still failing and must pass before merge.

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.

5 participants