Migrations folder option

[danicc097]

What does this PR do?

Add an option to run migrations automatically in the same transaction if a folder is given.

Use case: while developing it's simpler to create a dedicated empty database that gets clean state and allows to develop both migrations itself and queries at the same time depending on what file is opened (will execute migrations up to N revision accordingly)

What issues does this PR fix or reference?

Is it tested? How?

[yas7010uv]

@danicc097 It's interesting :)

I think to actually use this feature user need to add VSCode setting for migration database file, in the migration directory.

Is this understanding, correct? The sample needs to be enhanced.

[danicc097]

The migrationsFolder setting may be a relative directory. I'm currently using it as migrationsFolder: "./db/migrations/" in my .vscode/settings.json or workspace settings. So in ./db/migrations I can have a single schema.up.sql or multiple XXXXXXXX_<migration_name>.up.sql files that get executed in order

[yassun7010]

If you are writing development and migration in a single VSCode setting, you will only see one database.

In development we expect the database to have the tables predefined, but in migration we expect the database to be empty.

How would you reconcile the two?

We need to write two VSCode setting files.

[danicc097]

As it is now, if you are opening a migration file it will be detected (right now by filename: document.uri.endsWith(file), but could be matching the complete migrationsFolder setting to be 100% sure). <- needs testing

When opening any migration file inside migrationsFolder execution of the migrations stops until the last migration, so you can work on the latest or even check previous migrations for correctness

When opening a query file all migrations are executed

[yassun7010]

The database schema for production and migration should be the same name or switched by the VSCode setting.

The migration file for your test case does not do table drop first.

If it is not an empty database (or schema), is it possible to edit the migration file?

[danicc097]

I added more examples and tests, I hope it's clearer now

1. Running down migrations first ensures all queries will always be deterministically analyzed (errors during migrations are explicitly shown so we don't get a "table doesn't exist" without context)
2. I'm using savepoints for migrations (up +down) so that multiple statements share it
3. In the end everything is rollbacked as usual

Offtopic: Is queryFileStaticAnalysis used? It clutters the logs with errors but none are shown to the user, I'm not sure what it's supposed to do

[yas7010uv]

I can now confirm locally that it works!!

Migration will be a big features by itself and should be configured as follows to allow for future upgrade.

{
    "plpgsqlLanguageServer.migration": {
        "migrationFolder": "migrations/migrations_test",
        "upMigrationFilePattern": "*.up.pgsql",
        "downMigrationFilePattern": "*.down.pgsql",
    }
}

Just a note, for users with multiple RDBs, this tool uses ".pgsql" as the default extension.

[yas7010uv]

You added new option "postMigrations", What is the need for this option?

The migration features is only performed when editing the migration file, so post migrations will always ROLLBACK.

[danicc097]

I usually have some scripts that setup triggers, etc. which are generic and get called after all migrations run, so as to not rewrite them in each new migration. This flag is optional is by default

Currently migrations are run every time, not just when editing a migration file (the difference is that, when editing a migration file, execution of the migrations stop at the opened migration file so it is correctly analyzed).

If you want to work on queries with production data and not on a clean state from migrations, I can add an optional alwaysRunMigrations bool option. With alwaysRunMigrations undefined migrations will run only if we detect we are in the plpgsqlLanguageServer.migrations.folder.

[yas7010uv]

Hmm..., I don't think it is a good idea to implicitly update the database.

The way this tool currently update the database is only if user explicitly executes the "Execute the Current File Query" command.

The implicit way of updating the database is fine for those who understand it, but I think it will confuse newcomers to the project.

At this time I would like to remove the "postMigrations" feature from this PR.

[danicc097]

I'm not proposing to update the database on analysis, that would be destructive and confusing for the user. In the end all this PR would do is setup a clean state for the analysis by dropping and recreating inside a transaction if the user opts into the migration optional feature (then everything is rollbacked). Additionally, what I proposed is some extra configuration to allow for:

1. other setup scripts to run via postMigrations (optional parameter)
2. for your use case be able to choose when to run migrations (if migration setting set) via something like alwaysRunMigrations. If I understand correctly you would not like to have migrations run when you're editing a query, only when editing migrations.

I could add a comment specifying the migrations feature does not commit any changes (which reminds me I also need to sanitize and remove all begin, rollback and commit lines from migrations like it was done with statements)

[yas7010uv]

I see.

It certainly doesn't seem to update the database.

[yas7010uv]

I will merge this and release it after some refactoring.

I'm busy this week, so I will probably work on it next week.