Changes between Initial Version and Version 1 of Ticket #31286, comment 10


Ignore:
Timestamp:
Sep 23, 2025, 6:27:04 PM (2 weeks ago)
Author:
Tim Graham

Legend:

Unmodified
Added
Removed
Modified

  • Ticket #31286, comment 10

    initial v1  
    1 I think there's a misunderstanding with your statement: "Database checks ​were never ran by default even prior to this patch." The only database check (designated by `@register(Tags.database)`) invokes [https://github.com/django/django/blob/74a9c2711cd81708ae754c4d92432511efa96149/django/core/checks/database.py#L6-L14 DatabaseValidation.check()]. This invokes, for example, MySQL's [https://github.com/django/django/blob/74a9c2711cd81708ae754c4d92432511efa96149/django/db/backends/mysql/validation.py#L7-L36check on strict mode]. This check introspects the database and fails if it doesn't exist, so I understand that the test runner may need to skip these checks.
     1I think there's a misunderstanding with your statement: "Database checks ​were never ran by default even prior to this patch." The only database check (designated by `@register(Tags.database)`) invokes [https://github.com/django/django/blob/74a9c2711cd81708ae754c4d92432511efa96149/django/core/checks/database.py#L6-L14 DatabaseValidation.check()]. This invokes, for example, MySQL's [https://github.com/django/django/blob/74a9c2711cd81708ae754c4d92432511efa96149/django/db/backends/mysql/validation.py#L7-L36check on strict mode]. This check queries the database server, but as far as I can tell, it doesn't require the presence of the test database. Nonetheless, I see no issue with the test runner skipping these checks.
    22
    3 Separately, the `DatabaseValidation` class also has a [https://github.com/django/django/blob/74a9c2711cd81708ae754c4d92432511efa96149/django/db/backends/base/validation.py#L13-L32 check_field() hook] which calls `DatabaseValidation.check_field_type()` (if implemented by the backend, e.g. [https://github.com/django/django/blob/74a9c2711cd81708ae754c4d92432511efa96149/django/db/backends/mysql/validation.py#L38-L77 MySQL's]). This method does static analysis (checking field attributes) and doesn't require the presence of a database. The [https://github.com/django/django/blob/74a9c2711cd81708ae754c4d92432511efa96149/django/db/models/fields/__init__.py#L266-L274 calling chain] is `Field.check()` -> `Field._check_backend_specific_checks()`. These checks were run by `makemigrations` (and even by `runserver` which I'll get in to below) until 271fdab8b78af558238df51c64b4d1c8dd0792bb, which stopped iterating through all the databases (`django.db.connections`) and instead uses the list of `databases` aliases passed from (typically) the management command's `--database` argument (e.g. for [https://github.com/django/django/blob/74a9c2711cd81708ae754c4d92432511efa96149/django/core/management/commands/migrate.py#L94 migrate]).
     3Separately, the `DatabaseValidation` class also has a [https://github.com/django/django/blob/74a9c2711cd81708ae754c4d92432511efa96149/django/db/backends/base/validation.py#L13-L32 check_field() hook] which calls `DatabaseValidation.check_field_type()` (if implemented by the backend, e.g. [https://github.com/django/django/blob/74a9c2711cd81708ae754c4d92432511efa96149/django/db/backends/mysql/validation.py#L38-L77 MySQL's]). These checks do static analysis (checking field attributes), and while some of these checks may rely on a connection to the database server to populate `connection.features`, for example, I don't believe these checks require the presence of the test database. The [https://github.com/django/django/blob/74a9c2711cd81708ae754c4d92432511efa96149/django/db/models/fields/__init__.py#L266-L274 calling chain] is `Field.check()` -> `Field._check_backend_specific_checks()`. These checks were run by `makemigrations` (and even by `runserver` which I'll get in to below) until 271fdab8b78af558238df51c64b4d1c8dd0792bb, which stopped iterating through all the databases (`django.db.connections`) and instead uses the list of `databases` aliases passed from (typically) the management command's `--database` argument (e.g. for [https://github.com/django/django/blob/74a9c2711cd81708ae754c4d92432511efa96149/django/core/management/commands/migrate.py#L94 migrate]).
    44
    55I believe it's useful to have field-specific checks run for all databases at the `makemigrations` stage rather than deferring them until `migrate`, at which point a potentially invalid migration has already been created. Besides `Field._check_backend_specific_checks()`, the other checks that rely on `databases` are therefore are currently deferred are:
    … …  
    3232While thinking through this, I noticed another surprising, perhaps inadvertent regression / behavior change. Currently, in a multi-db setup (I'll use the aliases 'default' and 'other' as an example), unless I missed a place that passes all db aliases to `check()`, the only way to get the pre-Django 3.1 behavior for `Model._check_long_column_names()` (which consults all database aliases to find the maximum allowed column name) would be to invoke: `check --database=default --database=other`.
    3333
    34 Essentially, #31055 started treating the bulleted list of checks above as "database checks" which was overly aggressive. They are really static checks that don't require the presence of a database. These checks used to be a lot more visible since they were even invoked by `runserver` (right?). So arguably `runserver` (and even `check`) could get the same `get_check_kwargs()` treatment proposed above, and then we may want to go back to [https://github.com/django/django/commit/0b83c8cc4db95812f1e15ca19d78614e94cf38dd#diff-f10ce154e1f5bcdba92b54ebf8cb57357b404a3f03ab51ca85a031358f7372ecL66-L69 excluding database tagged checks by default].
     34Essentially, #31055 started treating the bulleted list of checks above as "database checks" which was perhaps overly aggressive and unneeded. They are really static checks that don't require the presence of a database. These checks used to be a lot more visible since they were even invoked by `runserver` (right?). So arguably `runserver` (and even `check`) could get the same `get_check_kwargs()` treatment proposed above, and then we may want to go back to [https://github.com/django/django/commit/0b83c8cc4db95812f1e15ca19d78614e94cf38dd#diff-f10ce154e1f5bcdba92b54ebf8cb57357b404a3f03ab51ca85a031358f7372ecL66-L69 excluding database tagged checks] unless `--database` is specified. What do you think?
Back to Top