[Django] #33509: Add SQL comment to describe deliberately no-op migration operations
Django
----------------------------------------+------------------------
Reporter: Adam Johnson | Owner: nobody
Type: New feature | Status: new
Component: Migrations | Version: dev
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
----------------------------------------+------------------------
Currently when a field migration is a no-op, the operation description is
output in SQL, but nothing else. This can be confusing as to which
operations are no-ops. It could be clearer if we output an extra SQL
comment when there are deliberately no statements to execute for a given
migration operation.
Take for example this output:
{{{
BEGIN;
--
-- Alter field name on Author
--
ALTER ...;
--
-- Alter field title on Book
--
COMMIT;
}}}
The `Author.name` field has an operation applied, whilst `Book.title`
needs no changes to the database. This isn't exactly clear from the output
- is the `COMMIT` part of the `Book.title` change?
It could be clearer as:
{{{
BEGIN;
--
-- Alter field name on Author
--
ALTER ...;
--
-- Alter field name on Author
--
-- (no-op)
COMMIT;
}}}
(Or perhaps more verbose wording, like "no SQL to execute")
I think this can help especially when there are consecutive operations
with no-op SQL:
{{{
BEGIN;
--
-- Alter field name on Author
--
-- (no-op)
--
-- Alter field name on Author
--
-- (no-op)
COMMIT;
}}}
(Inspired by #33470, where the OP suggested dropping such migration
operation header comments.)
--
Ticket URL: <https://code.djangoproject.com/ticket/33509>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Adam Johnson | Owner: nobody
Type: New feature | Status: new
Component: Migrations | Version: dev
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak):
Thanks for the ticket. Do you have an implementation idea? As far as I'm
aware we have the same issue here as in #33470, i.e. comments and
generated SQL are on different layers (migrations vs. schema editor).
--
Ticket URL: <https://code.djangoproject.com/ticket/33509#comment:1>
Django
Reporter: Adam Johnson | Owner: nobody
Type: New feature | Status: new
Component: Migrations | Version: dev
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Adam Johnson):
I made a PR: https://github.com/django/django/pull/15416 . Not so hard. If
the ticket is accepted I can finish off tests and docs.
It could also be a chance to move the “MIGRATION NOW PERFORMS OPERATION
THAT CANNOT BE WRITTEN AS SQL” comment after the operation header, for
consistency. I think it makes more sense to have the operation header
first, then a comment about its contents afterwards anyway. Otherwise the
"cannot be written as sql..." might be implied to mean the next several
named operations.
--
Ticket URL: <https://code.djangoproject.com/ticket/33509#comment:2>
Django
Reporter: Adam Johnson | Owner: Adam
Type: | Johnson
Cleanup/optimization | Status: assigned
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* status: new => assigned
* needs_tests: 0 => 1
* owner: nobody => Adam Johnson
* has_patch: 0 => 1
* type: New feature => Cleanup/optimization
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/33509#comment:3>
Django
Reporter: Adam Johnson | Owner: Adam
Type: | Johnson
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Adam Johnson):
* needs_tests: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/33509#comment:4>
Django
Reporter: Adam Johnson | Owner: Adam
Type: | Johnson
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/33509#comment:5>
Django
Reporter: Adam Johnson | Owner: Adam
Type: | Johnson
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Adam Johnson):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/33509#comment:6>
Django
Reporter: Adam Johnson | Owner: Adam
Type: | Johnson
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/33509#comment:7>
Django
Reporter: Adam Johnson | Owner: Adam
Type: | Johnson
Component: Migrations | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"6f453cd2981525b33925faaadc7a6e51fa90df5c" 6f453cd2]:
{{{
#!CommitTicketReference repository=""
revision="6f453cd2981525b33925faaadc7a6e51fa90df5c"
Fixed #33509 -- Added "(no-op)" to sqlmigrate output for operations
without SQL statement.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33509#comment:9>
Django
Reporter: Adam Johnson | Owner: Adam
Type: | Johnson
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"f15f7d395c99e3c1194c37eaa4f5958392c1ee07" f15f7d39]:
{{{
#!CommitTicketReference repository=""
revision="f15f7d395c99e3c1194c37eaa4f5958392c1ee07"
Refs #33509 -- Made sqlmigrate tests stricter and improved isolation.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33509#comment:8>