• 
      

    Allow SQLExecutor to execute within a parent transaction, safely.

    Review Request #11267 — Created Nov. 5, 2020 and submitted

    Information

    Django Evolution
    master

    Reviewers

    SQLExecutor was built to always require running outside of a
    transaction, so that it could manage transactions itself. This was done
    because some databases don't really support rollbacks of schema changes
    properly within a transaction, and some operations in databases require
    running outside a transaction (namely, SQLite renaming of primary keys).

    However, it's not feasible to always run a database evolution fully
    outside of a transaction. It won't happen, for instance, in Review Board
    when installing/upgrading an extension, and it normally won't be a
    problem.

    Rather than outright fail, we now log a warning ahead of time. We then
    check any grouping of SQL statements that are to be executed, check if
    any require running outside a transaction (currnetly just the SQLite
    primary key case), and then log and raise an exception to prevent
    executing those statements.

    This should cause statements to fail before any damage would be done,
    invalidating the entire batch of evolutions.

    Unit tests pass.

    Verified this fixed an issue with unit test runs in Djblets.

    Summary ID
    Allow SQLExecutor to execute within a parent transaction, but warn.
    `SQLExecutor` was built to always require running outside of a transaction, so that it could manage transactions itself. This was done because some databases don't really support rollbacks of schema changes properly within a transaction, and some operations in databases require running outside a transaction (namely, SQLite renaming of primary keys). However, it's not feasible to always run a database evolution fully outside of a transaction. It won't happen, for instance, in Review Board when installing/upgrading an extension, and it normally won't be a problem. Rather than outright fail, we now log a warning ahead of time. We then check any grouping of SQL statements that are to be executed, check if any require running outside a transaction (currnetly just the SQLite primary key case), and then log and raise an exception to prevent executing those statements. This should cause statements to fail before any damage would be done, invalidating the entire batch of evolutions.
    a7e4ff00bc309dbe090e2d84245f46a4aebd3f2d
    david
    1. Ship It!
    2. 
        
    chipx86
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to master (aa9d70f)