Fix(presto)!: preserve SHA256/SHA512 digest semantics, render SHA2 as hex string [CLAUDE]#7824
Open
Pawansingh3889 wants to merge 1 commit into
Open
Conversation
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.
Presto and Trino's native
SHA256/SHA512take and returnVARBINARY, but the parser mapped them toexp.SHA2, the string-to-hex-string expression that MySQL and Spark use. The codebase already models this split withexp.MD5andexp.MD5Digest, and the presto parser maps nativeMD5toMD5Digestone line above the SHA mappings, so this looked like an oversight rather than a decision.Two changes, both mirroring the MD5 precedent:
SHA256/SHA512toexp.SHA2Digest. Digests survive a round trip and transpile to other digest functions: TrinoSHA256(x)becomesUNHEX(SHA256(x))in DuckDB and staysSHA256(x)in BigQuery, instead of silently turning into a hex-string function.exp.SHA2the waymd5_sqlrendersexp.MD5:LOWER(TO_HEX(SHA256(TO_UTF8(x)))), with the UTF8 encode applied when the argument is typed as text. Spark'sSHA2(x, 256)now produces the same value on Trino that it produces on Spark. SHA224/SHA384 raise the usual unsupported warning since Presto has no equivalents.Updated the bigquery and exasol expectations that encoded the old equivalence. Exasol's
HASH_SHA256returns a hex string, soLOWER(TO_HEX(SHA256(x)))is the value-preserving translation there; its presto/trino read entries asserted that a digest function and a hex-string function were the same thing, which is the bug this fixes, so those two entries are removed.Found while chasing a surrogate key mismatch in SQLMesh on Trino (SQLMesh/sqlmesh#5871): MD5-based keys transpile correctly, SHA256-based keys silently change value.
One adjacent inconsistency deliberately left alone: the bigquery parser maps
SHA256toSHA2DigestbutSHA512toSHA2, while BigQuery returnsBYTESfor both. Happy to follow up separately if that is wanted.