Skip to content

Conversation

blarghmatey
Copy link
Member

What are the relevant tickets?

N/A

Description (What does it do?)

Adds sqlfmt as a pre-commit check and applies formatting to our dbt code.

How can this be tested?

N/A

sqlfmt is a tool similar to Ruff and Black that auto-formats SQL, including Jinja
templated SQL for dbt, with strong opinions. This adds this tool to our pre-commit
checks to keep our SQL consistently formatted.
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces sqlfmt as an automated pre-commit check and applies comprehensive formatting changes to SQL files in the dbt codebase. The changes focus on improving SQL code consistency and readability by standardizing formatting conventions.

  • Adds sqlfmt for automated SQL code formatting during pre-commit
  • Applies consistent formatting to 100+ dbt SQL model files
  • Standardizes syntax patterns like quote usage, line breaks, and indentation

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Contributor

@rachellougee rachellougee left a comment

Choose a reason for hiding this comment

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

I am not sure that reformatting the entire dbt code with sqlfmt is a good idea-it overwrites a lot of existing work. I often rely on "git blame" to trace changes and find the linked issue, and a full reformat would make it a lot harder.

Also, I remember I ran into an issue with sqlfuff auto-formatted some SQL by replacing double quotes with single quotes. In that case, dbt ran fine without errors, but the updated column ended up being blank. I had to add exclude_rules = "CV10” to pyproject.tml, which now replaced with "convention.quoted_literals" . So I’m a bit on the fence when it comes to auto-fixes for SQL formatting.

I think maybe we should instead add sqlfmt to pre-commit hooks so that it only formats the new or updated code. That way, it’s up to the person making the change to review and confirm the formatting. What do you all think?

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