fix(mysql,starrocks)!: correct DATEDIFF, LIKE, VARIANCE and TO_DAYS semantics [CLAUDE] - #7967
Closed
UCHPUCHMAK wants to merge 1 commit into
Closed
fix(mysql,starrocks)!: correct DATEDIFF, LIKE, VARIANCE and TO_DAYS semantics [CLAUDE]#7967UCHPUCHMAK wants to merge 1 commit into
UCHPUCHMAK wants to merge 1 commit into
Conversation
…emantics [CLAUDE]
All four rewrites below silently produced different numbers rather than an
error. Verified by executing both the original and the generated form on
StarRocks 3.5.18.
StarRocks DATEDIFF counts crossed day boundaries while DATE_DIFF counts whole
units, so rewriting one into the other is lossy on DATETIME inputs:
DATEDIFF('2026-03-11 01:00:00', '2026-03-10 23:00:00') -> 1
DATE_DIFF('DAY', '2026-03-11 01:00:00', '2026-03-10 23:00:00') -> 0
DATEDIFF now sets date_part_boundary, the flag already used for exactly these
semantics by BigQuery, MySQL and Snowflake. On generation, a boundary diff
over DAY (or with no unit, i.e. MySQL's DATEDIFF) lands on DATEDIFF, and any
other unit truncates its operands down to the unit, mirroring the Presto
generator, since whole-unit counting over truncated operands equals boundary
counting:
DATE_DIFF('MONTH', '2026-03-01', '2026-02-28') -> 0
DATE_DIFF('MONTH', DATE_TRUNC('MONTH', ...), DATE_TRUNC(...)) -> 1
Week units realign StarRocks' Monday-based DATE_TRUNC('WEEK', ...) to the
requested week start by shifting both operands. Units DATE_TRUNC cannot
handle, e.g. ISOYEAR, stay untruncated and fail loudly on the server;
MILLISECOND is also left alone: the analyzer rejects sub-second DATE_TRUNC
over DATETIME-typed arguments, and second-precision values coincide anyway. Diffs parsed from dialects that count
whole units keep generating plain DATE_DIFF.
StarRocks' LIKE(expr, pattern) takes its arguments in the natural order, but
the base builder reverses them, which flipped the predicate:
like('xyz', 'x%') -> 1, 'x%' LIKE 'xyz' -> 0
MySQL's VARIANCE is a synonym of VAR_POP, not of VAR_SAMP, and MySQL has no
VARIANCE_POP at all, so exp.Variance -> VARIANCE turned a sample variance into
a population one and exp.VariancePop -> VARIANCE_POP generated invalid MySQL:
var_samp(x) -> 2.3333, variance(x) = var_pop(x) -> 1.5556
Doris documents the same population semantics for VARIANCE and inherits this
fix; SingleStore already carried identical overrides, which are now redundant
with the base MySQL behavior.
StarRocks supports TO_DAYS natively, so MySQL's expansion into DATE_DIFF + 1
is both unnecessary and off by one there:
to_days('2026-03-10') -> 740050
DATE_DIFF('DAY', DATE('2026-03-10'), DATE('0000-01-01')) + 1 -> 740051
As a consequence StarRocks TO_DAYS no longer lowers into day arithmetic for
targets without the function; round-trip fidelity on StarRocks itself is the
priority here, matching how the base dialect treats TO_DAYS.
STDDEV is left alone: MySQL's STDDEV/STD/STDDEV_POP are population functions
too, but the round trip is already stable and changing exp.Stddev would affect
every dialect.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
|
@UCHPUCHMAK The PR makes a lot of changes all at once. Moreover, the description is AI generated and not very helpful for reviewing. I suggest you read the following: https://github.com/tobymao/sqlglot/blob/main/CONTRIBUTING.md . After that you can break the PR into smaller ones. |
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
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.
Fixes four rewrites in the StarRocks/MySQL dialects that silently produced different numbers rather than an error:
DATEDIFF(a, b)(StarRocks)DATE_DIFF('DAY', a, b)0→1on DATETIMEs crossing midnightlike(a, 'x%')(StarRocks)'x%' LIKE a0→1VAR_SAMP(x)/VAR_POP(x)(MySQL family)VARIANCE(x)/VARIANCE_POP(x)VARIANCE_POPisn't valid MySQLTO_DAYS(x)(StarRocks)(DATE_DIFF('DAY', DATE(x), DATE('0000-01-01')) + 1)740051→740050How this was verified
Every claim below was checked by executing both the original and the generated SQL on a live StarRocks 3.5.18 cluster and comparing results, not just parse round-trips. The MySQL-layer changes are additionally backed by the MySQL 8.4 Reference Manual, and the two other dialects inheriting the MySQL layer were checked explicitly: Doris (vendor docs confirm the same semantics) and SingleStore (already carried identical overrides, so this is a no-op there).
1. DATEDIFF ↔ DATE_DIFF
StarRocks
DATEDIFFcounts crossed day boundaries ("Only the date parts of the values are used"), whileDATE_DIFFcounts whole units, so rewriting one into the other is lossy on DATETIME inputs:DATEDIFFnow parses intoexp.DateDiff(..., date_part_boundary=True)— the flag BigQuery, MySQL and Snowflake parsers already use for boundary-crossing semantics. Generation handles the full unit matrix (each row executed on the cluster):DAY, or no unit (MySQLDATEDIFF)DATEDIFF(a, b)MONTH,QUARTER,YEAR,HOUR,MINUTE,SECONDDATE_DIFF('U', DATE_TRUNC('U', a), DATE_TRUNC('U', b))WEEK/WEEK(<DAY>)/ISOWEEKDATE_TRUNC('WEEK', x + INTERVAL 'n' DAY)on both operandsDATE_TRUNC('WEEK', ...)is Monday-based; the sharedweek_unit_to_dowhelper computes the shift (+1for Sunday,0for Monday/ISOWEEK, ...)MILLISECONDDATE_DIFF('MILLISECOND', a, b)untruncatedDATE_TRUNCover DATETIME-typed arguments (verified on 3.5.18 even withNOW(6), despite the docs advertising support since 3.1.7); for second-precision values whole-ms and boundary counting coincide anywayISOYEARand anything elseDATE_TRUNCcan't handleDATE_DIFF('ISOYEAR', a, b)untruncatedDATE_DIFFSample cluster evidence for the truncation rows (raw
DATE_DIFFreturns0for all of these):As a side effect,
DATEDIFFfrom StarRocks now fans out correctly to other dialects too, e.g. postgres now gets(CAST(a AS DATE) - CAST(b AS DATE))instead of an epoch division that counted whole days.2. LIKE()
StarRocks
like(expr, pattern)takes its arguments in the natural order, but the base builder reverses them, which flipped the predicate:The parser now uses
build_like(exp.Like), same as snowflake/spark/clickhouse. The 3-arg ESCAPE form keeps working through the same builder.3. VARIANCE family (MySQL layer)
Per the MySQL docs,
VARIANCE()is a synonym forVAR_POP()("Returns the population standard variance ... provided as a MySQL extension"),VAR_SAMP()is the sample variance, andVARIANCE_POPdoes not exist. sqlglot treatedVARIANCEas the sample variance (exp.Variance._sql_namesincludes it) and generatedVARIANCE_POPforexp.VariancePop, so:VAR_SAMP(x)→VARIANCE(x): a sample variance silently became a population one —2.3333vs1.5556on(1, 2, 4)VAR_POP(x)→VARIANCE_POP(x): invalid MySQLThe MySQL parser now maps
VARIANCEtoexp.VariancePop, and the generator emitsVAR_SAMP/VAR_POP. Inheritance checked: StarRocks documents the same population semantics (variance,var_pop,variance_pop with a separatevar_samppage), and so does Doris ("Unlike VARIANCE (population variance), VAR_SAMP uses n-1"); SingleStore already carried identical parser/generator overrides, which this makes redundant rather than conflicting.4. TO_DAYS
StarRocks supports
TO_DAYSnatively, and MySQL's expansion is off by one there:StarRocks now parses
TO_DAYSinto the existingexp.ToDays; MySQL keeps its expansion, which is correct for MySQL.Breaking changes (hence the
!)VARIANCE(x)→VAR_POP(x),VAR_SAMP(x)staysVAR_SAMP(x)(previouslyVARIANCE(x))DATEDIFF(a, b)staysDATEDIFF(a, b)(previouslyDATE_DIFF('DAY', a, b)),TO_DAYS(x)staysTO_DAYS(x)TO_DAYSno longer lowers into day arithmetic for targets without the function — round-trip fidelity on StarRocks is the priority, matching how the base dialect treatsTO_DAYSDeliberately out of scope
STDDEV: MySQL'sSTDDEV/STD/STDDEV_POPare population functions with the same naming quirk, but their round trip is already stable and changingexp.Stddevwould affect every dialectDATE_DIFF('MONTH', ...)→ BigQuery) is a target-generator concern shared by all whole-unit dialectsTIMESTAMPDIFFflows throughexp.TimestampDiffand is untouchedTests: identity/cross-dialect coverage for every rewrite in
test_starrocks.py/test_mysql.py, including pins for the MONTH/HOUR/WEEK/ISOWEEK/ISOYEAR unit shapes and the Doris/DuckDB/Postgres fan-outs.🤖 Generated with Claude Code