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

Updating sql-formatter will resolve several formatting issues #1489

Closed
pokutuna opened this issue May 28, 2023 · 0 comments · Fixed by #1490
Closed

Updating sql-formatter will resolve several formatting issues #1489

pokutuna opened this issue May 28, 2023 · 0 comments · Fixed by #1490

Comments

@pokutuna
Copy link
Contributor

Would you like to update sql-formatter?

Currently, dataform CLI depends on version ^2.3.3, which is about 4 years old.
The latest version is 12.2.1.

By updating, we can resolve the following issues related to formatting:

Handling dialects in sql-formatter

The current version of sql-formatter have a feature for "dialects" specific to different database products.
Now sql-formatter covers all SQL dialects supported warehouse by Dataform.
https://github.com/sql-formatter-org/sql-formatter/blob/master/docs/language.md

By passing a language value corresponding to the warehouse setting in Dataform to the formatter, we can expect more appropriate formatting.
But if we use the neutral default sql that means StandardSQL, formatting for UNNEST and QUALIFY will not work well, and I think it need to refer to the warehouse value in dataform.json.

I have also confirmed that the formatting result changes in some cases. The files under examples/formatter/ seems to assume BigQuery, so if we format as BigQuery,

  • The space after IF, IFNULL disappears
  • A space is added after UNNEST
  • A line break is added after ALTER TABLE {name}

While sql-formatter updating brings differences in the formatting results, QUALIFY is a frequently used clause, and named arguments are broken by current formatter. The update makes the format command practical.

Could you take a look at the pull request I'm going to send here? You may use only some of the commits.

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 a pull request may close this issue.

1 participant