From b92f683f2dee195cc23c7aa13e973fecd6ab35a7 Mon Sep 17 00:00:00 2001 From: Jason Pellerin Date: Thu, 14 Sep 2006 20:18:24 +0000 Subject: [PATCH] [multi-db] Fixed bugs in handling of pending references. Fixed dropping of test database, and ensured that it drops even if syncdb() fails. git-svn-id: http://code.djangoproject.com/svn/django/branches/multiple-db-support@3760 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/core/management.py | 52 +++++++-------- django/db/backends/ansi/sql.py | 65 +++++++++++-------- django/db/models/manager.py | 10 ++- django/test/simple.py | 11 ++-- django/test/utils.py | 13 ++-- tests/modeltests/multiple_databases/models.py | 15 +++-- tests/regressiontests/ansi_sql/models.py | 4 +- tests/regressiontests/ansi_sql/sql/car.sql | 2 +- .../manager_schema_manipulation/tests.py | 2 +- 9 files changed, 97 insertions(+), 77 deletions(-) diff --git a/django/core/management.py b/django/core/management.py index 9630eaf6bb..cc0fb12c02 100644 --- a/django/core/management.py +++ b/django/core/management.py @@ -73,11 +73,14 @@ def get_sql_create(app): # final output will be divided by comments into sections for each # named connection, if there are any named connections connection_output = {} - pending_references = {} + pending = {} final_output = [] app_models = models.get_models(app, creation_order=True) for model in app_models: + + print "Get create sql for model", model + opts = model._meta connection_name = model_connection_name(model) output = connection_output.setdefault(connection_name, []) @@ -109,21 +112,14 @@ def get_sql_create(app): # table list. tables = [] - installed_models = [ model for model in + installed_models = [ m for m in manager.get_installed_models(tables) - if model not in app_models ] + if m not in app_models ] models_output = set(installed_models) builder = creation.builder builder.models_already_seen.update(models_output) - model_output, references = builder.get_create_table(model, style) + model_output, pending = builder.get_create_table(model, style, pending) output.extend(model_output) - for refto, refs in references.items(): - try: - pending_references[refto].extend(refs) - except KeyError: - pending_references[refto] = refs - if model in pending_references: - output.extend(pending_references.pop(model)) # Create the many-to-many join tables. many_many = builder.get_create_many_to_many(model, style) @@ -131,14 +127,18 @@ def get_sql_create(app): output.extend(statements) final_output = _collate(connection_output) + # Handle references to tables that are from other apps # but don't exist physically - not_installed_models = set(pending_references.keys()) + not_installed_models = set(pending.keys()) if not_installed_models: alter_sql = [] for model in not_installed_models: - alter_sql.extend(['-- ' + sql - for sql in pending_references.pop(model)]) + builder = model._default_manager.db.builder.get_creation_module().builder + + for rel_class, f in pending[model]: + sql = builder._ref_sql(model, rel_class, f, style) + alter_sql.extend(['-- ', str(sql)]) if alter_sql: final_output.append('-- The following references should be added ' 'but depend on non-existent tables:') @@ -406,22 +406,20 @@ def _install(app, commit=True, initial_data=True): models_installed = manager.get_installed_models(tables) # Don't re-install already-installed models if not model in models_installed: - new_pending = manager.install(initial_data=initial_data) + pending = manager.install(initial_data=initial_data, + pending=pending) created_models.append(model) - for dep_model, statements in new_pending.items(): - pending.setdefault(dep_model, []).extend(statements) - # Execute any pending statements that were waiting for this model - if model in pending: - for statement in pending.pop(model): - statement.execute() + if pending: - for model, statements in pending.items(): - manager = model._default_manager - tables = manager.get_table_list() - models_installed = manager.get_installed_models(tables) + models_installed = manager.get_installed_models(tables) + + for model in pending.keys(): if model in models_installed: - for statement in statements: - statement.execute() + for rel_class, f in pending[model]: + manager = model._default_manager + for statement in manager.get_pending(rel_class, f): + statement.execute() + pending.pop(model) else: raise Exception("%s is not installed, but there are " "pending statements that need it: %s" diff --git a/django/db/backends/ansi/sql.py b/django/db/backends/ansi/sql.py index 6298749b1d..d830e9ccb2 100644 --- a/django/db/backends/ansi/sql.py +++ b/django/db/backends/ansi/sql.py @@ -50,7 +50,7 @@ class SchemaBuilder(object): # table cache; set to short-circuit table lookups self.tables = None - def get_create_table(self, model, style=None): + def get_create_table(self, model, style=None, pending=None): """Construct and return the SQL expression(s) needed to create the table for the given model, and any constraints on that table. The return value is a 2-tuple. The first element of the tuple @@ -61,6 +61,8 @@ class SchemaBuilder(object): """ if style is None: style = default_style + if pending is None: + pending = {} self.models_already_seen.add(model) opts = model._meta @@ -70,13 +72,6 @@ class SchemaBuilder(object): data_types = db.get_creation_module().DATA_TYPES table_output = [] - # pending field references, keyed by the model class - # they reference - pending_references = {} - - # pending statements to execute, keyed by - # the model class they reference - pending = {} for f in opts.fields: if isinstance(f, models.ForeignKey): rel_field = f.rel.get_related_field() @@ -108,7 +103,7 @@ class SchemaBuilder(object): else: # We haven't yet created the table to which this field # is related, so save it for later. - pending_references.setdefault(f.rel.to, []).append(f) + pending.setdefault(f.rel.to, []).append((model, f)) table_output.append(' '.join(field_output)) if opts.order_with_respect_to: table_output.append(style.SQL_FIELD(quote_name('_order')) + ' ' + \ @@ -128,24 +123,15 @@ class SchemaBuilder(object): full_statement.append(');') create = [BoundStatement('\n'.join(full_statement), db.connection)] - if (pending_references and + # Pull out any pending statements for me + if (pending and backend.supports_constraints): - for rel_class, cols in pending_references.items(): - for f in cols: - rel_opts = rel_class._meta - r_table = rel_opts.db_table - r_col = f.column - table = opts.db_table - col = opts.get_field(f.rel.field_name).column - # For MySQL, r_name must be unique in the first 64 - # characters. So we are careful with character usage here. - r_name = '%s_refs_%s_%x' % (col, r_col, - abs(hash((r_table, table)))) - sql = style.SQL_KEYWORD('ALTER TABLE') + ' %s ADD CONSTRAINT %s FOREIGN KEY (%s) REFERENCES %s (%s);' % \ - (quote_name(table), quote_name(r_name), - quote_name(r_col), quote_name(r_table), quote_name(col)) - pending.setdefault(rel_class, []).append( - BoundStatement(sql, db.connection)) + if model in pending: + for rel_class, f in pending[model]: + create.append(self.get_ref_sql(model, rel_class, f, + style=style)) + # What was pending for me is now no longer pending + pending.pop(model) return (create, pending) def get_create_indexes(self, model, style=None): @@ -322,7 +308,32 @@ class SchemaBuilder(object): 'PositiveSmallIntegerField')) \ and 'IntegerField' \ or f.get_internal_type() - + + def get_ref_sql(self, model, rel_class, f, style=None): + """Get sql statement for a reference between model and rel_class on + field f. + """ + if style is None: + style = default_style + + db = model._default_manager.db + qn = db.backend.quote_name + opts = model._meta + rel_opts = rel_class._meta + table = rel_opts.db_table + r_col = f.column + r_table = opts.db_table + col = opts.get_field(f.rel.field_name).column + # For MySQL, r_name must be unique in the first 64 + # characters. So we are careful with character usage here. + r_name = '%s_refs_%s_%x' % (col, r_col, + abs(hash((r_table, table)))) + sql = style.SQL_KEYWORD('ALTER TABLE') + \ + ' %s ADD CONSTRAINT %s FOREIGN KEY (%s) REFERENCES %s (%s);' % \ + (qn(table), qn(r_name), + qn(r_col), qn(r_table), qn(col)) + return BoundStatement(sql, db.connection) + def get_references(self): """Fill (if needed) and return the reference cache. """ diff --git a/django/db/models/manager.py b/django/db/models/manager.py index 8100d980f6..4cf667b125 100644 --- a/django/db/models/manager.py +++ b/django/db/models/manager.py @@ -118,7 +118,7 @@ class Manager(object): # SCHEMA MANIPULATION # ####################### - def install(self, initial_data=False): + def install(self, initial_data=False, pending=None): """Install my model's table, indexes and (if requested) initial data. Returns a dict of pending statements, keyed by the model that @@ -127,8 +127,10 @@ class Manager(object): such as foreign key constraints for tables that don't exist at install time.) """ + if pending is None: + pending = {} builder = self.db.get_creation_module().builder - run, pending = builder.get_create_table(self.model) + run, pending = builder.get_create_table(self.model, pending=pending) run += builder.get_create_indexes(self.model) many_many = builder.get_create_many_to_many(self.model) @@ -144,6 +146,10 @@ class Manager(object): self.load_initial_data() return pending + def get_pending(self, rel_class, f): + builder = self.db.get_creation_module().builder + return builder.get_ref_sql(self.model, rel_class, f) + def load_initial_data(self): """Install initial data for model in db, Returns statements executed. """ diff --git a/django/test/simple.py b/django/test/simple.py index 628fa464d2..2c35ae947f 100644 --- a/django/test/simple.py +++ b/django/test/simple.py @@ -78,8 +78,9 @@ def run_tests(module_list, verbosity=1, extra_tests=[]): old_name = settings.DATABASE_NAME create_test_db(verbosity) - management.syncdb(verbosity, interactive=False) - unittest.TextTestRunner(verbosity=verbosity).run(suite) - destroy_test_db(old_name, verbosity) - - teardown_test_environment() + try: + management.syncdb(verbosity, interactive=False) + unittest.TextTestRunner(verbosity=verbosity).run(suite) + finally: + destroy_test_db(old_name, verbosity) + teardown_test_environment() diff --git a/django/test/utils.py b/django/test/utils.py index d7a4a9c963..1a144031ab 100644 --- a/django/test/utils.py +++ b/django/test/utils.py @@ -66,19 +66,19 @@ def create_test_db(verbosity=1, autoclobber=False): cursor = connection.cursor() _set_autocommit(connection) try: - cursor.execute("CREATE DATABASE %s" % qn(db_name)) + cursor.execute("CREATE DATABASE %s" % qn(TEST_DATABASE_NAME)) except Exception, e: sys.stderr.write("Got an error creating the test database: %s\n" % e) if not autoclobber: - confirm = raw_input("It appears the test database, %s, already exists. Type 'yes' to delete it, or 'no' to cancel: " % db_name) + confirm = raw_input("It appears the test database, %s, already exists. Type 'yes' to delete it, or 'no' to cancel: " % TEST_DATABASE_NAME) if autoclobber or confirm == 'yes': try: if verbosity >= 1: print "Destroying old test database..." - cursor.execute("DROP DATABASE %s" % qn(db_name)) + cursor.execute("DROP DATABASE %s" % qn(TEST_DATABASE_NAME)) if verbosity >= 1: print "Creating test database..." - cursor.execute("CREATE DATABASE %s" % qn(db_name)) + cursor.execute("CREATE DATABASE %s" % qn(TEST_DATABASE_NAME)) except Exception, e: sys.stderr.write("Got an error recreating the test database: %s\n" % e) sys.exit(2) @@ -118,12 +118,15 @@ def destroy_test_db(old_database_name, old_databases, verbosity=1): # connected to it. if verbosity >= 1: print "Destroying test database..." - connection.close() + for cnx in connections.keys(): + connections[cnx].close() TEST_DATABASE_NAME = settings.DATABASE_NAME settings.DATABASE_NAME = old_database_name if settings.DATABASE_ENGINE != "sqlite3": settings.OTHER_DATABASES = old_databases + for cnx in connections.keys(): + connections[cnx].connection.cursor() cursor = connection.cursor() _set_autocommit(connection) time.sleep(1) # To avoid "database is being accessed by other users" errors. diff --git a/tests/modeltests/multiple_databases/models.py b/tests/modeltests/multiple_databases/models.py index ce0be8f6c1..97e91b429c 100644 --- a/tests/modeltests/multiple_databases/models.py +++ b/tests/modeltests/multiple_databases/models.py @@ -78,19 +78,20 @@ __test__ = {'API_TESTS': """ >>> from django.db import connection, connections, _default, model_connection_name >>> from django.conf import settings -# The default connection is in there, but let's ignore it +# Connections are referenced by name +>>> connections['_a'] +Connection: ... +>>> connections['_b'] +Connection: ... + +# Let's see what connections are available.The default connection is +# in there, but let's ignore it >>> non_default = connections.keys() >>> non_default.remove(_default) >>> non_default.sort() >>> non_default ['_a', '_b'] - -# Each connection references its settings ->>> connections['_a'].settings.DATABASE_NAME == settings.OTHER_DATABASES['_a']['DATABASE_NAME'] -True ->>> connections['_b'].settings.DATABASE_NAME == settings.OTHER_DATABASES['_b']['DATABASE_NAME'] -True # Invalid connection names raise ImproperlyConfigured >>> connections['bad'] diff --git a/tests/regressiontests/ansi_sql/models.py b/tests/regressiontests/ansi_sql/models.py index 625106f439..63dab57430 100644 --- a/tests/regressiontests/ansi_sql/models.py +++ b/tests/regressiontests/ansi_sql/models.py @@ -20,7 +20,7 @@ Set([]) # test pending relationships >>> builder.models_already_seen = set() >>> builder.get_create_table(Mod) -([BoundStatement('CREATE TABLE "ansi_sql_mod" (..."car_id" integer NOT NULL,...);')], {: [BoundStatement('ALTER TABLE "ansi_sql_mod" ADD CONSTRAINT ... FOREIGN KEY ("car_id") REFERENCES "ansi_sql_car" ("id");')]}) +([BoundStatement('CREATE TABLE "ansi_sql_mod" (..."car_id" integer NOT NULL,...);')], {: [(, )]}) >>> builder.models_already_seen = set() >>> builder.get_create_table(Car) ([BoundStatement('CREATE TABLE "ansi_sql_car" (...);')], {}) @@ -45,7 +45,7 @@ Set([]) # patch builder so that it looks for initial data where we want it to # >>> builder.get_initialdata_path = othertests_sql >>> builder.get_initialdata(Car) -[BoundStatement('insert into ansi_sql_car (...)...values (...);')] +[BoundStatement("insert into ansi_sql_car (...)...values (...);...")] # test drop >>> builder.get_drop_table(Mod) diff --git a/tests/regressiontests/ansi_sql/sql/car.sql b/tests/regressiontests/ansi_sql/sql/car.sql index 8a377aabfb..eaac52a102 100644 --- a/tests/regressiontests/ansi_sql/sql/car.sql +++ b/tests/regressiontests/ansi_sql/sql/car.sql @@ -1,2 +1,2 @@ insert into ansi_sql_car (make, model, year, condition) - values ("Chevy", "Impala", 1966, "mint"); \ No newline at end of file + values ('Chevy', 'Impala', 1966, 'mint'); diff --git a/tests/regressiontests/manager_schema_manipulation/tests.py b/tests/regressiontests/manager_schema_manipulation/tests.py index 56befae7ce..d1f9f1d560 100644 --- a/tests/regressiontests/manager_schema_manipulation/tests.py +++ b/tests/regressiontests/manager_schema_manipulation/tests.py @@ -123,7 +123,7 @@ >>> PA._default_manager.db.backend.supports_constraints = True >>> result = PA.objects.install() >>> result -{: [BoundStatement('ALTER TABLE "msm_pa" ADD CONSTRAINT "id_refs_c_id..." FOREIGN KEY ("c_id") REFERENCES "msm_pc" ("id");')]} +{: [(, )]} # NOTE: restore real constraint flag >>> PA._default_manager.db.backend.supports_constraints = real_cnst