fix(mysql,trino,singlestoredb): translate minutes token in strftime#12027
Draft
deepyaman wants to merge 1 commit into
Draft
fix(mysql,trino,singlestoredb): translate minutes token in strftime#12027deepyaman wants to merge 1 commit into
deepyaman wants to merge 1 commit into
Conversation
`strftime` with a format containing a time component (e.g. `"%H:%M:%S"`) emitted the wrong thing on MySQL/Trino/SingleStoreDB: Python's `%M` (minutes) was rendered as the dialect's `%M` (month *name*), so formatting a timestamp produced the month in the minutes position (e.g. "03:January:05" for 03:04:05). The cause is the same as ibis-project#12025 for the parse direction: `ops.Strftime` was routed through `self.f.date_format(..., dialect=...)` (via SIMPLE_OPS), which re-parses the format string as if it were dialect-native. Dropping that SIMPLE_OPS entry lets these backends fall back to the base `visit_Strftime`, which builds `sge.TimeToStr` directly and lets the generator translate the canonical format -> `DATE_FORMAT(x, '%Y-%m-%d %T')`. SingleStoreDB inherits the fix from MySQL. Adds a regression test that formats a timestamp with a time component. Assisted-by: Claude Opus 4.8 <noreply@anthropic.com>
github-actions
Bot
added
tests
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
Draft — companion to #12025 (same root cause, the format direction instead of parse). I don't have local MySQL/Trino/SingleStoreDB servers, so this is compile-validated; CI is the execution check. Markers beyond druid/exasol may need adding once CI runs.
Description
strftimewith a time component silently emits the wrong value on MySQL/Trino/SingleStoreDB:Root cause (same as #12025, opposite direction)
ops.Strftimewas routed throughself.f.date_format(arg, fmt, dialect=…)viaSIMPLE_OPS. Passingdialect=makes sqlglot re-parse the format string as if it were dialect-native, so Python's%M(minutes) is read as the dialect's%M(month name). Compiled SQL wasDATE_FORMAT(t, '%Y-%m-%d %H:%M:%s')— that%Mformats the month.Fix
The base compiler already has a correct
visit_Strftimethat buildssge.TimeToStrdirectly. mysql/trino'sSIMPLE_OPSentry was overriding it with the buggy parse-converting version. Dropping the entry restores the base behavior:It's a pure 2-line deletion;
DATE_FORMATis the same function these backends already emitted, just with the correct format (no SQL-shape change). SingleStoreDB inherits the fix from MySQL.Adds
test_strftime_with_time— the existingtest_strftimeonly uses"%Y%m%d"(date-only), which is why this slipped through.🤖 Generated with Claude Code