Page MenuHomePhabricator

Standardise SQL formatting and lint .sql files at CI
Closed, ResolvedPublic

Description

The SQL files are all over the place, making it hard to contribute to them.

It is also super stressful to edit them as nothing is checking them for correctness.

Event Timeline

@Lokal_Profil, I have found this one too: https://github.com/cweiske/php-sqllint.

As it is packaged on packagist we could also run it via composer at CI.

(Although it seems to be broken − on the one hand, git clone + composer install gives me a working CLI ; but adding a composer.json with it as a dependency I get a PHP error...

@hashar, would you have any recommendations for us?

@Lokal_Profil, I have found this one too: https://github.com/cweiske/php-sqllint.

As it is packaged on packagist we could also run it via composer at CI.

(Although it seems to be broken − on the one hand, git clone + composer install gives me a working CLI ; but adding a composer.json with it as a dependency I get a PHP error...

Reported and already fixed upstream :)

@Lokal_Profil, I have found this one too: https://github.com/cweiske/php-sqllint.

As it is packaged on packagist we could also run it via composer at CI.

Unless @hashar has any other recommendations I would run with that one.

In the long run we probably want to get rid of many of these sql files (i.e. build them from simple to understand configs). Especially the monster sized one gives me nightmares whenever I'm opening it.

@Lokal_Profil, I have found this one too: https://github.com/cweiske/php-sqllint.

As it is packaged on packagist we could also run it via composer at CI.

Unless @hashar has any other recommendations I would run with that one.

Unfortunately, the underlying SQL parser does not support the syntax REPLACE INTO X SELECT a, b FROM Y, which we use massively...

In the long run we probably want to get rid of many of these sql files (i.e. build them from simple to understand configs). Especially the monster sized one gives me nightmares whenever I'm opening it.

Indeed. :)

Change 315226 had a related patch set uploaded (by Jean-Frédéric):
Lint SQL files using php-sqllint

https://gerrit.wikimedia.org/r/315226

Change 315226 merged by jenkins-bot:
Lint SQL files using php-sqllint

https://gerrit.wikimedia.org/r/315226

Awesome! I have filled T153842 to get HTTPS support for the composer dependency cweiske/php-sqllint.