sqlfmt icon indicating copy to clipboard operation
sqlfmt copied to clipboard

sqlfmt breaks quoted jinja expressions with nested single quotes

Open luke-bassett opened this issue 2 years ago • 3 comments

Description In the scenario where a jinja block is enclosed in single quotes, and contains single quotes, a space is added inside the single quotes which causes an error. Perhaps this isn't a bug, but the only way to deal with it now is to manually edit all .sql files to change the quotation type inside jinja blocks from ' to ".

To Reproduce Line which causes issue:

where to_date(export_date::varchar) = '{{ var('dst_exp_date') }}'

Expected behavior Expected formatting is either this:

where to_date(export_date::varchar) = '{{ var("dst_exp_date") }}'

or this (unchanged):

where to_date(export_date::varchar) = '{{ var('dst_exp_date') }}'

Actual behavior Reformatted to

where to_date(export_date::varchar) = '{{ var(' dst_exp_date ') }}'

but this causes an error when running dbt.

Additional context version 0.9.0

luke-bassett avatar Jul 18 '22 22:07 luke-bassett

Thanks for the report!

We're parsing this as two strings and a name (the first string is '{{ var(') instead of a jinja expression inside a string.

I'm surprised this is valid dbt jinja, but I guess the templater goes first and only operates on the curlies (it doesn't know or care the curlies are quoted).

I need to think about how we can support this with our parser.

tconbeer avatar Jul 18 '22 22:07 tconbeer

Yeah, it is a bit of a weird/sloppy scenario, and I'm definitely planning to change it to use double quotes inside the curlies, but I figured I'd submit it since it is a scenario where running sqlfmt causes valid dbt to become invalid.

luke-bassett avatar Jul 18 '22 23:07 luke-bassett

I was all caught up on the collision with other valid sql syntax, like this statement that should select two strings in columns foo and bar:

select '{{ my_macro(' foo, ') }}' bar

But dbt fails with 'my_macro' is undefined. So I think I'm okay implementing this the easy way, and including the ' as part of the jinja bracket. There are some complications with bracket matching and jinja formatting that we'll have to handle.

tconbeer avatar Dec 19 '22 20:12 tconbeer