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

Author: vi3k6i5

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 <https://cloud.google.com/spanner/quotas#schema_limits>`_
 It is recommended that you go through that list.
 
-Django spanner has a set of limitations as well, please go through the `list <https://googleapis.dev/python/django-google-spanner/latest/limitations.html>`_.
+Django spanner has a set of limitations as well, please go through the `list <https://github.com/googleapis/python-spanner-django/blob/main/docs/limitations.rst>`_.

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:

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

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(