From 1ec07849787dfcfeda7206e038e0a63c0b45d74c Mon Sep 17 00:00:00 2001 From: Vikash Singh <3116482+vi3k6i5@users.noreply.github.com> Date: Wed, 13 Oct 2021 20:14:51 +0530 Subject: [PATCH] feat: enable support for `get_key_columns` and cleanup tests with unknown failures to specific failures. (#721) * docs: lint fix for samples * feat: enable foreign key support * feat: add support for spanner fk * feat: disable foreign key as without on delete cascade the db cleanup process fails * fix: add comment as to why foreign key creation is disabled in django * fix: move test_check_constraints_sql_keywords to django3.2 skipped tests * chor: cleanup tests * tests: add failing tests in skipped test --- README.rst | 2 +- django_spanner/features.py | 28 ++++++++------------------ django_spanner/introspection.py | 35 ++++++++++++++++++++++++++++----- django_spanner/schema.py | 17 +++++++++++++--- 4 files changed, 53 insertions(+), 29 deletions(-) diff --git a/README.rst b/README.rst index 4f0d9ca886..93f7116271 100644 --- a/README.rst +++ b/README.rst @@ -260,4 +260,4 @@ LIMITATIONS Spanner has certain limitations of it's own and a full set of limitations are documented over `here `_ It is recommended that you go through that list. -Django spanner has a set of limitations as well, please go through the `list `_. +Django spanner has a set of limitations as well, please go through the `list `_. diff --git a/django_spanner/features.py b/django_spanner/features.py index 87abddf87f..d2099a78f0 100644 --- a/django_spanner/features.py +++ b/django_spanner/features.py @@ -61,11 +61,12 @@ class DatabaseFeatures(BaseDatabaseFeatures): # Django tests that aren't supported by Spanner. skip_tests = ( - # No foreign key constraints in Spanner. + # Spanner does not support very long FK name: 400 Foreign Key name not valid "backends.tests.FkConstraintsTests.test_check_constraints", + # No foreign key ON DELETE CASCADE in Spanner. + "fixtures_regress.tests.TestFixtures.test_loaddata_raises_error_when_fixture_has_invalid_foreign_key", # Spanner does not support empty list of DML statement. "backends.tests.BackendTestCase.test_cursor_executemany_with_empty_params_list", - "fixtures_regress.tests.TestFixtures.test_loaddata_raises_error_when_fixture_has_invalid_foreign_key", # No Django transaction management in Spanner. "basic.tests.SelectOnSaveTests.test_select_on_save_lying_update", # django_spanner monkey patches AutoField to have a default value. @@ -119,6 +120,7 @@ class DatabaseFeatures(BaseDatabaseFeatures): "validation.test_validators.TestModelsWithValidators.test_custom_validator_raises_error_for_incorrect_value", "validation.test_validators.TestModelsWithValidators.test_field_validators_can_be_any_iterable", # Tests that assume a serial pk. + "servers.tests.LiveServerDatabase.test_fixtures_loaded", "admin_filters.tests.ListFiltersTests.test_booleanfieldlistfilter_nullbooleanfield", "admin_filters.tests.ListFiltersTests.test_booleanfieldlistfilter_tuple", "admin_filters.tests.ListFiltersTests.test_booleanfieldlistfilter", @@ -283,12 +285,6 @@ class DatabaseFeatures(BaseDatabaseFeatures): "transaction_hooks.tests.TestConnectionOnCommit.test_inner_savepoint_does_not_affect_outer", # No sequence for AutoField in Spanner. "introspection.tests.IntrospectionTests.test_sequence_list", - # DatabaseIntrospection.get_key_columns() is only required if this - # backend needs it (which it currently doesn't). - "introspection.tests.IntrospectionTests.test_get_key_columns", - # DatabaseIntrospection.get_relations() isn't implemented: - # https://github.com/googleapis/python-spanner-django/issues/311 - "introspection.tests.IntrospectionTests.test_get_relations", # pyformat parameters not supported on INSERT: # https://github.com/googleapis/python-spanner-django/issues/343 "backends.tests.BackendTestCase.test_cursor_execute_with_pyformat", @@ -375,19 +371,15 @@ class DatabaseFeatures(BaseDatabaseFeatures): "model_forms.tests.UniqueTest.test_override_unique_together_message", # os.chmod() doesn't work on Kokoro? "file_uploads.tests.DirectoryCreationTests.test_readonly_root", - # Tests that sometimes fail on Kokoro for unknown reasons. + # Failing on kokoro but passes locally. Issue: Multiple queries executed expected 1. "contenttypes_tests.test_models.ContentTypesTests.test_cache_not_shared_between_managers", - "migration_test_data_persistence.tests.MigrationDataNormalPersistenceTestCase.test_persistence", - "servers.test_liveserverthread.LiveServerThreadTest.test_closes_connections", - "servers.tests.LiveServerDatabase.test_fixtures_loaded", - "view_tests.tests.test_csrf.CsrfViewTests.test_no_cookies", - "view_tests.tests.test_csrf.CsrfViewTests.test_no_referer", - "view_tests.tests.test_i18n.SetLanguageTests.test_lang_from_translated_i18n_pattern", ) if USING_DJANGO_3: skip_tests += ( # Spanner does not support UUID field natively "model_fields.test_uuid.TestQuerying.test_iexact", + # Spanner does not support very long FK name: 400 Foreign Key name not valid + "backends.tests.FkConstraintsTests.test_check_constraints_sql_keywords", # Spanner does not support setting a default value on columns. "schema.tests.SchemaTests.test_alter_text_field_to_not_null_with_default_value", # Direct SQL query test that do not follow spanner syntax. @@ -452,8 +444,6 @@ class DatabaseFeatures(BaseDatabaseFeatures): # Spanner does not support SELECTing an arbitrary expression that also # appears in the GROUP BY clause. "annotations.tests.NonAggregateAnnotationTestCase.test_grouping_by_q_expression_annotation", - # No foreign key constraints in Spanner. - "backends.tests.FkConstraintsTests.test_check_constraints_sql_keywords", # No Django transaction management in Spanner. "transactions.tests.DisableDurabiltityCheckTests.test_nested_both_durable", "transactions.tests.DisableDurabiltityCheckTests.test_nested_inner_durable", @@ -489,9 +479,7 @@ class DatabaseFeatures(BaseDatabaseFeatures): "schema.tests.SchemaTests.test_ci_cs_db_collation", # Spanner limitation: Cannot rename tables and columns. "migrations.test_operations.OperationTests.test_rename_field_case", - # Tests that sometimes fail on Kokoro for unknown reasons. - "migrations.test_operations.OperationTests.test_add_constraint_combinable", - # Tests that fail but are not related to spanner. + # Warning is not raised, not related to spanner. "test_utils.test_testcase.TestDataTests.test_undeepcopyable_warning", ) else: diff --git a/django_spanner/introspection.py b/django_spanner/introspection.py index 938718d2d5..4861751acf 100644 --- a/django_spanner/introspection.py +++ b/django_spanner/introspection.py @@ -150,11 +150,6 @@ def get_relations(self, cursor, table_name): """Return a dictionary of {field_name: (field_name_other_table, other_table)} representing all the relationships in the table. - TODO: DO NOT USE THIS METHOD UNTIL - https://github.com/googleapis/python-spanner-django/issues/313 - is resolved so that foreign keys can be supported, as documented in: - https://github.com/googleapis/python-spanner-django/issues/311 - :type cursor: :class:`~google.cloud.spanner_dbapi.cursor.Cursor` :param cursor: A reference to a Spanner Database cursor. @@ -348,3 +343,33 @@ def get_constraints(self, cursor, table_name): constraints[index_name]["unique"] = is_unique return constraints + + def get_key_columns(self, cursor, table_name): + """ + Return a list of (column_name, referenced_table, referenced_column) + for all key columns in the given table. + """ + key_columns = [] + cursor.execute( + """SELECT + tc.COLUMN_NAME as column_name, + ccu.TABLE_NAME as referenced_table, + ccu.COLUMN_NAME as referenced_column + from + INFORMATION_SCHEMA.KEY_COLUMN_USAGE AS tc + JOIN + INFORMATION_SCHEMA.REFERENTIAL_CONSTRAINTS as rc + ON + tc.CONSTRAINT_NAME = rc.CONSTRAINT_NAME + JOIN + INFORMATION_SCHEMA.CONSTRAINT_COLUMN_USAGE as ccu + ON + rc.CONSTRAINT_NAME = ccu.CONSTRAINT_NAME + WHERE + tc.TABLE_NAME="{table}" + """.format( + table=self.connection.ops.quote_name(table_name) + ) + ) + key_columns.extend(cursor.fetchall()) + return key_columns diff --git a/django_spanner/schema.py b/django_spanner/schema.py index 4f40f0574d..2ef4ea7d24 100644 --- a/django_spanner/schema.py +++ b/django_spanner/schema.py @@ -41,6 +41,10 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor): sql_alter_column_type = "ALTER COLUMN %(column)s %(type)s" sql_delete_column = "ALTER TABLE %(table)s DROP COLUMN %(column)s" + # Spanner does not suppport ON DELETE CASCADE for foreign keys. + # This can cause failures in django, hence sql_create_inline_fk is disabled. + # sql_create_inline_fk = "CONSTRAINT FK_%(to_table)s_%(to_column)s_%(from_table)s_%(from_column)s FOREIGN KEY (%(from_column_norm)s) REFERENCES %(to_table_norm)s (%(to_column_norm)s)" # noqa + sql_create_inline_fk = None def create_model(self, model): """ @@ -77,14 +81,21 @@ def create_model(self, model): params.extend(extra_params) # FK if field.remote_field and field.db_constraint: + from_table = field.model._meta.db_table + from_column = field.column to_table = field.remote_field.model._meta.db_table to_column = field.remote_field.model._meta.get_field( field.remote_field.field_name ).column if self.sql_create_inline_fk: - definition += " " + self.sql_create_inline_fk % { - "to_table": self.quote_name(to_table), - "to_column": self.quote_name(to_column), + definition += ", " + self.sql_create_inline_fk % { + "from_table": from_table, + "from_column": from_column, + "to_table": to_table, + "to_column": to_column, + "from_column_norm": self.quote_name(from_column), + "to_table_norm": self.quote_name(to_table), + "to_column_norm": self.quote_name(to_column), } elif self.connection.features.supports_foreign_keys: self.deferred_sql.append(