Skip to content

doc: clarify timeTruncate units and when to use it - #1792

Open
milanobrtlik wants to merge 1 commit into
go-sql-driver:masterfrom
milanobrtlik:doc-time-truncate
Open

doc: clarify timeTruncate units and when to use it#1792
milanobrtlik wants to merge 1 commit into
go-sql-driver:masterfrom
milanobrtlik:doc-time-truncate

Conversation

@milanobrtlik

Copy link
Copy Markdown

Description

timeTruncate is documented as what it does, but not why you would enable it — and the listed unit suffixes omit the one that matters most for it.

1. The unit list is incomplete. The current sentence lists "ms", "s", "m", "h"; it is a copy of the one under readTimeout, where sub-millisecond units are pointless. time.ParseDuration also accepts ns and us, and 1us is the interesting value here because it matches DATETIME(6). As written, the README implies microsecond truncation isn't available.

2. There is no motivation. time.Time arguments are formatted with up to 9 fractional digits (appendDateTime trims only trailing zeros), so a time.Now()-derived value almost always carries more precision than the column stores. On MariaDB this silently prevents an index range scan — nothing errors and nothing is logged.

Measured on a 1M-row table with an indexed DATETIME(6) column, WHERE last_activity_at >= ? with a 9-fractional-digit argument, driver defaults:

engine without timeTruncate with timeTruncate=1us
MariaDB 10.5.29 59.0 ms — 1,000,000 index reads 275 µs — 876
MariaDB 10.11.15 53.8 ms — 1,000,000 index reads 363 µs — 876
MariaDB 11.8.8 58.6 ms — 1,000,000 index reads 257 µs — 876
MySQL 5.7.44 377 µs — 876 index reads 248 µs — 876
MySQL 8.4.11 607 µs — 876 index reads 280 µs — 876

Row counts are identical in every run (876), so truncation does not change the result set. MySQL is unaffected across the supported range; only MariaDB degrades, which is why the README text names it rather than generalising.

On MariaDB, EXPLAIN shows type going from range (1000 rows) to index (998490 rows). key and key_len are unchanged, so the plan still looks like it is using the index — which is what makes this easy to miss.

The interpolateParams=true path behaves identically (both paths go through appendDateTime).

I have deliberately kept these numbers out of the README; the README change only states the qualitative behaviour.

3. Config.Params is a footgun here. Config.timeTruncate is unexported, so the natural-looking cfg.Params["timeTruncate"] = "1us" does not set it. Instead the key falls through to handleParams, which emits SET timeTruncate = 1us at connect time and fails every connection. The supported paths are the DSN string or cfg.Apply(mysql.TimeTruncate(time.Microsecond)). This is also true of compress and charset; I documented it only under timeTruncate to keep the change small, but I'm happy to drop that paragraph or move it to a general note under #### Parameters if you'd prefer.

Reproduction

Fixture (portable across both engines, no CTEs):

CREATE TABLE tt_seq (n TINYINT NOT NULL);
INSERT INTO tt_seq VALUES (0),(1),(2),(3),(4),(5),(6),(7),(8),(9);

CREATE TABLE tt_demo (
  id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY,
  last_activity_at DATETIME(6) NOT NULL,
  KEY idx_last_activity_at (last_activity_at)
) ENGINE=InnoDB;

INSERT INTO tt_demo (last_activity_at)
SELECT TIMESTAMPADD(MICROSECOND,
  (a.n + b.n*10 + c.n*100 + d.n*1000 + e.n*10000 + f.n*100000) * 1000,
  '2024-01-01 00:00:00.000000')
FROM tt_seq a, tt_seq b, tt_seq c, tt_seq d, tt_seq e, tt_seq f;

ANALYZE TABLE tt_demo;

Server-side control, no driver involved:

EXPLAIN SELECT COUNT(*) FROM tt_demo WHERE last_activity_at >= '2024-01-01 00:16:39.000000';
EXPLAIN SELECT COUNT(*) FROM tt_demo WHERE last_activity_at >= '2024-01-01 00:16:39.000000123';

Driver side — run with and without timeTruncate=1us, using time.Date(2024, 1, 1, 0, 16, 39, 123456789, time.UTC) as the argument, on a single pinned *sql.Conn, reading Handler_read_next + Handler_read_rnd_next before and after:

conn.QueryRowContext(ctx, "SELECT COUNT(*) FROM tt_demo WHERE last_activity_at >= ?", arg).Scan(&n)

SELECT CAST(? AS CHAR) with the same argument confirms what goes on the wire:
2024-01-01 00:16:39.123456789 without the option, 2024-01-01 00:16:39.123456 with it.

Checklist

  • Code compiles correctly
  • Created tests which fail without the change (if possible) — docs-only, no behaviour change
  • All tests passing
  • Extended the README / documentation, if necessary
  • Added myself / the copyright holder to the AUTHORS file — docs-only change with no copyrightable code, so I left this alone to match previous README-only commits; happy to add myself if you'd prefer

The listed unit suffixes omitted "ns" and "us", which hid the most
useful setting for DATETIME(6) columns. Also document that truncation
applies to query arguments only, why truncating to the column's
precision matters for indexed comparisons, and that the option cannot
be set through Config.Params.
@milanobrtlik

Copy link
Copy Markdown
Author

For context: this documents the motivation timeTruncate was originally introduced for. #1541 was opened with exactly this MariaDB index-degradation case (including EXPLAIN screenshots comparing MariaDB 11.2.2 with MySQL 8.3), which in turn followed from #1121 / #1172, where the driver stopped rounding time.Time and began sending full nanosecond precision.

None of that reached the README, so the parameter currently reads as a generic rounding knob rather than the index-usage fix it was added to be. The measurements above just re-confirm it across the currently supported range (MariaDB 10.5-11.8, MySQL 5.7-8.4) and add the Config.Params note.

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

The README expands timeTruncate documentation to cover nanosecond-capable durations, query argument truncation, server-read values, and configuration through the DSN or mysql.TimeTruncate.

Changes

timeTruncate documentation

Layer / File(s) Summary
Document duration precision and configuration
README.md
Documents supported duration units, including 1us, explains truncation of sent time.Time arguments, and clarifies DSN and cfg.Apply(mysql.TimeTruncate(...)) configuration.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Possibly related PRs

  • go-sql-driver/mysql#1789: Both PRs address timeTruncate DSN duration handling. This PR updates documentation, while the related PR changes parsing and validation code.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the documentation change to clarify timeTruncate units and usage.
Description check ✅ Passed The description directly explains the README changes, their motivation, supported configuration paths, and documentation-only scope.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@README.md`:
- Line 317: Update the README description for the timeTruncate query argument to
explicitly document "0" as a valid value that disables truncation, while
retaining the existing unit-suffixed duration formats and examples.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 48b7e9b5-bb89-44b9-9edd-d44bd509fb7d

📥 Commits

Reviewing files that changed from the base of the PR and between 416cd99 and f0bf295.

📒 Files selected for processing (1)
  • README.md

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant