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 any of the introspection methods 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.

Version 0, edited 2 years ago by Anvesh Mishra (next)

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