Also flag concurrent index creation
Creating indices concurrently (provided that index creation *completes*) is generally a much safer way to build an index (since it allows reads *and* writes to the table during index creation). However, due to the inherent tradeoffs of concurrent index creation, the operation will almost always take *longer* than a traditional (blocking) index creation. It's actually much *more* important to lint against concurrent index creation when an exclusive lock is open & held. Test is from a comment by @David-Wobrock (I corrected syntax, though): https://github.com/3YOURMIND/django-migration-linter/pull/251#discussion_r1249298197
This commit is contained in:
1 parent
15dbe22f24
commit
86b660fbf6
2 files changed
+22
-4
No files matched your search
@@ -31,17 +31,17 @@ def has_create_index_in_transaction(sql_statements: list[str], **kwargs) -> bool
|
|||||||
# https://www.postgresql.org/docs/current/sql-altertable.html
|
# https://www.postgresql.org/docs/current/sql-altertable.html
|
||||||
# (Most common example is `ALTER TABLE... ADD COLUMN`, then later `CREATE INDEX`)
|
# (Most common example is `ALTER TABLE... ADD COLUMN`, then later `CREATE INDEX`)
|
||||||
if sql.startswith("ALTER TABLE"):
|
if sql.startswith("ALTER TABLE"):
|
||||||
return has_create_index(sql_statements[i + 1 :])
|
return has_create_index(sql_statements[i + 1 :], ignore_concurrently=False)
|
||||||
return False
|
return False
|
||||||
|
|
||||||
|
|
||||||
def has_create_index(sql_statements: list[str], **kwargs) -> bool:
|
def has_create_index(sql_statements: list[str], ignore_concurrently: bool = True, **kwargs) -> bool:
|
||||||
regex_result = None
|
regex_result = None
|
||||||
for sql in sql_statements:
|
for sql in sql_statements:
|
||||||
regex_result = re.search(r"CREATE (UNIQUE )?INDEX.*ON (.*) \(", sql)
|
regex_result = re.search(r"CREATE (UNIQUE )?INDEX.*ON (.*) \(", sql)
|
||||||
if re.search("INDEX CONCURRENTLY", sql):
|
if ignore_concurrently and re.search("INDEX CONCURRENTLY", sql):
|
||||||
regex_result = None
|
regex_result = None
|
||||||
elif regex_result:
|
if regex_result:
|
||||||
break
|
break
|
||||||
if not regex_result:
|
if not regex_result:
|
||||||
return False
|
return False
|
||||||
|
|||||||
@@ -331,6 +331,17 @@ class PostgresqlAnalyserTestCase(SqlAnalyserTestCase):
|
|||||||
]
|
]
|
||||||
self.assertWarningSql(sql, code="CREATE_INDEX_EXCLUSIVE")
|
self.assertWarningSql(sql, code="CREATE_INDEX_EXCLUSIVE")
|
||||||
|
|
||||||
|
def test_create_concurrently_index_exclusive(self):
|
||||||
|
sql = [
|
||||||
|
"BEGIN;",
|
||||||
|
'ALTER TABLE "users" ADD COLUMN "email" varchar(254) NULL;',
|
||||||
|
# Counter-intuitively, concurrent index creation can take *longer*.
|
||||||
|
# It's especially important that we flag an exclusive lock being held.
|
||||||
|
'CREATE INDEX CONCURRENTLY "user_email" ON "users" ("email");',
|
||||||
|
"COMMIT;",
|
||||||
|
]
|
||||||
|
self.assertWarningSql(sql, code="CREATE_INDEX_EXCLUSIVE")
|
||||||
|
|
||||||
def test_create_index_exclusive_no_lock(self):
|
def test_create_index_exclusive_no_lock(self):
|
||||||
sql = [
|
sql = [
|
||||||
'ALTER TABLE "users" ADD COLUMN "email" varchar(254) NULL;',
|
'ALTER TABLE "users" ADD COLUMN "email" varchar(254) NULL;',
|
||||||
@@ -339,6 +350,13 @@ class PostgresqlAnalyserTestCase(SqlAnalyserTestCase):
|
|||||||
# Will warn about `CREATE_INDEX`, but *not* `CREATE_INDEX_EXCLUSIVE`
|
# Will warn about `CREATE_INDEX`, but *not* `CREATE_INDEX_EXCLUSIVE`
|
||||||
self.assertValidSql(sql, allow_warnings=True)
|
self.assertValidSql(sql, allow_warnings=True)
|
||||||
|
|
||||||
|
def test_create_index_exclusive_concurrently_no_lock(self):
|
||||||
|
sql = [
|
||||||
|
'ALTER TABLE "users" ADD COLUMN "email" varchar(254) NULL;',
|
||||||
|
'CREATE INDEX CONCURRENTLY "user_email" ON "users" ("email");',
|
||||||
|
]
|
||||||
|
self.assertValidSql(sql)
|
||||||
|
|
||||||
|
|
||||||
class SqlUtilsTestCase(unittest.TestCase):
|
class SqlUtilsTestCase(unittest.TestCase):
|
||||||
def test_unknown_analyser_string(self):
|
def test_unknown_analyser_string(self):
|
||||||
|
|||||||
Reference in new issue
Block a user