Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

missing space around union all when formatting dbt_utils.unpivot #162

Closed
rcaddell opened this issue Apr 19, 2022 · 3 comments · Fixed by #173
Closed

missing space around union all when formatting dbt_utils.unpivot #162

rcaddell opened this issue Apr 19, 2022 · 3 comments · Fixed by #173
Labels
bug Something isn't working
Milestone

Comments

@rcaddell
Copy link

Describe the bug
When using the dbt_utils.unpivot SQL Generator after sqlfmt has run the compiled SQL is missing spaces around the union all between select statements. This does not happen if the statement is not put through sqlfmt first. A working format for the unpivot generator is provided under additional context
To Reproduce
sqlfmt . && dbt build -s <model using unpivot>

Expected behavior
When complied each union all should be separated:

WITH unpivoted AS (
    

    select

      cast('ID' as 
    varchar
) as column_name,
      cast(  
           ID
             
           as decimal) as percentage

    from <database>.<schema>.<model>

    union all
    select

      cast('USERNAME' as 
    varchar
) as column_name,
      cast(  
           USERNAME
             
           as decimal) as percentage

    from <database>.<schema>.<model>

    union all
    select

      cast('LAST_NAME' as 
    varchar
) as column_name,
      cast(  
           LAST_NAME
             
           as decimal) as percentage

    from <database>.<schema>.<model>

    union all
    select ...

Actual behavior

with
    unpivoted as (
        

select

    cast('ID' as  varchar) as column_name,
    cast(
         ID
         as decimal
    ) as percentage

from <database>.<schema>.<model>union allselect

    cast('USERNAME' as  varchar) as column_name,
    cast(
         USERNAME
         as decimal
    ) as percentage

from <database>.<schema>.<model>union allselect

    cast('LAST_NAME' as  varchar) as column_name,
    cast(
         LAST_NAME
         as decimal
    ) as percentage

from <database>.<schema>.<model>union allselect...

Additional context
What is the output of sqlfmt --version?
sqlfmt, version 0.6.0

Working format

 WITH unpivoted AS (
     {{ dbt_utils.unpivot(
         relation = <model_name>,
         cast_to = 'decimal',
         field_name = 'column_name',
         value_name = 'percentage'
     ) }}
 )
 SELECT
     *
 FROM
     unpivoted
@tconbeer
Copy link
Owner

Thanks for the report, @rcaddell !

I think what is happening is that sqlfmt is formatting the vendored dbt-utils code inside your project. These lines look problematic to me:

https://github.com/dbt-labs/dbt-utils/blob/33299334a305d0acb99ebee8cc2eb6eb2ba5ca31/macros/sql/unpivot.sql#L72-L76

sqlfmt is one-lining those lines into

from {{ relation }}{% if not loop.last -%} union all{% endif -%}

but because of the jinja whitespace control, the space before union gets wiped out when the template is rendered.

I think the fix here is to be more conservative about destroying whitespace around jinja tags. (A single space before {% if ... would fix this).

In the meantime, a workaround for you:

  1. run dbt clean to delete the vendored/formatted dbt_utils code that isn't working
  2. run dbt deps to re-install dbt_utils
  3. run sqlfmt models macros instead of sqlfmt . -- this will make sure we don't touch the code in the dbt_packages directory

The next release will include a command-line option to exclude a directory #131 , and the option to configure sqlfmt using a pyproject.toml file (so you could set the excluded dirs there) #90 , which would make this workaround easier, but I haven't quite gotten around to the exclude logic yet (PRs welcome!)

@tconbeer tconbeer added the bug Something isn't working label Apr 19, 2022
@rcaddell
Copy link
Author

thanks @tconbeer, that works

@tconbeer
Copy link
Owner

tconbeer commented May 4, 2022

this patch has been released in v0.8.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants