Fix!: serialize blueprint variables separately to leverage AST#4061
Merged
georgesittas merged 2 commits intomainfrom Apr 3, 2025
Merged
Fix!: serialize blueprint variables separately to leverage AST#4061georgesittas merged 2 commits intomainfrom
georgesittas merged 2 commits intomainfrom
Conversation
tobymao
approved these changes
Mar 31, 2025
417eb37 to
c516498
Compare
izeigerman
reviewed
Mar 31, 2025
3813e17 to
257c1c8
Compare
257c1c8 to
dcaa576
Compare
02bf7fe to
ba6b042
Compare
ba6b042 to
a91f4ae
Compare
afzaljasani
pushed a commit
that referenced
this pull request
Apr 8, 2025
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 #4017
Given a model like this:
MODEL ( name m.@{foo}, blueprints ( (foo := "s p a c e s"), (foo := 'c'), ), ); SELECT @foo AS colThe blueprint variables that contain quotes are incorrectly rendered today:
This PR addresses this bug with the help of this commit, by changing the representation of the blueprint values: their values are now the parsed ASTs, and not the corresponding SQL strings. This behavior is more intuitive, because for example quoted identifiers will be correctly interpolated in contexts such as model names, without dropping the quotes.
Since SQLGlot expressions aren't serialized today, making this fix wasn't as simple as pushing the blueprint variables to the
SQLMESH_VARSdict, since that only contains Python literals that can beeval'd on the fly without additional context, when hydrating the Python environment.Additionally, we need to serialize blueprint values because if, e.g., a model query references them, then the raw SQL with the variable reference is serialized and we need to have the variable's payload at deserialization time to ensure the query can be properly rendered.
This led to a new dictionary in the python env. that will contain only the blueprint variables, so that we can unambiguously serialize them as strings that are produced by
sql(dialect=...)and then deserialize them in the macro evaluator by parsing with the same dialect.