Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CREATE TABLE and CREATE INDEX do not trigger prefer-robust-statements warnings individually #325

Open
febpetroni opened this issue Dec 5, 2023 · 2 comments

Comments

@febpetroni
Copy link

Version: v0.24.2

Config file:

excluded_rules = [
    "adding-field-with-default", 
    "adding-not-nullable-field", 
    "ban-drop-column", 
    "ban-drop-table", 
    "ban-drop-not-null", 
    "changing-column-type", 
    "renaming-table", 
    "renaming-column", 
    "require-concurrent-index-creation", 
    "require-concurrent-index-deletion", 
    "prefer-big-int", 
    "prefer-bigint-over-int", 
    "prefer-bigint-over-smallint", 
    "prefer-timestamptz" 
]

Migration file 1:

CREATE TABLE some_table(
    id UUID PRIMARY KEY,
    version INTEGER NOT NULL,
    created_at TIMESTAMP WITHOUT TIME ZONE NOT NULL,
    updated_at TIMESTAMP WITHOUT TIME ZONE NOT NULL
);

Output:

Found 0 issues in 1 file 🎉

Expected:

warning: prefer-robust-stmts

   1 | CREATE TABLE some_table(
   2 |     id UUID PRIMARY KEY,
   3 |     version INTEGER NOT NULL,
   4 |     created_at TIMESTAMP WITHOUT TIME ZONE NOT NULL,
   5 |     updated_at TIMESTAMP WITHOUT TIME ZONE NOT NULL
   6 | );

  help: Consider wrapping in a transaction or adding a IF NOT EXISTS clause if the statement supports it.

Migration file 2:

CREATE INDEX some_table_idx ON some_table (version);

Output:

Found 0 issues in 1 file 🎉

Expected:

warning: prefer-robust-stmts

   1 | CREATE INDEX some_table_idx ON some_table (version);

  help: Consider wrapping in a transaction or adding a IF NOT EXISTS clause if the statement supports it.


However, if both statements are in the same migration file, it works as expected:

Migration file 3:

CREATE TABLE some_table(
    id UUID PRIMARY KEY,
    version INTEGER NOT NULL,
    created_at TIMESTAMP WITHOUT TIME ZONE NOT NULL,
    updated_at TIMESTAMP WITHOUT TIME ZONE NOT NULL
);
CREATE INDEX some_table_idx ON some_table (version);

Output (as expected):

some_clever_test.sql:1:0: warning: prefer-robust-stmts

   1 | CREATE TABLE some_table(
   2 |     id UUID PRIMARY KEY,
   3 |     version INTEGER NOT NULL,
   4 |     created_at TIMESTAMP WITHOUT TIME ZONE NOT NULL,
   5 |     updated_at TIMESTAMP WITHOUT TIME ZONE NOT NULL
   6 | );

  help: Consider wrapping in a transaction or adding a IF NOT EXISTS clause if the statement supports it.

some_clever_test.sql:7:1: warning: prefer-robust-stmts

   7 | CREATE INDEX some_table_idx ON some_table (version);

  help: Consider wrapping in a transaction or adding a IF NOT EXISTS clause if the statement supports it.
@chdsbd
Copy link
Collaborator

chdsbd commented Dec 6, 2023

Hi @febpetroni

I think this behavior is expected.

prefer-robust-statements works on a single file level. The rule ensures that the statements in a file can be run multiple times in case of a partial failure.

If a file only has one statement, it can be run multiple times without issue.

If there are multiple statements, then more care is needed. Either wrapping a transaction or adding an if not exists clause

@febpetroni
Copy link
Author

Hi @chdsbd , thanks for the quick response!

I think I understand what you mean... if you're using a migration tool, like flyway, a one-line migration will either work and be persisted (so that it won't actually run again), or will fail and must be fixed; meanwhile for migrations with more than one line and without transactional scope, we will have partial commits that won't let us run the whole migration again.

I had the impression that prefer-robust-statements would ask to use if exists / if not exists as a good practice, regardless of the number of lines in the migration. Thanks for clarifying!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants