From 86b660fbf63c85f1b36ec60d528ffb771400129f Mon Sep 17 00:00:00 2001
From: David Cain <david.cain@color.com>
Date: Mon, 3 Jul 2023 13:04:30 -0700
Subject: [PATCH] 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
---
 .../sql_analyser/postgresql.py                 |  8 ++++----
 tests/unit/test_sql_analyser.py                | 18 ++++++++++++++++++
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/django_migration_linter/sql_analyser/postgresql.py b/django_migration_linter/sql_analyser/postgresql.py
index 9fbf920..cbaec23 100644
--- a/django_migration_linter/sql_analyser/postgresql.py
+++ b/django_migration_linter/sql_analyser/postgresql.py
@@ -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
         # (Most common example is `ALTER TABLE... ADD COLUMN`, then later `CREATE INDEX`)
         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
 
 
-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
     for sql in sql_statements:
         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
-        elif regex_result:
+        if regex_result:
             break
     if not regex_result:
         return False
diff --git a/tests/unit/test_sql_analyser.py b/tests/unit/test_sql_analyser.py
index 0781f51..b0a78aa 100644
--- a/tests/unit/test_sql_analyser.py
+++ b/tests/unit/test_sql_analyser.py
@@ -331,6 +331,17 @@ class PostgresqlAnalyserTestCase(SqlAnalyserTestCase):
         ]
         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):
         sql = [
             '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`
         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):
     def test_unknown_analyser_string(self):