Opened 12 years ago

Closed 3 years ago

#18088 closed Cleanup/optimization (fixed)

Add a "supports_foreign_key" database feature to ease testing

Reported by: Anssi Kääriäinen Owned by: nobody
Component: Testing framework Version: 1.4
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Currently our test coverage for checking that foreign keys are properly enforced in the database are almost non-existent. One reason for this is that it has not been easy to write tests, as some backends do not support foreign keys and information of foreign key support has not been available as !skipUnlessDBFeature target.

The attached patch adds a new database feature "supports_foreign_keys". The patch also uses this feature in regressiontests/backends/tests.py. The test results for backends tests are similar pre/post patch. MySQL + MyISAM fails one test both pre/post patch, other backends work correctly.

This patch is needed for writing proper tests for #18081 for example.

Attachments (1)

18088.diff (4.8 KB ) - added by Anssi Kääriäinen 12 years ago.

Download all attachments as: .zip

Change History (11)

by Anssi Kääriäinen, 12 years ago

Attachment: 18088.diff added

comment:1 by Claude Paroz, 12 years ago

I wonder if we really need both can_introspect_foreign_keys and supports_foreign_key... Could you investigate?

comment:2 by Anssi Kääriäinen, 12 years ago

Surprisingly we can introspect foreign keys on SQLite, but they are not enforced. Worth a comment though.

comment:3 by Russell Keith-Magee, 12 years ago

@akaariai -- they're not enforced *currently*, but they could be. Foreign key constraints were only added in SQLite 3.6.19 (see #14204); Django's SQLite backend doesn't currently provide any way to exploit them.

comment:4 by Anssi Kääriäinen, 12 years ago

Has patch: set

Understood.

Main point is that supports_foreign_keys is not always the same as can_introspect_foreign_keys. It would be actually possible to check if the SQLite backend supports foreign keys in the _supports_foreign_keys() call of SQLite. I am not going to piggy-back that change into this ticket. The current changes are relatively simple, SQLite foreign key support is not.

comment:5 by Aymeric Augustin, 12 years ago

Triage Stage: UnreviewedDesign decision needed

Sqlite 3.6.19 was released nearly three years ago. Debian stable ships Sqlite 3.7.3.

Shouldn't we at some point:

  • have a minimal version requirement for Sqlite,
  • deprecate support for MySQL + MyISAM,

and support only "sane" databases -- that is, databases that enforce foreign keys?

This is well beyond the scope of this ticket, but I'm not thrilled by the idea of adding code to deal with broken databases. Make that a -0...

comment:6 by Aymeric Augustin, 11 years ago

Can we make a decision? Anssi, it's your call.

comment:7 by Anssi Kääriäinen, 11 years ago

Triage Stage: Design decision neededAccepted

I think that as long as we have databases that do not support foreign keys then supports_foreign_keys flag is needed. So, accepted. This could of course be closed if somebody were to remove all databases without foreign key support.

comment:8 by Tim Graham, 10 years ago

Patch needs improvement: set

Patch no longer applies cleanly.

comment:9 by Tim Graham, 9 years ago

The supports_foreign_keys flag was added in d683263f97aedd67f17f4b78ac65d915f4e70d36, but there are still bits of the proposed patch that could be applied.

comment:10 by Mariusz Felisiak, 3 years ago

Resolution: fixed
Status: newclosed

We documented supported versions of SQLite in 1160a975968f86953a6c4405b772eb86283c5a4a and added a check in 7444f3252757ed4384623e5afd7dcfeef3e0c74e. Dropping support for MyISAM is beyond the scope of this ticket, so as far as I'm aware we can close it as fixed.

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