Merge pull request juju#17449 from SimonRichardson/lint-sql-files

juju#17449

~~Requires juju#17428 to land first~~

----

This formats and lints all the SQL files. We're using sqlfluff for
linting and formatting. The style isn't 100% nice, but then again
any formatter isn't perfect (see gofmt as an example).

The files are 100% consistent though.

In addition to formatting, I found a lot of stupid and simple mistakes
that the linter found.

 - using keywords for column names
 - missing commas
 - ambiguous column names in joins
 - duplicated columns in views

I'm not even sure how some of the foreign keys worked (unless the sqlite
parser is very lenient) because missing commas were present in multiple
locations.

The point of this is to improve the correctness of our SQL
statements.

## Checklist

<!-- If an item is not applicable, use `~strikethrough~`. -->

- [x] Code style: imports ordered, good names, simple structure, etc
- [x] Comments saying why design decisions were made
- [x] Go unit tests, with comments saying what you're testing

## QA steps

Install sqlfluff.

Use an env rather than stomping on the system is probably wise.

```sh
$ pip install sqlfluff
```


## Links

**Jira card:** JUJU-

.github/.sqlfluff

@@ -0,0 +1,17 @@
+[sqlfluff]
+dialect = sqlite
+max_line_length = 180
+aliasing.column = explicit
+capitalisation.keywords = upper
+capitalisation.functions = upper
+capitalisation.literals = true
+convention.not_equal = consistent
+
+[sqlfluff:indentation]
+tab_space_size = 4
+indent_unit = space
+indented_joins = false
+indented_using_on = true
+allow_implicit_indents = true
+indented_on_contents = false
+indented_ctes = false

.github/workflows/static-analysis.yml

@@ -142,3 +142,22 @@ jobs:
       run: |
         STATIC_ANALYSIS_JOB=test_schema make static-analysis
       shell: bash
+
+  sql:
+    name: SQL
+    runs-on: [self-hosted, linux, x64, aws, large]
+    if: github.event.pull_request.draft == false
+    steps:
+    - name: Checkout
+      uses: actions/checkout@v3
+
+    - name: Set up Python
+      uses: "actions/setup-python@v2"
+      with:
+          python-version: "3.9"
+
+    - name: Install SQLFluff
+      run: "pip install sqlfluff==3.0.7"
+
+    - name: Lint SQL
+      run: "sqlfluff lint --config .github/.sqlfluff domain/schema/**/sql/*.sql"

domain/schema/controller/sql/0001-life.sql

@@ -1,9 +1,9 @@
 CREATE TABLE life (
-    id    INT PRIMARY KEY,
+    id INT PRIMARY KEY,
     value TEXT NOT NULL
 );
 
 INSERT INTO life VALUES
-    (0, 'alive'), 
-    (1, 'dying'),
-    (2, 'dead');
+(0, 'alive'),
+(1, 'dying'),
+(2, 'dead');

domain/schema/controller/sql/0002-lease.sql

@@ -1,26 +1,26 @@
 CREATE TABLE lease_type (
-    id   INT PRIMARY KEY,
+    id INT PRIMARY KEY,
     type TEXT
 );
 
 CREATE UNIQUE INDEX idx_lease_type_type
 ON lease_type (type);
 
 INSERT INTO lease_type VALUES
-    (0, 'singular-controller'),    -- The controller running singular controller/model workers.
-    (1, 'application-leadership'); -- The unit that holds leadership for an application.
+(0, 'singular-controller'),    -- The controller running singular controller/model workers.
+(1, 'application-leadership'); -- The unit that holds leadership for an application.
 
 CREATE TABLE lease (
-    uuid            TEXT PRIMARY KEY,
-    lease_type_id   INT NOT NULL,
-    model_uuid      TEXT,
-    name            TEXT,
-    holder          TEXT,
-    start           TIMESTAMP,
-    expiry          TIMESTAMP,
-    CONSTRAINT      fk_lease_lease_type
-        FOREIGN KEY (lease_type_id)
-        REFERENCES  lease_type(id)
+    uuid TEXT PRIMARY KEY,
+    lease_type_id INT NOT NULL,
+    model_uuid TEXT,
+    name TEXT,
+    holder TEXT,
+    start TIMESTAMP,
+    expiry TIMESTAMP,
+    CONSTRAINT fk_lease_lease_type
+    FOREIGN KEY (lease_type_id)
+    REFERENCES lease_type (id)
 );
 
 CREATE UNIQUE INDEX idx_lease_model_type_name
@@ -32,12 +32,12 @@ ON lease (expiry);
 CREATE TABLE lease_pin (
     -- The presence of entries in this table for a particular lease_uuid
     -- implies that the lease in question is pinned and cannot expire.
-    uuid       TEXT PRIMARY KEY,
+    uuid TEXT PRIMARY KEY,
     lease_uuid TEXT,
-    entity_id  TEXT,
-    CONSTRAINT      fk_lease_pin_lease
-        FOREIGN KEY (lease_uuid)
-        REFERENCES  lease(uuid)
+    entity_id TEXT,
+    CONSTRAINT fk_lease_pin_lease
+    FOREIGN KEY (lease_uuid)
+    REFERENCES lease (uuid)
 );
 
 CREATE UNIQUE INDEX idx_lease_pin_lease_entity

domain/schema/controller/sql/0003-changelog.sql

@@ -1,5 +1,5 @@
 CREATE TABLE change_log_edit_type (
-    id        INT PRIMARY KEY,
+    id INT PRIMARY KEY,
     edit_type TEXT
 );
 
@@ -9,31 +9,31 @@ ON change_log_edit_type (edit_type);
 -- The change log type values are bitmasks, so that multiple types can be
 -- expressed when looking for changes.
 INSERT INTO change_log_edit_type VALUES
-    (1, 'create'),
-    (2, 'update'),
-    (4, 'delete');
+(1, 'create'),
+(2, 'update'),
+(4, 'delete');
 
 CREATE TABLE change_log_namespace (
-    id          INT PRIMARY KEY,
-    namespace   TEXT,
+    id INT PRIMARY KEY,
+    namespace TEXT,
     description TEXT
 );
 
 CREATE UNIQUE INDEX idx_change_log_namespace_namespace
 ON change_log_namespace (namespace);
 
 CREATE TABLE change_log (
-    id                  INTEGER PRIMARY KEY AUTOINCREMENT,
-    edit_type_id        INT NOT NULL,
-    namespace_id        INT NOT NULL,
-    changed             TEXT NOT NULL,
-    created_at          DATETIME NOT NULL DEFAULT(STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW', 'utc')),
-    CONSTRAINT          fk_change_log_edit_type
-            FOREIGN KEY (edit_type_id)
-            REFERENCES  change_log_edit_type(id),
-    CONSTRAINT          fk_change_log_namespace
-            FOREIGN KEY (namespace_id)
-            REFERENCES  change_log_namespace(id)
+    id INTEGER PRIMARY KEY AUTOINCREMENT,
+    edit_type_id INT NOT NULL,
+    namespace_id INT NOT NULL,
+    changed TEXT NOT NULL,
+    created_at DATETIME NOT NULL DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW', 'utc')),
+    CONSTRAINT fk_change_log_edit_type
+    FOREIGN KEY (edit_type_id)
+    REFERENCES change_log_edit_type (id),
+    CONSTRAINT fk_change_log_namespace
+    FOREIGN KEY (namespace_id)
+    REFERENCES change_log_namespace (id)
 );
 
 -- The change log witness table is used to track which nodes have seen
@@ -42,8 +42,8 @@ CREATE TABLE change_log (
 -- We'll delete all change log entries that are older than the lower_bound
 -- change log entry that has been seen by all controllers.
 CREATE TABLE change_log_witness (
-    controller_id       TEXT PRIMARY KEY,
-    lower_bound         INT NOT NULL DEFAULT(-1),
-    upper_bound         INT NOT NULL DEFAULT(-1),
-    updated_at          DATETIME NOT NULL DEFAULT(STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW', 'utc'))
+    controller_id TEXT PRIMARY KEY,
+    lower_bound INT NOT NULL DEFAULT (-1),
+    upper_bound INT NOT NULL DEFAULT (-1),
+    updated_at DATETIME NOT NULL DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW', 'utc'))
 );

domain/schema/controller/sql/0004-changelog-namespaces.sql

@@ -1,14 +1,14 @@
 INSERT INTO change_log_namespace VALUES
-    (0, 'external_controller', 'external controller changes based on the UUID'),
-    (1, 'controller_node', 'controller node changes based on the controller ID'),
-    (2, 'controller_config', 'controller config changes based on the key'),
-    (3, 'model_migration_status', 'model migration status changes based on the UUID'),
-    (4, 'model_migration_minion_sync', 'model migration minion sync changes based on the UUID'),
-    (5, 'upgrade_info', 'upgrade info changes based on the UUID'),
-    (6, 'cloud', 'cloud changes based on the UUID'),
-    (7, 'cloud_credential', 'cloud credential changes based on the UUID'),
-    (8, 'autocert_cache', 'autocert cache changes based on the UUID'),
-    (9, 'upgrade_info_controller_node', 'upgrade info controller node changes based on the upgrade info UUID'),
-    (10, 'object_store_metadata_path', 'object store metadata path changes based on the path'),
-    (11, 'secret_backend_rotation', 'secret backend rotation changes based on the backend UUID and next rotation time'),
-    (12, 'model', 'model changes based on the model UUID');
+(0, 'external_controller', 'external controller changes based on the UUID'),
+(1, 'controller_node', 'controller node changes based on the controller ID'),
+(2, 'controller_config', 'controller config changes based on the key'),
+(3, 'model_migration_status', 'model migration status changes based on the UUID'),
+(4, 'model_migration_minion_sync', 'model migration minion sync changes based on the UUID'),
+(5, 'upgrade_info', 'upgrade info changes based on the UUID'),
+(6, 'cloud', 'cloud changes based on the UUID'),
+(7, 'cloud_credential', 'cloud credential changes based on the UUID'),
+(8, 'autocert_cache', 'autocert cache changes based on the UUID'),
+(9, 'upgrade_info_controller_node', 'upgrade info controller node changes based on the upgrade info UUID'),
+(10, 'object_store_metadata_path', 'object store metadata path changes based on the path'),
+(11, 'secret_backend_rotation', 'secret backend rotation changes based on the backend UUID and next rotation time'),
+(12, 'model', 'model changes based on the model UUID');