Skip to content

Fix: bug in model creation related to metadata-only statements#4836

Merged
georgesittas merged 1 commit intomainfrom
jo/fix_gen_issue
Jul 1, 2025
Merged

Fix: bug in model creation related to metadata-only statements#4836
georgesittas merged 1 commit intomainfrom
jo/fix_gen_issue

Conversation

@georgesittas
Copy link
Copy Markdown
Collaborator

@georgesittas georgesittas commented Jun 27, 2025

The statements list can contain pairs of expressions & booleans, which we didn't take into account in the line I changed. It's a very small change targeting an edge case so I didn't feel like it was worth spending much time testing it. It probably wouldn't have crashed anyway, see gen source here.

@georgesittas georgesittas requested a review from a team June 27, 2025 10:44

jinja_macro_references, used_variables = extract_macro_references_and_variables(
*(gen(e) for e in statements)
*(gen(e if isinstance(e, exp.Expression) else e[0]) for e in statements)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure this is a non-breaking change?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be fine, I think. These generated strings are used for extracting macro & variable references. If I'm not mistaken, we'd previously produce "(True, <expression SQL>)" due to reaching this section. After this PR we'll only search <expression SQL>.

@georgesittas georgesittas merged commit 84d5341 into main Jul 1, 2025
26 checks passed
@georgesittas georgesittas deleted the jo/fix_gen_issue branch July 1, 2025 08:07
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.

2 participants