[Django] #27060: Take indexes into account in inspectdb command
Django
--------------------------------------------+------------------------
Reporter: akki | Owner: nobody
Type: New feature | Status: new
Component: Core (Management commands) | Version: master
Severity: Normal | Keywords: db-indexes
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------------+------------------------
Currently, `inspectdb` command takes into account existing tables (via
`models.Model`), columns (via `models.Field`) and unique constraints (via
`unique_together`) while inspecting the db. It would be helpful to take
into account the existing indexes (via `models.Index`) on the table as
well.
--
Ticket URL: <https://code.djangoproject.com/ticket/27060>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: akki | Owner: akki
Type: New feature | Status: assigned
Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution:
Keywords: db-indexes | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by akki):
* owner: nobody => akki
* needs_better_patch: => 0
* status: new => assigned
* needs_tests: => 0
* needs_docs: => 0
Comment:
I have an almost ready patch for this feature.
--
Ticket URL: <https://code.djangoproject.com/ticket/27060#comment:1>
Django
Reporter: akki | Owner: akki
Type: New feature | Status: assigned
Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/27060#comment:2>
Django
Reporter: akki | Owner: akki
Type: New feature | Status: assigned
Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution:
Keywords: db-indexes | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by akki):
* has_patch: 0 => 1
Comment:
PR: https://github.com/django/django/pull/7083
--
Ticket URL: <https://code.djangoproject.com/ticket/27060#comment:3>
Django
Reporter: akki | Owner: akki
Type: New feature | Status: assigned
Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution:
Keywords: db-indexes | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
Comment:
The complication is that `models.Index` might not match the type of the
index. As noted on the PR, I'd think we'd at least want to add a comment
in the output if the type of the introspected index doesn't match.
--
Ticket URL: <https://code.djangoproject.com/ticket/27060#comment:4>
Django
Reporter: akki | Owner: akki
Type: New feature | Status: assigned
Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution:
Keywords: db-indexes | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by akki):
* keywords: db-indexes => db-indexes inspectdb
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/27060#comment:5>
Django
Reporter: akki | Owner: akki
Type: New feature | Status: assigned
Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution:
Keywords: db-indexes | Triage Stage: Accepted
inspectdb |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/27060#comment:6>
Django
Reporter: akki | Owner: akki
Type: New feature | Status: assigned
Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution:
Keywords: db-indexes | Triage Stage: Accepted
inspectdb |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by claudep):
Another question is about `index_together` vs `models.Index`. Are we going
to keep both way of defining multicolumn indexes? Should one be privileged
over the other? Depending on the answer, the patch might have to be
adapted.
--
Ticket URL: <https://code.djangoproject.com/ticket/27060#comment:7>
Django
Reporter: akki | Owner: akki
Type: New feature | Status: assigned
Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution:
Keywords: db-indexes | Triage Stage: Accepted
inspectdb |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by akki):
Some steps have been taken to deprecate `index_together`, see -
https://code.djangoproject.com/ticket/27064
I created a ticket to track deprecation of `index_together` here -
https://code.djangoproject.com/ticket/27236
--
Ticket URL: <https://code.djangoproject.com/ticket/27060#comment:8>
Django
Reporter: Akshesh Doshi | Owner: (none)
Type: New feature | Status: new
commands) |
Severity: Normal | Resolution:
Keywords: db-indexes | Triage Stage: Accepted
inspectdb |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by Akshesh Doshi):
* owner: Akshesh Doshi => (none)
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/27060#comment:9>
Django
Reporter: Akshesh Doshi | Owner: pahaz
Type: New feature | Status: assigned
commands) |
Severity: Normal | Resolution:
Keywords: db-indexes | Triage Stage: Accepted
inspectdb |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by pahaz):
* owner: (none) => pahaz
* needs_better_patch: 1 => 0
* status: new => assigned
Comment:
PR: https://github.com/django/django/pull/11944
This PR:
- [x] Add db_index field param
Next PR:
- [ ] Add Meta.indexes support
--
Ticket URL: <https://code.djangoproject.com/ticket/27060#comment:10>
Django
Reporter: Akshesh Doshi | Owner: Pavel
| White
Type: New feature | Status: assigned
Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution:
Keywords: db-indexes | Triage Stage: Accepted
inspectdb |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* needs_better_patch: 0 => 1
Comment:
This ticket is about adding support for `Meta.indexes` to the `inspectdb`
not about the `db_index` option. Mainly because it can be hard to ensure
that an inspected index is exactly the same as an index created by Django
(type, ordering, operator classes, `fillfactor` etc.). That's why we
accepted a ticket to support `Meta.indexes` where these options may be
included.
--
Ticket URL: <https://code.djangoproject.com/ticket/27060#comment:11>
Django
Reporter: Akshesh Doshi | Owner: Drew
| Winstel
Type: New feature | Status: assigned
Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution:
Keywords: db-indexes | Triage Stage: Accepted
inspectdb |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by felixxm):
* owner: Pavel White => Drew Winstel
Comment:
[https://github.com/django/django/pull/12712 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/27060#comment:12>
Django
Reporter: Akshesh Doshi | Owner: Drew
| Winstel
Type: New feature | Status: assigned
commands) |
Severity: Normal | Resolution:
Keywords: db-indexes | Triage Stage: Accepted
inspectdb |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by David Wobrock):
* cc: David Wobrock (added)
Comment:
I started looking a bit into this ticket, and one of the issues I
encounter is that we don't get back most parameters needed to reconstruct
the `Index` class.
With the current `introspection.get_constraint` implementations, we will
mainly be able to re-create the fields and order. We have some
information, but:
- few information about `expressions` (we could mainly infer that we used
expressions if no columns are specified for that index)
- nothing about `opclasses` (even if postgresql specific)
- nothing about `db_tablespace`
- nothing about `condition`
- nothing about `include` (even if postgresql specific)
And I strongly suspect nothing either on `buffering`, `fillfactor` and
others for `postgresql.indexes` :/
The best I can think of would be to
1. add a new introspection method for each database, that would allow to
fetch the `CREATE INDEX` SQL statement from a tablename+indexname (if this
is possible on all supported databases)
2. from the SQL statement, try to reverse engineer it in a best effort
manner and extract all given elements. Some elements will be very
difficult to express in Django again (like expressions), so we probably
should display a comment with the original `CREATE INDEX`
How does that sound? (to me a bit brittle admittedly)
An even simpler approach could be to only handle fields+order, and always
add the `CREATE INDEX` statement as a comment for the user then to
manually convert it to the right `Index` parameters.
--
Ticket URL: <https://code.djangoproject.com/ticket/27060#comment:13>