Skip to content

MDEV-40467 [to-squash] Improve Y2038/2106 problem error message#5436

Open
mariadb-YuchenPei wants to merge 1 commit into
bb-main-mdev-15621from
bb-main-mdev-40467
Open

MDEV-40467 [to-squash] Improve Y2038/2106 problem error message#5436
mariadb-YuchenPei wants to merge 1 commit into
bb-main-mdev-15621from
bb-main-mdev-40467

Conversation

@mariadb-YuchenPei

Copy link
Copy Markdown
Contributor

When nearing the end times TIMESTAMP_MAX_VALUE, auto-partition creation may encounter overflow when calculating new partition range. This patch improves the message of such an overflow

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@AITWINS

AITWINS commented Jul 22, 2026 via email

Copy link
Copy Markdown

@mariadb-YuchenPei

Copy link
Copy Markdown
Contributor Author

claude review comment


Review: 13d30768 — MDEV-40467 [to-squash] Improve Y2038/2106 problem error message

Overview

Same targeted code change as the prior revision, now with a complete, self-consistent test. In check_range_interval_constants(), a TIMESTAMP partition boundary that crosses TIMESTAMP_MAX_VALUE during DML-triggered auto-partition creation raises a clean ER_DATA_OUT_OF_RANGE. The test (at timestamp=4294967295, i.e. 2106-02-07) covers TIMESTAMP (errors) plus DATE and DATETIME (succeed), each with recorded show create table output.

Resolved from previous reviews ✅

  • Type-scoping fix is correct — the if (error) guard lives inside case MYSQL_TYPE_TIMESTAMP:, so DATE/DATETIME columns aren't spuriously rejected near 2106.
  • .test/.result are now in sync — the previously-missing show create table t1; output is recorded for all three cases. The TIMESTAMP case correctly shows only p0 (proving the failed auto-creation is atomic — no partial p1…pN are added on error), while DATE/DATETIME show p0…p7 created up to the 2106-02-08 boundary. This is a genuinely good, behavior-locking test.

Correctness

  • Logic is sound. The error out-param from Timestamp_or_zero_datetime is only consulted where it's meaningful, and the recorded results match the expected partition math (transition 2106-02-01 + 1-day steps up to end = query_start, one overshoot to 2106-02-08).

Potential risk — timezone determinism (worth a look)

  • The number of partitions created depends on converting the max timestamp (4294967295) to local time via the session time zone (gmt_sec_to_TIME(&end_time, thd->query_start())), and the recorded result hard-codes p0…p7 (last boundary 2106-02-08). The test does not pin time_zone. Under a sufficiently negative UTC offset, end could fall on 2106-02-06, yielding one fewer partition and a result mismatch.
    • In practice MTR runs with a fixed default TZ so CI is likely stable, but relying on that is implicit. Recommend making it explicit and self-documenting:
      SET time_zone='+00:00';
      ...
      SET time_zone=default;
    This also guards against future MTR/host changes.

Minor

  • Duplicated my_error block — now identical to the date_add_interval overflow handler earlier in the same function. Optional: extract a tiny helper. Not blocking.
  • set timestamp never restored — add set timestamp=default; at the end per convention (it's the last block today, so harmless, but tidy).
  • Comment nit: with the check now scoped to TIMESTAMP, /* Likely Year 2038/2106 Problem */ could drop "Likely" — the cause is definite (timestamp range exceeded).

Performance & security

  • No performance impact (single branch on the cold partition-creation path); no security concerns.

Summary

This revision resolves both prior findings — the code change is correct and well-scoped, and the test is now consistent and meaningfully verifies TIMESTAMP-errors / DATE-DATETIME-succeed plus error atomicity. The only substantive follow-up is pinning time_zone so the recorded partition count is deterministic across environments; the duplicated my_error and missing set timestamp=default; are cosmetic. Good to squash once the time-zone point is addressed (or consciously accepted).

When nearing the end times TIMESTAMP_MAX_VALUE, auto-partition
creation may encounter overflow when calculating new partition range.
This patch improves the message of such an overflow
@mariadb-YuchenPei

Copy link
Copy Markdown
Contributor Author

claude:


Review: 2e423a48 — MDEV-40467 [to-squash] Improve Y2038/2106 problem error message

Overview

Same code change as prior revisions, with the test now pinned to a fixed time zone. In check_range_interval_constants(), a TIMESTAMP auto-interval partition boundary crossing TIMESTAMP_MAX_VALUE raises a clean ER_DATA_OUT_OF_RANGE. The test (time_zone='+00:00', timestamp=4294967295) covers TIMESTAMP (errors, only p0 remains) plus DATE and DATETIME (succeed, p0…p7 created).

All prior findings resolved ✅

  • Type-scopingif (error) correctly lives inside case MYSQL_TYPE_TIMESTAMP:; DATE/DATETIME unaffected.
  • .test/.result syncshow create table output recorded for all three cases; TIMESTAMP case confirms error atomicity (no partial p1…pN).
  • Time-zone determinismSET time_zone='+00:00' pins the max-timestamp→local conversion, so the recorded p0…p7 count is stable across environments. With +00:00, end = 2106-02-07 06:28:15 and stepping from 2106-02-01 yields exactly p0…p7 — matches the recorded result. Correctly restored with SET time_zone=default;.

Correctness

  • Logic and recorded results are consistent and reproducible. No issues.

Minor (all optional, non-blocking)

  • set timestamp not restored. time_zone is now reset, but timestamp is left overridden. It's the last block in the file so there's no bleed, but for symmetry consider set timestamp=default; alongside the time_zone reset.
  • Duplicated my_error block — identical to the date_add_interval overflow handler earlier in the same function; could be factored into a small helper, but fine as-is given it's a fixup.
  • Comment nit: with the check scoped to TIMESTAMP, /* Likely Year 2038/2106 Problem */ — "Likely" is no longer needed; the cause is definite.

Performance & security

  • No performance impact (one branch on the cold partition-creation path); no security concerns.

Summary

This revision resolves every substantive point raised across earlier revisions — the fix is correct and well-scoped, and the test is now consistent, meaningful (error path + atomicity + DATE/DATETIME success), and deterministic. Ready to squash. The only leftovers are cosmetic: restoring timestamp, the duplicated my_error, and the "Likely" wording — none of which need to block.

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

Development

Successfully merging this pull request may close these issues.

4 participants