This avoids security shenanigans that can come if your `git-commit-id`
may come from potentially untrusted source (e.g. an API), an attacker
could trick you to run shell commands like:
./manage.py lintmigrations --git-commit-id '; rm -rf dangerous'
or a malicious project may set a config like:
[tool.django_migration_linter]
git_commit_id = "; rm -rf dangerous"
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
While it's always a dangerous *pattern* to create an index while a
transaction is open & holds an exclusive lock, in practice it's not
always a big deal (for example, indexing a table with very few rows can
be fast). Accordingly, downgrade to a warning.
Briefly, this check helps prevent a long-running index creation from
preventing all reads and writes to a table.
This supplements an existing check
==================================
There is an existing check, `CREATE_INDEX` which warns about creating
indices without the `CONCURRENTLY` argument. It's a helpful check!
However, there are some valid reasons to prefer non-concurrent index
creation. For example, concurrent index creation may struggle to
complete when applied to write-heavy tables (since Postgres will have to
make repeated passes on the table to accommodate the new reads since the
last round).
The danger of `EXCLUSIVE` locks
===============================
If you *do* want to create indices nonconcurrently, there is still a
**huge footgun** to be aware of -- locks which are held in a transaction
are not released until the transaction closes.
Dangerous Django defaults
=========================
By default, any time you add a new Django field with `db_index=True` or
`unique=True`, the automatically generated `Migration` has:
- implicit setting of `atomic=True` (transactions are on)
- an `ALTER TABLE...ADD COLUMN` call (exclusively locks the table)
- a `CREATE INDEX` invocation (*hopefully* quick, due to nullable
columns)
It's even more dangerous if you combine an `ALTER TABLE` call with
`CREATE INDEX` of an existing, non-nullable column.
Favor `atomic=False` if you must index
======================================
Index-creating migrations should have few `operations`. Since index
creation does not modify row-level data (and only requires locking
against writes), there's no reason to use transactions.
It's prudent to bypass the default `atomic=True` for index creation.
Kevin Wu
David Wobrock
Igor Zmitrovich
Deven Dayal
Artem Streltsov
Lie Ryan
dependabot[bot]
jason.lee
David Cain
David Cain
Caspar Wylie
Nikita Karamov