Skip to content

Fix: generate_surrogate_key renders valid hex strings on MySQL, T-SQL, StarRocks and Snowflake - #6033

Open
ptimizeroracle wants to merge 1 commit into
SQLMesh:mainfrom
ptimizeroracle:fix/surrogate-key-hex-mysql-tsql
Open

Fix: generate_surrogate_key renders valid hex strings on MySQL, T-SQL, StarRocks and Snowflake#6033
ptimizeroracle wants to merge 1 commit into
SQLMesh:mainfrom
ptimizeroracle:fix/surrogate-key-hex-mysql-tsql

Conversation

@ptimizeroracle

Copy link
Copy Markdown

Description

@GENERATE_SURROGATE_KEY promises a hex string on every dialect, but on four
adapter dialects the rendered SQL is invalid or returns binary:

  • MySQL: hash_function := 'SHA256' / 'SHA512' rendered as bare
    SHA256(...) / SHA512(...). MySQL only has SHA2(expr, digest_length),
    so the query fails at runtime:

    ERROR 1305 (42000): FUNCTION sqlmesh.SHA256 does not exist
    

    (reproduced on MySQL 8.4.11). The cause: MySQL's sqlglot parser has no
    entry for SHA256, so exp.func returns Anonymous and the existing
    SHA2Digest/MD5Digest conversions never fire.

  • T-SQL (MSSQL, Fabric): every hash renders as HASHBYTES(...), which
    returns VARBINARY, not a hex string. This includes the default MD5
    path, so every plain @GENERATE_SURROGATE_KEY(a) on these adapters
    produced binary keys. Reproduced on Azure SQL Edge:
    HASHBYTES('MD5','x') -> 0x9DD4E461... while DuckDB returns
    9dd4e461....

  • StarRocks, Snowflake: same Anonymous fall-through as MySQL; bare
    SHA256(...) is not a valid function on either engine (both spell it
    SHA2(expr, length), length optional on Snowflake).

Fix, in generate_surrogate_key:

  1. The Anonymous -> exp.SHA2(length) mapping is no longer scoped to the
    Presto family: any dialect whose parser hands back Anonymous for a
    known SHA-2 name now gets the typed call with the canonical digest
    length. Unknown hash names still pass through untouched (MYHASH case
    covered by the existing test).
  2. T-SQL family gets an explicit hex conversion, mirroring the Presto
    TO_HEX wrap: LOWER(CONVERT(VARCHAR(n), HASHBYTES(...), 2)), with n
    sized to the digest width. Like the Presto branch, a probe keeps it inert
    if the sqlglot tsql generator ever emits the conversion itself.

End-to-end verification on live engines, fixed rendering executed and
compared against DuckDB / hashlib oracles (input 'x'):

hash MySQL 8.4.11 Azure SQL Edge (T-SQL) oracle
MD5 valid before 9dd4e461268c8034f5c8564e155c67a6 DuckDB md5 match
SHA1 valid before 11f6ad8ec52a2984abaafd7c3b516503785c2072 DuckDB sha1 match
SHA256 2d711642b726b04401627ca9fbac32f5c8530fb1903cc4db02258717921a4881 same DuckDB sha256 match
SHA512 a4abd4448c49562d...afe62 (128 hex) same form Python hashlib match

The existing test_generate_surrogate_key_hash_semantics snowflake assertion
was pinning the bare SHA256(...) form; since that is not a Snowflake
function, it is updated to the mapped SHA2(..., 256) form (Snowflake
defaults the digest size to 256).

Same invariant family as #5888 (Presto/Trino) and #5871 (Athena).

Test Plan

  • New test_generate_surrogate_key_hex_string_on_mysql_tsql_starrocks pins
    exact rendered SQL for mysql SHA256/SHA512, starrocks SHA256, tsql
    MD5/SHA1/SHA256 and fabric SHA512; fails on main, passes with the fix.
  • Full tests/core/test_macros.py (143 tests) and the macros doctests pass.
  • Live-engine execution evidence above (MySQL 8.4.11 and Azure SQL Edge
    containers, scripts in the PR discussion).
  • ruff check, ruff format, and mypy (pinned 1.13) clean on the changed
    files.

Checklist

  • I have run make style on the code (ruff + format + mypy equivalents on changed files)
  • I have added tests that prove my fix is effective
  • pytest tests/core/test_macros.py passes (143 passed)
  • Commits are signed off (DCO)

…l, tsql, starrocks, snowflake

Signed-off-by: ptimizeroracle <contact@binblok.com>
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