Add SQL comment to describe deliberately no-op migration operations

[Adam Johnson]

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.)

[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).

[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.

[Adam Johnson]

​PR

[Mariusz Felisiak <felisiak.mariusz@…>]

In f15f7d39:

Refs #33509 -- Made sqlmigrate tests stricter and improved isolation.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 6f453cd2:

Fixed #33509 -- Added "(no-op)" to sqlmigrate output for operations without SQL statement.