Skip to content

Fix: generate_surrogate_key returns hex strings for SHA256/SHA512 on Presto and Trino#5888

Open
Pawansingh3889 wants to merge 4 commits into
SQLMesh:mainfrom
Pawansingh3889:fix/surrogate-key-sha2-hex
Open

Fix: generate_surrogate_key returns hex strings for SHA256/SHA512 on Presto and Trino#5888
Pawansingh3889 wants to merge 4 commits into
SQLMesh:mainfrom
Pawansingh3889:fix/surrogate-key-sha2-hex

Conversation

@Pawansingh3889

@Pawansingh3889 Pawansingh3889 commented Jul 7, 2026

Copy link
Copy Markdown

Description

Fixes #5871.

@GENERATE_SURROGATE_KEY(..., hash_function := 'SHA256') produces a key that changes value depending on the engine. With MD5 the macro converts the parsed exp.MD5Digest into exp.MD5, so Presto and Trino render the hex-string form (LOWER(TO_HEX(MD5(TO_UTF8(...))))). With SHA256/SHA512 there is no equivalent conversion, so Trino gets a bare SHA256(varchar): a type error at execution time, and a binary digest rather than a hex string where it does run.

Two halves to the fix:

  1. exp.SHA2Digest results convert to exp.SHA2 (keeping the length), mirroring the MD5 conversion one line above, and the concatenated argument is annotated as text so generators that wrap an encode around string inputs (TO_UTF8 on Presto/Trino) can do so. This is the half that matters once the sqlglot pin includes Fix(presto)!: preserve SHA256/SHA512 digest semantics, render SHA2 as hex string [CLAUDE] tobymao/sqlglot#7824 (merged upstream, not yet in a release).

  2. A Presto-family fallback for the currently pinned sqlglot (~=30.8.0), where the trino/presto parsers hand back exp.SHA2 directly and render it as a bare SHA256(varchar). When a cached probe (_sha2_renders_binary) sees that the target dialect renders exp.SHA2 without TO_HEX, the macro builds the hex-string form itself, the same shape those generators produce for MD5. Once the pin moves past #7824 the probe turns the fallback off and the macro defers to the native rendering, so nothing double-wraps.

The repro from #5871 on this branch (trino):

SELECT LOWER(TO_HEX(SHA256(TO_UTF8(CONCAT(CAST(COALESCE(CAST(a AS VARCHAR), '_sqlmesh_surrogate_key_null_') AS VARCHAR), CAST('|' AS VARCHAR), CAST(COALESCE(CAST(b AS VARCHAR), '_sqlmesh_surrogate_key_null_') AS VARCHAR)))))) AS k FROM foo

Rendered SQL for duckdb, bigquery and snowflake is unchanged.

Test Plan

test_generate_surrogate_key_hash_semantics in tests/core/test_macros.py asserts: the macro returns exp.SHA2 (never a digest node) with a text-typed argument; the exact Trino and Presto SHA256 and SHA512 output strings (the reported fix); the fallback stays off for duckdb/bigquery and snowflake output is unchanged. The Trino/Presto string assertions hold on both sides of the sqlglot pin bump.

Full tests/core/test_macros.py: 139 passed. ruff check and ruff format clean on the changed files.

Checklist

  • I have run make style and fixed any issues (ruff check and ruff format pass on the changed files; happy to fix anything else CI flags)
  • I have added tests for my changes (if applicable)
  • All existing tests pass (make fast-test) - ran tests/core/test_macros.py in full (139 passed); leaving the broader suite to CI
  • My commits are signed off (git commit -s) per the DCO

…Presto and Trino

Signed-off-by: Pawan Singh Kapkoti <42340841+Pawansingh3889@users.noreply.github.com>
@Pawansingh3889 Pawansingh3889 force-pushed the fix/surrogate-key-sha2-hex branch from a6384fe to f580eb3 Compare July 7, 2026 14:26
@mday-io

mday-io commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Thanks for picking this up. I think the direction makes sense, but this PR does not actually fix the reported Trino/Presto behavior with the currently pinned sqlglot~=30.8.0.

On this branch, this still renders the same invalid Trino SQL from #5871:

from sqlglot import parse_one
from sqlmesh.core.macros import MacroEvaluator

sql = "SELECT @GENERATE_SURROGATE_KEY(a, b, hash_function := 'SHA256') AS k FROM foo"
print(MacroEvaluator(dialect="trino").transform(parse_one(sql, dialect="trino")).sql("trino"))

Output is still:

SELECT SHA256(CONCAT(...VARCHAR...)) AS k FROM foo

Trino expects sha256(varbinary), and for surrogate-key semantics we need a string key, so the expected shape is:

LOWER(TO_HEX(SHA256(TO_UTF8(CONCAT(...)))))

The reason is that, on the current sqlglot pin, exp.func("SHA256", ..., dialect="trino") already returns exp.SHA2, not exp.SHA2Digest, so the new elif isinstance(func, exp.SHA2Digest) branch is not reached for Trino/Presto.

To make this PR complete, please do one of these:

  1. Include the sqlglot bump that contains the Trino/Presto SHA2 rendering behavior, then add direct assertions for Trino and Presto SHA256/SHA512 output.
  2. Or implement a SQLMesh-side fallback for affected dialects so SHA256/SHA512 render as LOWER(TO_HEX(<digest>(TO_UTF8(...)))) under the current sqlglot pin.

Either way, the test should assert the actual reported fix, for example that Trino @GENERATE_SURROGATE_KEY(..., hash_function := 'SHA256') renders LOWER(TO_HEX(SHA256(TO_UTF8(...)))), not only that BigQuery converts SHA2Digest to SHA2.

@mday-io mday-io self-requested a review July 9, 2026 06:29
@Pawansingh3889

Pawansingh3889 commented Jul 15, 2026

Copy link
Copy Markdown
Author

Yes, still on it. Going with your option 2: a Presto/Trino-side fallback in the macro so SHA256/SHA512 render as LOWER(TO_HEX(SHA256(TO_UTF8(...)))) under the current sqlglot pin. The fallback is gated by a probe (render a throwaway exp.SHA2 for the target dialect and check for TO_HEX), so once a sqlglot release containing tobymao/sqlglot#7824 lands and the pin moves, the macro defers to the native rendering and never double-wraps. Tests will assert the exact Trino and Presto SHA256/SHA512 output strings as you specified.

Bare SHA256(varchar) is a type error on Trino and binary semantics on
Presto, so the macro now builds LOWER(TO_HEX(SHA256(TO_UTF8(...))))
itself for the Presto family, mirroring those generators' MD5 handling.
A cached probe checks whether the dialect already renders exp.SHA2 in
the hex form, so once the sqlglot pin includes tobymao/sqlglot#7824 the
macro defers to the native rendering and never wraps twice. Adds direct
Trino and Presto SHA256/SHA512 output assertions that hold on both
sides of the pin bump.

Signed-off-by: Pawan Singh Kapkoti <42340841+Pawansingh3889@users.noreply.github.com>
@Pawansingh3889

Copy link
Copy Markdown
Author

Pushed. Your repro on this branch now renders:

SELECT LOWER(TO_HEX(SHA256(TO_UTF8(CONCAT(CAST(COALESCE(CAST(a AS VARCHAR), '_sqlmesh_surrogate_key_null_') AS VARCHAR), CAST('|' AS VARCHAR), CAST(COALESCE(CAST(b AS VARCHAR), '_sqlmesh_surrogate_key_null_') AS VARCHAR)))))) AS k FROM foo

The macro builds the hex-string form itself for the Presto family under the current pin, mirroring those generators' MD5 handling. The fallback is gated by a cached probe (_sha2_renders_binary) that checks whether the dialect already renders exp.SHA2 with TO_HEX, so once the pin moves past tobymao/sqlglot#7824 the macro defers to the native rendering and nothing double-wraps. The test now asserts the exact Trino and Presto SHA256/SHA512 output strings as requested, plus that the fallback stays off for duckdb/bigquery/snowflake; those assertions hold unchanged on both sides of the pin bump. 139 macro tests pass, ruff check and format clean.

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.

BUG [TRINO]: @generate_surrogate_key macro does not work with SHA256 on trino

2 participants