Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#32234 closed Cleanup/optimization (fixed)

inspectdb should inform about composite keys.

Reported by: Damien Owned by: Anvesh Mishra
Component: Core (Management commands) Version: 3.1
Severity: Normal Keywords:
Cc: Ad Timmering 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 (last modified by Damien)

Since django does not support multiple multiple-column primary keys, there should be a warning that the inspectdb result does not match exactly the database

PR

Change History (12)

comment:1 by Mariusz Felisiak, 3 years ago

Easy pickings: unset
Has patch: set
Needs tests: set
Owner: changed from nobody to Damien
Patch needs improvement: set
Status: newassigned
Triage Stage: UnreviewedAccepted

inspectdb is meant as a shortcut, not as definitive model generation. I would rather add a comment to the field's notes like we do in other cases. Raising a warning can be really annoying when you try to inspect a legacy database.

comment:2 by Mariusz Felisiak, 3 years ago

Component: Database layer (models, ORM)Core (Management commands)
Summary: Add warning when multiple-column primary keys are found when using inspectdbinspectdb should inform about composite keys.
Type: New featureCleanup/optimization

comment:3 by Damien, 3 years ago

Description: modified (diff)

comment:4 by Damien, 3 years ago

Updated the PR to change the warning to be a comment instead of printing it to stderr

comment:5 by Ad Timmering, 2 years ago

Cc: Ad Timmering added

comment:6 by Anvesh Mishra, 2 years ago

Owner: changed from Damien to Anvesh Mishra

comment:7 by Anvesh Mishra, 2 years ago

I actually found a solution to this in SQLite3 through PRAGMA keyword but since PRAGMA is a SQLite3 specific command so my PR didn't passed the Jenkins tests https://github.com/django/django/pull/15683. If you could provide any suggestions cause if we use get_constraint method to fetch the primary key it only takes the first column as primary key and gives no info if there were other primary-key columns. Also I tried the changes given in PR https://github.com/django/django/pull/13736 but they didn't work.

Last edited 2 years ago by Anvesh Mishra (previous) (diff)

comment:8 by Anvesh Mishra, 2 years ago

Submitted a new PR with all the required changes. https://github.com/django/django/pull/15730

comment:9 by Mariusz Felisiak, 2 years ago

Needs documentation: set

comment:10 by Mariusz Felisiak, 2 years ago

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

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

Resolution: fixed
Status: assignedclosed

In 295249c:

Fixed #32234 -- Made inspectdb inform about composite primary keys.

comment:12 by GitHub <noreply@…>, 2 years ago

In 9a3b7e5e:

Refs #32234 -- Removed hardcoded IntegerField in inspectdb test.

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