Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: enable support for get_key_columns and cleanup tests with unknown failures to specific failures. #721

Merged
merged 12 commits into from Oct 13, 2021
Merged
2 changes: 1 addition & 1 deletion README.rst
Expand Up @@ -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>`_.
28 changes: 8 additions & 20 deletions django_spanner/features.py
Expand Up @@ -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.
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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:
Expand Down
35 changes: 30 additions & 5 deletions django_spanner/introspection.py
Expand Up @@ -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.

Expand Down Expand Up @@ -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
vi3k6i5 marked this conversation as resolved.
Show resolved Hide resolved
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
17 changes: 14 additions & 3 deletions django_spanner/schema.py
Expand Up @@ -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):
"""
Expand Down Expand Up @@ -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(
Expand Down