-
Notifications
You must be signed in to change notification settings - Fork 4
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
Reconfigure sqlfluff linter and pre-commit hooks #456
Comments
Note that SQLFluff just did a compatibility release for dbt 1.8: https:/sqlfluff/sqlfluff/releases/tag/3.0.7 |
I did a bunch of research on this today, including test driving the dbt templater and trying out a few different sets of configs, and unfortunately my conclusion is that sqlfluff does not support linting macro blocks in isolation. Since it's not supported using either the dbt templater or the jinja templater, and since the dbt templater is slightly slower, I think we should stick to the jinja templater for now. It's not really documented anywhere, but it appears that the sqlfluff policy is to not lint macros because they are only fragments and hence may not be valid SQL. Macros do not get linted in isolation, but instead are linted when they are called by a non-macro SQL statement (like in a model), at which point the templater uses the macro to generate a full SQL statement and then validate it. This policy is particularly apparent when using the dbt templater, which explicitly skips macro files -- here's a sample log line when running
This SO answer suggests using the dbt templater for models and the jinja templater for macros, but that doesn't work because the jinja templater will skip anything inside a I couldn't find any discussion on this particular issue, but I did find some discussion and planned work around the related issue of linting unexecuted portions of jinja templates. It's possible this may eventually lead to support for linting macros in isolation, but it's hard to tell based on the current discussion, which is very weedsy. The fact that there's no discussion outside of the SO answer I linked leaves me less than confident that I've exhausted all options, however, so it's possible I've just missed something. Note that I timed the dbt templater vs the jinja templater locally, and found that the docs are correct in that the dbt templater is slower. On my machine, linting the whole I also combed through the sqlfluff releases between 2.1.1 and 3.0.7 and didn't find anything particularly relevant to us. Nevertheless, I put up #478 to upgrade to the most recent version to make sure we stay up to date. |
Reopening in case Dan would like to continue pursuing this line of inquiry. |
Thanks @jeancochrane, this looks great to me and aligns with my super preliminary poking around last week. Let's keep this open as an issue, as it does feel like something that could eventually be resolved by one of the upstream projects. |
It's been awhile since we've touched the SQLFluff/sqlfmt configuration we use for CI/CD. SQLFluff has been updated a lot in the last year or so. We should take a quick look at our configuration and make sure it's still doing everything we want/expect.
Specifically, I don't think the SQLFluff linting in the
macros/
ortests/
directories is working quite right. It doesn't lint locally for me within macro blocks. Perhaps it's time for us to retry the dbt templater.The text was updated successfully, but these errors were encountered: