Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#33379 closed New feature (fixed)

Add minimum database version checks

Reported by: Tim Graham Owned by: Hasan Ramezani
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

7444f3252757ed4384623e5afd7dcfeef3e0c74e added a minimum version check for SQLite, but the other database backends don't have such a check. We sometimes get bug reports from people using an unsupported database version, so I think the checks would add value. Is a query to fetch the database version on startup an acceptable cost?

Perhaps there's a better way to run a query just once, but here's what I did in django-cockroachdb:
https://github.com/cockroachdb/django-cockroachdb/commit/27ebbefa515edf3ba68a5373dea48c4acdda60ab

We could make the check generic by adding DatabaseFeatures.minimum_database_version as well as a standard method to get the database version as suggested in #18332.

Change History (16)

comment:1 by Mariusz Felisiak, 2 years ago

Triage Stage: UnreviewedAccepted

Makes sense, thanks. IMO an extra query on startup is acceptable.

comment:2 by Claude Paroz, 2 years ago

Through a system check?

comment:3 by Jacob Walls, 2 years ago

Nick proposed a minimum check for MySQL in ticket:18392#comment:49.

comment:4 by Hasan Ramezani, 2 years ago

Has patch: set
Owner: changed from nobody to Hasan Ramezani
Status: newassigned

I've created a draft patch and implemented the check for the SQLite backend.
@Tim, please check it and let me know if the implementation and if you are ok I can continue with the other backends.

comment:5 by Mariusz Felisiak, 2 years ago

Patch needs improvement: set

comment:6 by Hasan Ramezani, 2 years ago

Patch needs improvement: unset

comment:7 by Mariusz Felisiak, 2 years ago

Patch needs improvement: set

comment:8 by Hasan Ramezani, 2 years ago

Patch needs improvement: unset

comment:9 by Tim Graham, 2 years ago

Patch needs improvement: set

As noted on the PR, I tested this with CockroachDB and it doesn't work because the system check framework runs after other initialization queries, some of which fail on older versions of CockroachDB. Perhaps moving the error to another method like DatabaseWrapper.init_connection_state() as I did for CockroachDB (implementation linked in ticket description) is more suitable.

comment:10 by Hasan Ramezani, 2 years ago

Patch needs improvement: unset

I've added a new commit to the existing patch and moved the database version check to init_connection_state.
Also, I've changed the tests.

comment:11 by Mariusz Felisiak, 2 years ago

Patch needs improvement: set

comment:12 by Hasan Ramezani, 2 years ago

Patch needs improvement: unset

comment:13 by Tim Graham, 2 years ago

Triage Stage: AcceptedReady for checkin

comment:14 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

Resolution: fixed
Status: assignedclosed

In 9ac3ef59:

Fixed #33379 -- Added minimum database version checks.

Thanks Tim Graham for the review.

comment:15 by Carlton Gibson <carlton@…>, 2 years ago

In 2cec020f:

Refs #33379 -- Fixed minimum supported version of MariaDB.

comment:16 by Carlton Gibson <carlton.gibson@…>, 2 years ago

In fad2e598:

[4.1.x] Refs #33379 -- Fixed minimum supported version of MariaDB.

Backport of 2cec020f5bc462954d902bdad1a5955852cecff6 from main

Note: See TracTickets for help on using tickets.
Back to Top