public/django-migration-linter
15
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix #264 -- Don't detect an index creation during a transaction with an exclusive lock, when the table is being created

Browse Source 
Improve documentation on this issue.
This commit is contained in:
Chris Conn authored and David Wobrock committed 2025-03-30 18:53:34 +02:00
1 parent c2c0d5886a
commit eb59c472fb
4 files changed
+50 -21
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

CHANGELOG.md
+3
View File
@@ -1,5 +1,8 @@
## 5.x.y ## 5.x.y
Bug:
- Don't detect an index creation during a transaction with an exclusive lock, when the table is being created (#264)
Miscellaneous: Miscellaneous:
- Add support for Python 3.13 - Add support for Python 3.13
docs/incompatibilities.md
+18 -18
View File
@@ -25,24 +25,24 @@ On data migrations, the linter will show a warning when:
You can ignore checks through the `--exclude-migration-tests` option by specifying any of the codes: You can ignore checks through the `--exclude-migration-tests` option by specifying any of the codes:
| Code | Description | Default type | | Code | Description | Default type |
|------------------------------------|----------------------------------------------------------------------------------------------------------------------|--------------| |------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|--------------|
| `NOT_NULL` | Not NULL constraint on columns | Error | | `NOT_NULL` | Not NULL constraint on columns | Error |
| `DROP_COLUMN` | Dropping columns | Error | | `DROP_COLUMN` | Dropping columns | Error |
| `DROP_TABLE` | Dropping tables | Error | | `DROP_TABLE` | Dropping tables | Error |
| `RENAME_COLUMN` | Renaming columns | Error | | `RENAME_COLUMN` | Renaming columns | Error |
| `RENAME_TABLE` | Renaming tables | Error | | `RENAME_TABLE` | Renaming tables | Error |
| `ALTER_COLUMN` | Altering columns (could be backward compatible) | Error | | `ALTER_COLUMN` | Altering columns (could be backward compatible) | Error |
| `ADD_UNIQUE` | Add unique constraints | Error | | `ADD_UNIQUE` | Add unique constraints | Error |
| `RUNPYTHON_REVERSIBLE` | RunPython data migration is not reversible (missing reverse code) | Warning | | `RUNPYTHON_REVERSIBLE` | RunPython data migration is not reversible (missing reverse code) | Warning |
| `RUNPYTHON_ARGS_NAMING_CONVENTION` | By convention, RunPython names two arguments: apps, schema_editor | Warning | | `RUNPYTHON_ARGS_NAMING_CONVENTION` | By convention, RunPython names two arguments: apps, schema_editor | Warning |
| `RUNPYTHON_MODEL_IMPORT` | Missing apps.get_model() calls for model | Error | | `RUNPYTHON_MODEL_IMPORT` | Missing apps.get_model() calls for model | Error |
| `RUNPYTHON_MODEL_VARIABLE_NAME` | The model variable name is different from the model class itself | Warning | | `RUNPYTHON_MODEL_VARIABLE_NAME` | The model variable name is different from the model class itself | Warning |
| `RUNSQL_REVERSIBLE` | RunSQL data migration is not reversible (missing reverse SQL) | Warning | | `RUNSQL_REVERSIBLE` | RunSQL data migration is not reversible (missing reverse SQL) | Warning |
| `CREATE_INDEX` | (Postgresql specific) Creating an index without the concurrent keyword will lock the table and may generate downtime | Warning | | `CREATE_INDEX` | (Postgresql specific) Creating an index without the concurrent keyword will lock the table and may generate downtime | Warning |
| `CREATE_INDEX_EXCLUSIVE` | (Postgresql specific) Creating an index in a transaction (after an `EXCLUSIVE` lock) prolongs the exclusive lock | Warning | | `CREATE_INDEX_EXCLUSIVE` | (Postgresql specific) Creating an index in a transaction acquiring an `EXCLUSIVE` lock (e.g. most `ALTER TABLE` statements acquire one) prolongs the exclusive lock on the table. Using concurrently clause does not address the issue. On the contrary, it prolongs the transaction, making it more dangerous in this situation. | Warning |
| `DROP_INDEX` | (Postgresql specific) Dropping an index without the concurrent keyword will lock the table and may generate downtime | Warning | | `DROP_INDEX` | (Postgresql specific) Dropping an index without the concurrent keyword will lock the table and may generate downtime | Warning |
| `REINDEX` | (Postgresql specific) Reindexing will lock the table and may generate downtime | Warning | | `REINDEX` | (Postgresql specific) Reindexing will lock the table and may generate downtime | Warning |
## Details about backward incompatibilities ## Details about backward incompatibilities
src/django_migration_linter/sql_analyser/postgresql.py
+5 -3
View File
@@ -33,7 +33,7 @@ def has_create_index_in_transaction(sql_statements: list[str], **kwargs) -> bool
# (Most common example is `ALTER TABLE... ADD COLUMN`, then later # (Most common example is `ALTER TABLE... ADD COLUMN`, then later
# `CREATE INDEX`) # `CREATE INDEX`)
if sql.startswith("ALTER TABLE"): if sql.startswith("ALTER TABLE"):
return has_create_index(sql_statements[i + 1 :], ignore_concurrently=False) return has_create_index(sql_statements, ignore_concurrently=False)
return False return False
@@ -41,7 +41,7 @@ def has_create_index(
sql_statements: list[str], ignore_concurrently: bool = True, **kwargs sql_statements: list[str], ignore_concurrently: bool = True, **kwargs
) -> bool: ) -> bool:
regex_result = None regex_result = None
for sql in sql_statements: for i, sql in enumerate(sql_statements):
regex_result = re.search(r"CREATE (UNIQUE )?INDEX.*ON (.*) \(", sql) regex_result = re.search(r"CREATE (UNIQUE )?INDEX.*ON (.*) \(", sql)
if ignore_concurrently and re.search("INDEX CONCURRENTLY", sql): if ignore_concurrently and re.search("INDEX CONCURRENTLY", sql):
regex_result = None regex_result = None
@@ -50,9 +50,11 @@ def has_create_index(
if not regex_result: if not regex_result:
return False return False
preceding_sql_statements = sql_statements[:i]
concerned_table = regex_result.group(2) concerned_table = regex_result.group(2)
table_is_added_in_transaction = any( table_is_added_in_transaction = any(
sql.startswith(f"CREATE TABLE {concerned_table}") for sql in sql_statements sql.startswith(f"CREATE TABLE {concerned_table}")
for sql in preceding_sql_statements
) )
return not table_is_added_in_transaction return not table_is_added_in_transaction
tests/unit/test_sql_analyser.py
+24
View File
@@ -315,6 +315,13 @@ class PostgresqlAnalyserTestCase(SqlAnalyserTestCase):
'CREATE INDEX ON "films" ((lower("title")));', 'CREATE INDEX ON "films" ((lower("title")));',
] ]
self.assertValidSql(sql) self.assertValidSql(sql)
sql = [
'CREATE TABLE "films" ("title" text);',
'ALTER TABLE "films" ADD CONSTRAINT "title_not_empty" CHECK (char_length("title") > 0)'
'CREATE INDEX ON "films" ((lower("title")));',
]
self.assertValidSql(sql)
# Not the same column or tables.
sql = [ sql = [
'CREATE TABLE "some_table" ("title" text);', 'CREATE TABLE "some_table" ("title" text);',
'CREATE INDEX ON "films" ((lower("title")));', 'CREATE INDEX ON "films" ((lower("title")));',
@@ -363,6 +370,23 @@ class PostgresqlAnalyserTestCase(SqlAnalyserTestCase):
] ]
self.assertWarningSql(sql, code="CREATE_INDEX_EXCLUSIVE") self.assertWarningSql(sql, code="CREATE_INDEX_EXCLUSIVE")
def test_create_index_exclusive_with_prior_table_creation(self):
sql = [
"BEGIN;",
'CREATE TABLE "users" ("email" text);',
'CREATE INDEX "user_email" ON "users" ("email");',
"COMMIT;",
]
self.assertValidSql(sql)
sql = [
"BEGIN;",
'CREATE TABLE "users" ("id" number);',
'ALTER TABLE "users" ADD COLUMN "email" varchar(254) NULL;',
'CREATE INDEX "user_email" ON "users" ("email");',
"COMMIT;",
]
self.assertValidSql(sql)
def test_create_concurrently_index_exclusive(self): def test_create_concurrently_index_exclusive(self):
sql = [ sql = [
"BEGIN;", "BEGIN;",