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.
Problem: Previously in gh-117 we aimed to solve gh-115, which described a problem where we used `SET DEFAULT` and then `DROP DEFAULT` immediately afterward. The solution was to track whether any defaults are retained, but the `ends_with_default` variable is initialized as `None` rather than `False`. At the end of the function we check whether `ends_with_default is False`, never occurs unless a default is set.
Solution: Assume that there is no default unless proven otherwise, and iniitializie `ends_with_default` to `False` instead of `None`.
Fixes: gh-158
* Misspelled filename => fail (instead of linting all migrations and ignoring the error)
* An empty file/a file with no valid migration paths => lint no migrations (instead oflinting all migrations)
* If the option is not specified, keep the behaviour of linting all migrations
Instead of using the file path when the git commit is not
added, use the file and the migrations list no matter if we select
all migrations or just from a certain git commit.
And unit tests :)
* don't use the project fixtures anymore but the one Django project
* update and refactor cache tests to be less brittle
* explicitly test mysql, postgresql and sqlite3 databases
* remove 'multiple_call' tests since it is not possible anymore
(with the Django singleton)
* update Travis and tox (dropping Python 3.4)
David Cain
David Cain
David Wobrock
Sondre Lillebø Gundersen
Sören Wegener
David Wobrock
Christian Bundy
Felix