757 sql reformatting finetune sqlfluff

[temmey]

closes #757

Basics

• I added a line to /doc/CHANGELOG.md
• The PR is rebased with current master.
• Details of what you changed are in commit messages.
• References to issues, e.g. close #X, are in the commit messages.
• The buildserver is happy.

Checklist

• I have installed and I am using pre-commit hooks
• I fully described what my PR does in the documentation
  (not in the PR description)
• I fixed all affected documentation
• I fixed the introduction tour
• I wrote migrations in a way that they are compatible with already present data
• I fixed all affected decisions
• I added unit tests for my code
• I added code comments, logging, and assertions as appropriate
• I translated all strings visible to the user
• I mentioned every code or binary not directly written or done by me in reuse syntax
• I created left-over issues for things that are still to be done
• Code is conforming to our Architecture
• Code is conforming to our Guidelines
  (exceptions are documented)
• Code is consistent to our Design Decisions

Review

• I've tested the code
• I've read through the whole code
• Examples are well chosen and understandable

[markus2330]

This is not according to current database guidelines, we wrote: "use snake_case for everything except database keywords, which should be UPPERCASE". So it should be privacy_option. Isn't this supported by sqlfluff?

Btw. even for INTEGER it is questionable if it is a keyword, according to https://www.postgresql.org/docs/current/sql-keywords-appendix.html INTEGER is non-reserved (cannot be function or type).

[temmey]

This is not according to current database guidelines, we wrote: "use snake_case for everything except database keywords, which should be UPPERCASE". So it should be privacy_option. Isn't this supported by sqlfluff?

No, sadly it's not, but I applied it for now by manually changing the rules.
https://docs.sqlfluff.com/en/stable/rules.html#rule-capitalisation.types

Btw. even for INTEGER it is questionable if it is a keyword, according to https://www.postgresql.org/docs/current/sql-keywords-appendix.html INTEGER is non-reserved (cannot be function or type).

I could make it snake_case, but since it's a standard keyword, I would keep it uppercase.

[markus2330]

@4ydan what do you think about running sqlfluff twice with different configs? Is this even possible to have in pre-commit? (In CI it would work.)

@temmey can you create an upstream bug report, maybe there is some way we didn't see. Please link from here when you created the issue.

[temmey]

@4ydan what do you think about running sqlfluff twice with different configs? Is this even possible to have in pre-commit? (In CI it would work.)

We can solve it by creating a new entry into .pre-commit-config.yaml and copying the config .sqlfluff. That way, it will generate errors though, which will be fixed right after.

[temmey]

@temmey can you create an upstream bug report, maybe there is some way we didn't see. Please link from here when you created the issue.

sqlfluff/sqlfluff#5049

[markus2330]

sqlfluff/sqlfluff#5049

Great job, let us see what they say, maybe we can go the direct way without workarounds in our pre-commit/CI configs.

[4ydan]

@4ydan what do you think about running sqlfluff twice with different configs? Is this even possible to have in pre-commit? (In CI it would work.)

Yes as temmy said, we could write a second sqlfluff hook with a different id and make sure they dont run over the same files.

[4ydan]

Pre-commit looks good now, but isnt it possible to just have two different hooks in there instead of manuall shifting something around in the config file?

[temmey]

If we want to do it like that, I will write a commit for it. Manual shifting is a bad solution.

[badnames]

Good work! I just noticed a few minor styling and grammar mistakes.

[4ydan]

Isnt this now doing exactly what we want in one config file?
This is at least what I would expect the .sql files to look after reading the guidelines, if this is still wrong, we need to update the guidelines.

[markus2330]

Great job, finally done! I am happy with these a bit relaxed rules. The tool also shouldn't get in the way too much.