search

onemoredata / bagger

Massive log storage in PostgreSQL
BSD 2-Clause "Simplified" License
9 stars 0 forks source link

Makefile.PL: add sqlfluff as linter and formatter #108

Closed 65278 closed 7 months ago

65278 commented 7 months ago

I've added custom make targets to Makefile.pl. These are sql-lint and sql-fmt, both calling sqlfluff with .sqlfluff as a config.

See this PR here to get an ebuild: https://github.com/onemoredata/bagger-overlay/pull/2

The only problem I've encountered is that function definitions need to be reworked to use dollar quoted strings (and this seems to break PGObject, as far I can see). I will have to look for a workaround here. We may discuss linting specifics after we agreed to this.

einhverfr commented 7 months ago

So there are a number of reasons not to rework the functions to use dollar sign quoting. The BEGIN ATOMIC syntax actually has a number of real advantages we probably want to use. These include the fact that quoted-string functions are stored as strings and then parsed etc when run. The current functions are stored as ASTs with proper references to tables by OID.

This means also there's better dependency tracking, fewer concerns about injection attacks (since there is no searching for tables or functions etc at invocation time) etc.

I am fine with this being added, but I think rather than reworking the functions we should work with SQLFluff to fix their linter.

einhverfr commented 7 months ago

Actually.... no need to work with them to fix this. A PR was merged 3 weeks ago.

https://github.com/sqlfluff/sqlfluff/issues/5310

65278 commented 7 months ago

Already packaged 2.3.5 with that fix. I now only need to fix function creation without dollar qouting ...

65278 commented 7 months ago

I've added a 9999 version so anybody interested can try the linter out. I'm not quite satisfied with the formatter results on indentation, I think that requires more work. I've committed them as an example anyway.

einhverfr commented 7 months ago

yeah I think we can worry about the formatting later. Having the linting is a good step.