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

Unstable formatting with chained jinja expressions and operators #175

Closed
tconbeer opened this issue May 5, 2022 · 0 comments · Fixed by #216
Closed

Unstable formatting with chained jinja expressions and operators #175

tconbeer opened this issue May 5, 2022 · 0 comments · Fixed by #216
Labels
bug Something isn't working
Milestone

Comments

@tconbeer
Copy link
Owner

tconbeer commented May 5, 2022

Describe the bug
sqlfmt should always produce the same output on a given query, even when run multiple times. When this is not the case, we call this "unstable formatting".

There is an interaction between jinja expressions (and jinjafmt) and operators that is causing unstable formatting.

To Reproduce
This file in the gitlab project has already been formatted by 0.8.0.

Run it through sqlfmt --diff

Expected behavior
sqlfmt should no-op

Actual behavior

--- source_query
+++ formatted_query
@@ -29,25 +29,38 @@
         as dim_sales_qualified_source_id,
         {{ get_keyed_nulls("order_type.dim_order_type_id") }} as dim_order_type_id
     from {{ ref("sheetload_sales_funnel_targets_matrix_source") }}
-    left join date on {{
+    left join
+        date
+        on
+        {{
             sales_funnel_text_slugify(
                 "sheetload_sales_funnel_targets_matrix_source.month"
             )
-        }} = {{ sales_funnel_text_slugify("date.fiscal_month_name_fy") }}
-    left join sales_qualified_source on {{
+        }}
+        = {{ sales_funnel_text_slugify("date.fiscal_month_name_fy") }}
+    left join
+        sales_qualified_source
+        on
+        {{
             sales_funnel_text_slugify(
                 "sheetload_sales_funnel_targets_matrix_source.opportunity_source"
             )
-        }} = {{
+        }}
+        =
+        {{
             sales_funnel_text_slugify(
                 "sales_qualified_source.sales_qualified_source_name"
             )
         }}
-    left join order_type on {{
+    left join
+        order_type
+        on
+        {{
             sales_funnel_text_slugify(
                 "sheetload_sales_funnel_targets_matrix_source.order_type"
             )
-        }} = {{ sales_funnel_text_slugify("order_type.order_type_name") }}
+        }}
+        = {{ sales_funnel_text_slugify("order_type.order_type_name") }}

 ),
 fy22_user_hierarchy as (
@@ -94,11 +107,9 @@
     from target_matrix
     left join
         fy22_user_hierarchy
-        on {{ sales_funnel_text_slugify("target_matrix.area") }} = {{
-            sales_funnel_text_slugify(
-                "fy22_user_hierarchy.crm_opp_owner_area_stamped"
-            )
-        }}
+        on {{ sales_funnel_text_slugify("target_matrix.area") }}
+        =
+        {{ sales_funnel_text_slugify("fy22_user_hierarchy.crm_opp_owner_area_stamped") }}
     where target_matrix.fiscal_year = 2022

     union all
@@ -126,7 +137,8 @@
     left join
         fy23_and_beyond_user_hierarchy
         on {{ sales_funnel_text_slugify("target_matrix.area") }}
-        = {{
+        =
+        {{
             sales_funnel_text_slugify(
                 "fy23_and_beyond_user_hierarchy.crm_opp_owner_sales_segment_geo_region_area_stamped"
             )

Additional context
What is the output of sqlfmt --version?
0.8.0

@tconbeer tconbeer added the bug Something isn't working label May 5, 2022
@tconbeer tconbeer added this to the v0.9.0 milestone May 5, 2022
@tconbeer tconbeer modified the milestones: v0.9.0, v0.10.0 Jun 2, 2022
tconbeer added a commit that referenced this issue Jul 21, 2022
* fix: removing caching of node, line, comment props, which can cause issues when objects are mutated by jinjaformatter

* fix #175: add extra split before multiline jinja nodes

* fix: allow more merging of multiline jinja nodes

* chore: bump primer refs
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.

1 participant