fabro(01KKTJ36NEWZZZZ509B67W586V): solve (success)

Fabro-Run: 01KKTJ36NEWZZZZ509B67W586V
Fabro-Completed: 3
Fabro-Checkpoint: 9e38fc4ec5

⚒️ Generated with [Fabro](https://fabro.sh)

SOLUTION_SUMMARY.md

@@ -0,0 +1,83 @@
+# Django Issue Fix: sqlmigrate wraps output in BEGIN/COMMIT even if database doesn't support transactional DDL
+
+## Problem Statement
+
+The `sqlmigrate` command was wrapping its output in `BEGIN`/`COMMIT` statements based only on whether a migration was atomic (`migration.atomic`), without considering whether the database actually supports transactional DDL.
+
+The migration executor already handles this correctly by checking both conditions:
+- `migration.atomic` - whether the migration is marked as atomic
+- `connection.features.can_rollback_ddl` - whether the database supports transactional DDL
+
+However, `sqlmigrate` only checked `migration.atomic`, leading to incorrect wrapping in databases that don't support transactional DDL (e.g., MySQL with MyISAM tables).
+
+## Solution
+
+### Change 1: Fix `django/core/management/commands/sqlmigrate.py` (Line 59-60)
+
+**Before:**
+```python
+# Show begin/end around output only for atomic migrations
+self.output_transaction = migration.atomic
+```
+
+**After:**
+```python
+# Show begin/end around output only for atomic migrations, and only if
+# the database supports transactional DDL.
+self.output_transaction = migration.atomic and connection.features.can_rollback_ddl
+```
+
+This change ensures that transaction wrappers are only added when:
+1. The migration is atomic (`migration.atomic == True`)
+2. **AND** the database supports transactional DDL (`connection.features.can_rollback_ddl == True`)
+
+This matches the behavior of the migration executor's schema editor, which uses the same logic for `atomic_migration`.
+
+### Change 2: Add Test in `tests/migrations/test_commands.py`
+
+Added a new test `test_sqlmigrate_for_atomic_migration_without_rollback_ddl` that verifies the fix:
+
+```python
+@override_settings(MIGRATION_MODULES={"migrations": "migrations.test_migrations"})
+def test_sqlmigrate_for_atomic_migration_without_rollback_ddl(self):
+    """
+    Transaction wrappers aren't shown for atomic migrations when the database
+    doesn't support transactional DDL.
+    """
+    out = io.StringIO()
+    with mock.patch.object(connection.features, 'can_rollback_ddl', False):
+        call_command("sqlmigrate", "migrations", "0001", stdout=out)
+    output = out.getvalue().lower()
+    queries = [q.strip() for q in output.splitlines()]
+    if connection.ops.start_transaction_sql():
+        self.assertNotIn(connection.ops.start_transaction_sql().lower(), queries)
+    self.assertNotIn(connection.ops.end_transaction_sql().lower(), queries)
+```
+
+This test:
+- Uses an atomic migration (`migrations.test_migrations.0001_initial`)
+- Mocks the database feature `can_rollback_ddl` to return `False`
+- Verifies that BEGIN/COMMIT statements are NOT in the output
+- Follows the same pattern as the existing `test_sqlmigrate_for_non_atomic_migration` test
+
+## Test Results
+
+All tests pass:
+- ✅ `test_sqlmigrate_forwards` - Existing test for atomic migrations (with DDL support)
+- ✅ `test_sqlmigrate_backwards` - Existing test for reverse migrations
+- ✅ `test_sqlmigrate_for_non_atomic_migration` - Existing test for non-atomic migrations
+- ✅ `test_sqlmigrate_for_atomic_migration_without_rollback_ddl` - New test for atomic migrations without DDL support
+
+## Impact
+
+### Databases Affected
+- **MySQL with MyISAM**: Does not support transactional DDL → will no longer wrap in BEGIN/COMMIT
+- **PostgreSQL, SQLite**: Support transactional DDL → behavior unchanged
+- **Oracle, MSSQL**: Support transactional DDL → behavior unchanged
+
+### Backward Compatibility
+This is a bug fix that corrects the behavior to match the migration executor. Any code relying on the previous incorrect behavior should be updated.
+
+## Files Modified
+1. `django/core/management/commands/sqlmigrate.py` - Fixed the condition for `output_transaction`
+2. `tests/migrations/test_commands.py` - Added test for the fix

sqlmigrate.py.fixed

@@ -0,0 +1,66 @@
+from django.apps import apps
+from django.core.management.base import BaseCommand, CommandError
+from django.db import DEFAULT_DB_ALIAS, connections
+from django.db.migrations.executor import MigrationExecutor
+from django.db.migrations.loader import AmbiguityError
+
+
+class Command(BaseCommand):
+    help = "Prints the SQL statements for the named migration."
+
+    output_transaction = True
+
+    def add_arguments(self, parser):
+        parser.add_argument('app_label', help='App label of the application containing the migration.')
+        parser.add_argument('migration_name', help='Migration name to print the SQL for.')
+        parser.add_argument(
+            '--database', default=DEFAULT_DB_ALIAS,
+            help='Nominates a database to create SQL for. Defaults to the "default" database.',
+        )
+        parser.add_argument(
+            '--backwards', action='store_true',
+            help='Creates SQL to unapply the migration, rather than to apply it',
+        )
+
+    def execute(self, *args, **options):
+        # sqlmigrate doesn't support coloring its output but we need to force
+        # no_color=True so that the BEGIN/COMMIT statements added by
+        # output_transaction don't get colored either.
+        options['no_color'] = True
+        return super().execute(*args, **options)
+
+    def handle(self, *args, **options):
+        # Get the database we're operating from
+        connection = connections[options['database']]
+
+        # Load up an executor to get all the migration data
+        executor = MigrationExecutor(connection)
+
+        # Resolve command-line arguments into a migration
+        app_label, migration_name = options['app_label'], options['migration_name']
+        # Validate app_label
+        try:
+            apps.get_app_config(app_label)
+        except LookupError as err:
+            raise CommandError(str(err))
+        if app_label not in executor.loader.migrated_apps:
+            raise CommandError("App '%s' does not have migrations" % app_label)
+        try:
+            migration = executor.loader.get_migration_by_prefix(app_label, migration_name)
+        except AmbiguityError:
+            raise CommandError("More than one migration matches '%s' in app '%s'. Please be more specific." % (
+                migration_name, app_label))
+        except KeyError:
+            raise CommandError("Cannot find a migration matching '%s' from app '%s'. Is it in INSTALLED_APPS?" % (
+                migration_name, app_label))
+        targets = [(app_label, migration.name)]
+
+        # Show begin/end around output only for atomic migrations, and only if
+        # the database supports transactional DDL.
+        self.output_transaction = migration.atomic and connection.features.can_rollback_ddl
+
+        # Make a plan that represents just the requested migrations and show SQL
+        # for it
+        plan = [(executor.loader.graph.nodes[targets[0]], options['backwards'])]
+        sql_statements = executor.collect_sql(plan)
+        return '\n'.join(sql_statements)