• 
  
    
 
    
 
    

    



    
    
    
    
     
    




    
 
    

  
    



    
 

 

  

  

 

    


  
    





    
 
    



  

    
 
 
    
  
    Fix the order of SQL statements when creating batches of new models.
    
 
    

    



 
    
 
    

  Review Request #11167
  —
  Created Sept. 10, 2020 and submitted 

 
    


    




    




    






 
    
  
    
   Information

  
    



    
 
 
    
  chipx86
 
    

    





    
 
 
    
  Django Evolution
 
    

    





    
 
 
    
  master
 
    

    





    
 
 
    
  
 
    

    





    
 
 
    
  
 
    

    



 
    



 
    
  
    
   Reviewers

  
    



    
 
 
    
  django-evolution
 
    

    





    
 
 
    
  
 
    

    



 
    



    


    
 
    




  

    
 
 
    
  
    When creating models, EvolveAppTask was generating and storing the SQL
    
for an entire model creation, which included not just the CREATE TABLE
    
but any deferred constraints. Each batch of SQL statements would later
    
be executed in order. If any of these SQL statements contained
    
constraints that referenced a table introduced by a subsequent task, the
    
SQL would fail to execute, because that table wouldn't have existed yet.
    

    What's supposed to happen is that the deferred statements are executed
    
after the tables are created. Hence the "deferred" part. The way the
    
logic worked, this wasn't possible. SchemaEditor knows how to do this
    
correctly, as does our legacy logic for Django 1.6, but they require
    
having the full list of new models (not just a subset) passed to them.
    

    This change redoes this logic to gather all models up-front and to
    
generate the SQL in one go, ensuring that the ordering of statements is
    
correct. The only real behavioral change introduced from this is that we
    
have to emit all creating_models signals up-front, and then emit all
    
created_models after all models are created, rather than emitting
    
these in pairs. This means that callers can't expect to receive them in
    
that order.
    

    It also means that the evolve command will be listing all
    
"Creating new database models for ..." up-front, rather than when each
    
model will be created. It's an annoyance, but hopefully a mostly
    
harmless one, and is necessary without a much larger reworking of how
    
this SQL is generated.
    
 
    

    





  

    
 
 
    
  
    Unit tests pass for all supported databases.
    

    Verified this fixed an issue with setting up a Review Board database.
    
 
    

    




 
    

    





    
 
    




  
    
 
 
    
  


    
 
    
  
   
    

    Summary
    ID

   
    
      
  

   
    

    

     
    
      Fix the order of SQL statements when creating batches of new models.
      
    When creating models, `EvolveAppTask` was generating and storing the SQL
for an entire model creation, which included not just the `CREATE TABLE`
but any deferred constraints. Each batch of SQL statements would later
be executed in order. If any of these SQL statements contained
constraints that referenced a table introduced by a subsequent task, the
SQL would fail to execute, because that table wouldn't have existed yet.

What's supposed to happen is that the deferred statements are executed
after the tables are created. Hence the "deferred" part. The way the
logic worked, this wasn't possible. `SchemaEditor` knows how to do this
correctly, as does our legacy logic for Django 1.6, but they require
having the full list of new models (not just a subset) passed to them.

This change redoes this logic to gather all models up-front and to
generate the SQL in one go, ensuring that the ordering of statements is
correct. The only real behavioral change introduced from this is that we
have to emit all `creating_models` signals up-front, and then emit all
`created_models` after all models are created, rather than emitting
these in pairs. This means that callers can't expect to receive them in
that order.

It also means that the `evolve` command will be listing all
"Creating new database models for ..." up-front, rather than when each
model will be created. It's an annoyance, but hopefully a mostly
harmless one, and is necessary without a much larger reworking of how
this SQL is generated.
    
     
    

        		
    281c6ac14d4064c0485212b190f20553a1cf34b6

   
    


      
 
    

    
    
 
    

    






 





    
 
 
 
    
  
    
   
    
    
    
     
    
    
    
      
     
    • 
      
     
      • 
     
    • 
      
     
      • 
     
    • 
      
     
      • 
     
    • 
      
     
      • 
     
    • 
      
     
      • 
    
    
   
    
   
    
    
     
    
      Description
      From
      Last Updated
     
    
        
    



     
    
      
       
       
    E303 too many blank lines (2)
    
          		
      
       reviewbotreviewbot
      
      
       
      
     
    

        
   
    
  
    
 
    

    




 
    

    



  
    
 
    

    


    












    
 




 

    


 













    
 





 
    
  
    
   
    



david




   
    
   
    



    
  
    
 
    


 
    
  
    

   
    
   
    
    
     
david


    
    
   
    

  
    
  
    
  
    


      




    1. 
 
      
  
      
   
      Ship It!
      
  
      
 
      

 
      
  


      
 
      
 
        

  

 
      


      

 
      

      2. 















    2. 
 
      
  
      
   
      
  
      
 
      

 
      
  


      
 
      
 
        

  

 
      


      

 
      

      3. 


    

  
    
 
    

    












    
 



 
    
  
    
   
    



chipx86




   
    
   
    
  
    
 
    


 
    
  
    

   
    
   
    
    
     Review request changed
    
    
   
    

  
    
  
    
  
    




    



  
    
   
    
    Change Summary:
   
    
   
    
    
    This is largely rewritten and requires re-review.
    

      

    • Changed back to generating SQL per-app, but also across all apps. This is to cover the "create these immediate models right now for this app" use case (used when creating initial Django Evolution tables) vs. "create for multiple apps" (standard use case).
      • 

    • Added unit tests
      • 

    • Added more compatible index name generation for unit tests for Django 1.7 through 1.10.
      • 

    
   
    
  
    







  
    
   
    
    Commits:
   
    
   
    
    
    



    
 
    
  
   
    

    

    Summary
    ID

   
    
      
  

   
    

    

    

     
    
      Fix the order of SQL statements when creating batches of new models.
      
    When creating models, `EvolveAppTask` was generating and storing the SQL
for an entire model creation, which included not just the `CREATE TABLE`
but any deferred constraints. Each batch of SQL statements would later
be executed in order. If any of these SQL statements contained
constraints that referenced a table introduced by a subsequent task, the
SQL would fail to execute, because that table wouldn't have existed yet.

What's supposed to happen is that the deferred statements are executed
after the tables are created. Hence the "deferred" part. The way the
logic worked, this wasn't possible. `SchemaEditor` knows how to do this
correctly, as does our legacy logic for Django 1.6, but they require
having the full list of new models (not just a subset) passed to them.

This change redoes this logic to gather all models up-front and to
generate the SQL in one go, ensuring that the ordering of statements is
correct. The only real behavioral change introduced from this is that we
have to emit all `creating_models` signals up-front, and then emit all
`created_models` after all models are created, rather than emitting
these in pairs. This means that callers can't expect to receive them in
that order.

It also means that the `evolve` command will be listing all
"Creating new database models for ..." up-front, rather than when each
model will be created. It's an annoyance, but hopefully a mostly
harmless one, and is necessary without a much larger reworking of how
this SQL is generated.
    
     
    

        		
    0db50a53b7c1a2364c49a483b8cac811ea82f1f7

   
    



   
    

    

    

     
    
      Fix the order of SQL statements when creating batches of new models.
      
    When creating models, `EvolveAppTask` was generating and storing the SQL
for an entire model creation, which included not just the `CREATE TABLE`
but any deferred constraints. Each batch of SQL statements would later
be executed in order. If any of these SQL statements contained
constraints that referenced a table introduced by a subsequent task, the
SQL would fail to execute, because that table wouldn't have existed yet.

What's supposed to happen is that the deferred statements are executed
after the tables are created. Hence the "deferred" part. The way the
logic worked, this wasn't possible. `SchemaEditor` knows how to do this
correctly, as does our legacy logic for Django 1.6, but they require
having the full list of new models (not just a subset) passed to them.

This change redoes this logic to gather all models up-front and to
generate the SQL in one go, ensuring that the ordering of statements is
correct. The only real behavioral change introduced from this is that we
have to emit all `creating_models` signals up-front, and then emit all
`created_models` after all models are created, rather than emitting
these in pairs. This means that callers can't expect to receive them in
that order.

It also means that the `evolve` command will be listing all
"Creating new database models for ..." up-front, rather than when each
model will be created. It's an annoyance, but hopefully a mostly
harmless one, and is necessary without a much larger reworking of how
this SQL is generated.
    
     
    

        		
    281c6ac14d4064c0485212b190f20553a1cf34b6

   
    


      
 
    

    


    

   
    
  
    




  
    
   
    
    Diff:
   
    
   
    
    
     Revision 2 (+742 -92)
    Show changes
     
        django_evolution/evolve.py 
        django_evolution/signals.py 
        django_evolution/management/commands/evolve.py 
        django_evolution/tests/test_evolver.py 
        django_evolution/tests/utils.py 
        django_evolution/tests/db/mysql.py 
        django_evolution/tests/db/postgresql.py 
        django_evolution/tests/db/sqlite3.py 
     
    
   
    
  
    





    



    
 
    
  
    Checks run (1 failed, 1 succeeded)
    


  
    
 flake8
 failed.
 


    


  
    
 JSHint
 passed.
 


    


 
    





    
 
    flake8
    
 
    

 

    








    




  
    
 
    

    












    
 





 
    
  
    
   
    



david




   
    
   
    





    
  
    
 
    


 
    
  
    

   
    
   
    
    
     
david


    
    
   
    

  
    
  
    
  
    


      




    1. 
 
      
  
      
   
      Ship It!
      
  
      
 
      

 
      
  


      
 
      
 
        

  

 
      


      

 
      

      2. 















    2. 
 
      
  
      
   
      
  
      
 
      

 
      
  


      
 
      
 
        

  

 
      


      

 
      

      3. 


    

  
    
 
    

    












    
 



 
    
  
    
   
    



chipx86




   
    
   
    
  
    
 
    


 
    
  
    

   
    
   
    
    
     Review request changed
    
    
   
    

  
    
  
    
  
    




    

  
    
   
    
    Status:
   
    
   
    

    Completed

   
    
  
    



  
    
   
    
    Change Summary:
   
    
   
    
    
    Pushed to master (5ebb605)