new cmd: `supabase db lint`

[Marviel]

Feature request

I'm writing this spec to gauge interest in a PR I'm going to put together that wraps up a script we use internally to type-check the functions in our database.

TL;DR

• Runtime exceptions should be avoided.
• Type safety at the database level is critical for avoiding runtime exceptions, b/c the database should be a source of truth and clarity
• Functions in Postgres are effectively not - Type Safe at "compile time" / "commit time" by default
• Tools like plpgsql_check exist to type-check Postgres functions to a relatively high degree of accuracy, however, while robust, they are not as user-friendly.
• It is impractical to force "on commit" type checking (a-la passive mode) (probably? for speed reasons?)
• Because it cannot be done "on commit", and this data is imho critical to having a consistent database -> it should be made easier for users to type-check their schema, at any time they choose.
• Therefore, there should exist a supabase db lint command (exact naming up for grabs) which is inspired by the eslint / tslint ecosystem, and follows the following rough spec:
  • [x] supabase db lint should check the existing database (after migrations have been run) to identify any typing errors, warnings, and notes, at different log levels, as available on the console environment they are running. (Can be achieved via the plpgsql_check Postgres extension, as described above -- or by other, similar projects)
  • [x] Users should be able to specify what level of lint errors they would like to be output, via a flag.
  • [ ] Another flag should exist, to allow the user to optionally drop and recreate their local database at the same time -> this would be helpful as a "pre-PR" smoke test, so one could validate that new migrations haven't caused any issues.
• Other Nice-To-Haves / Future Work
  • [ ] Users should be able to easily write their own custom linters for both their migrations, and their built schemas, in whatever language is found most suitable for this purpose.

Longform

Runtime exceptions should be avoided whenever possible.

One cause of Runtime Exceptions is calling a function (procedure, etc) "in the wrong way" -- i.e. with the wrong parameter count, or with the wrong parameter types.

Typed languages (~Typescript, Rust, C#, et. al.) afford the compiler the ability to check ahead of time whether or not a given function call is valid in a given context, based on the type signature of the function (i.e. the types of its parameters, and the type of its return value.) This reduces the cognitive load of a programmer, improves clarity, self-documents, and also allows more individuals to collaborate on the same schema at scale.

Postgres allows for some level of type definition in its functional types (Function, Procedure, and Trigger et. al.) -- but prefers not to perform type checks on function changes for (what I assume are) performance reasons, and perhaps some lack of edge-case exhaustiveness.

Because so many benefits hinge on type safety -> I view this as a problem. The database, in my view, is supposed to be the "oracle of truth" for whatever segment of data / business logic it holds. If it can possibly be brought into an inconsistent state, that is quite scary. Many real-world issues, simply chalked up to "buggy software" can be ultimately traced down to lack of type enforcement, and lack of null-checks.

But we can't give up the speed of the database either -- and we don't want a frictional path to function updates. Which means in the short term -- we likely should NOT be forcing function type checks whenever a function changes -> So what we should probably do is have a recommended path for diagnosing and solving problems with type-safety in the schema.

One way around this would be to test every code path through the function definitions -> but that has issues because it requires either a test fuzzing suite, or automated tests to be written.

Instead, I propose we add a supabase db lint command, which allows one to check the validity of the current database.

I have written a rough spec for this above.

Describe the solution you'd like

This is a typescript file we're currently using to perform this job. It is rough, but will be roughly what I'd like to implement in Go over this weekend:

import { Command } from "commander";
import _ from "lodash";
import { resolve } from "path";
import { psqlCommandString, runCommandSingle, runMultipleDatabaseCommands } from "../../dbUtils";


if (require.main === module) {
    const program = new Command();

    program
        .name('supabase:db:check-types')
        .description('CLI to check types on the supabase database.')
        .version('0.0.0')
        // .option('-s --selectedSchemas <selectedSchemas>', "comma separated, space-less schemas (i.e. 'public,auth')");

    program.parse(process.argv);

    console.log(program.opts());

    const {
        selectedSchemas
    } = program.opts();


    Promise.resolve().then(async () => {
        await runCommandSingle("CREATE EXTENSION IF NOT EXISTS plpgsql_check;")
        const result = await runCommandSingle(`
            SELECT p.oid, p.proname, plpgsql_check_function(p.oid) FROM pg_catalog.pg_namespace n
            JOIN pg_catalog.pg_proc p ON pronamespace = n.oid
            JOIN pg_catalog.pg_language l ON p.prolang = l.oid
            WHERE l.lanname = 'plpgsql' AND p.prorettype <> 2279 AND n.nspname = 'public';
        `);

        const grouped = _.groupBy(result.rows, (r) => r.oid);

        _.keys(grouped).map((k) => {
            const lines = grouped[k];

            const texts = lines.map((l) => l.plpgsql_check_function);
            const isWarning = !!_.find(texts, (l)=>_.startsWith(l, "warning"));
            const str = ["", ...texts].join("\n\t");

            if (isWarning){
                console.log(`WARNING in function '${lines[0].proname}' (oid: ${k})`)
                console.warn(str)
            }
            else{
                console.log(`ERROR in function '${lines[0].proname}' (oid: ${k})`)
                console.error(str)
            }

            console.log("\n")
        })
    })
  
}

export {};

Describe alternatives you've considered

There may be other linters out there, but this one seems fine.

We could also ask people just to run this command on the database themselves, but it's a bit wordy, no? :)