Skip to content

django: add support for FOREIGN KEY constraints #313

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

Closed
odeke-em opened this issue Mar 6, 2020 · 4 comments
Closed

django: add support for FOREIGN KEY constraints #313

odeke-em opened this issue Mar 6, 2020 · 4 comments
Assignees
Labels
api: spanner Issues related to the googleapis/python-spanner-django API. blocked Issues that cannot be implemented because of some limit priority: p2 Moderately-important priority. Fix may not be included in next release.

Comments

@odeke-em
Copy link
Contributor

odeke-em commented Mar 6, 2020

On Thu 5 Mar 2020, Cloud Spanner announced support for FOREIGN KEYS as per https://cloud.google.com/spanner/docs/release-notes and https://cloud.google.com/spanner/docs/foreign-keys/overview

We need to add support for FOREIGN KEYS now, perhaps per https://github.com/orijtech/django-spanner/blob/189b71de3ab868c4924708396086843db6a6e506/spanner/django/features.py#L13

and then enable the tests that assumed no FOREIGN KEY support.

@odeke-em odeke-em changed the title django: add support for FOREIGN KEYS now that Cloud Spanner supports them django: add support for FOREIGN KEY constraint now that Cloud Spanner supports it Mar 6, 2020
@odeke-em
Copy link
Contributor Author

odeke-em commented Mar 6, 2020

As per https://cloud.google.com/spanner/docs/foreign-keys/overview#how-to-define-foreign-keys this is a little more involved because other SQL as there seem to be 2 flavors:
a) named CONSTRAINT so CONSTRAINT FK_CustomerOrder FOREIGN KEY (CustomerID) REFERENCES Customers (CustomerID)
b) unnamed CONSTRAINT so FOREIGN KEY (CustomerID) REFERENCES Customers (CustomerID)

but the ALTER statements are familiar

though it DOESN'T support CASCADE.

@timgraham
Copy link
Contributor

I'll start looking at this but it's not the sort of change we can merge until the build is green as I don't have much confidence it won't break something unexpected.

At first glance, I see tracebacks like this:

======================================================================
ERROR: test_add_after_prefetch (many_to_one.tests.ManyToOneTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tim/.virtualenvs/django37/lib/python3.7/site-packages/google/api_core/grpc_helpers.py", line 57, in error_remapped_callable
    return callable_(*args, **kwargs)
  File "/home/tim/.virtualenvs/django37/lib/python3.7/site-packages/grpc/_channel.py", line 690, in __call__
    return _end_unary_response_blocking(state, call, False, None)
  File "/home/tim/.virtualenvs/django37/lib/python3.7/site-packages/grpc/_channel.py", line 592, in _end_unary_response_blocking
    raise _Rendezvous(state, None, None, deadline)
grpc._channel._Rendezvous: <_Rendezvous of RPC that terminated with:
	status = StatusCode.FAILED_PRECONDITION
	details = "Foreign key constraint violation when deleting or updating referenced row(s): referencing row(s) found in table `many_to_one_district`."
	debug_error_string = "{"created":"@1583460182.610622984","description":"Error received from peer ipv4:172.217.10.106:443","file":"src/core/lib/surface/call.cc","file_line":1055,"grpc_message":"Foreign key constraint violation when deleting or updating referenced row(s): referencing row(s) found in table `many_to_one_district`.","grpc_status":9}"
>

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/tim/code/spanner-orm/spanner/dbapi/cursor.py", line 135, in execute
    self.__handle_update(self.__get_txn(), sql, args or None)
  File "/home/tim/code/spanner-orm/spanner/dbapi/cursor.py", line 214, in __handle_update
    res = txn.execute_update(sql, params=params, param_types=get_param_types(params))
  File "/home/tim/.virtualenvs/django37/lib/python3.7/site-packages/google/cloud/spanner_v1/transaction.py", line 202, in execute_update
    metadata=metadata,
  File "/home/tim/.virtualenvs/django37/lib/python3.7/site-packages/google/cloud/spanner_v1/gapic/spanner_client.py", line 812, in execute_sql
    request, retry=retry, timeout=timeout, metadata=metadata
  File "/home/tim/.virtualenvs/django37/lib/python3.7/site-packages/google/api_core/gapic_v1/method.py", line 143, in __call__
    return wrapped_func(*args, **kwargs)
  File "/home/tim/.virtualenvs/django37/lib/python3.7/site-packages/google/api_core/retry.py", line 277, in retry_wrapped_func
    on_error=on_error,
  File "/home/tim/.virtualenvs/django37/lib/python3.7/site-packages/google/api_core/retry.py", line 182, in retry_target
    return target()
  File "/home/tim/.virtualenvs/django37/lib/python3.7/site-packages/google/api_core/timeout.py", line 214, in func_with_timeout
    return func(*args, **kwargs)
  File "/home/tim/.virtualenvs/django37/lib/python3.7/site-packages/google/api_core/grpc_helpers.py", line 59, in error_remapped_callable
    six.raise_from(exceptions.from_grpc_error(exc), exc)
  File "<string>", line 3, in raise_from
google.api_core.exceptions.FailedPrecondition: 400 Foreign key constraint violation when deleting or updating referenced row(s): referencing row(s) found in table `many_to_one_district`.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/tim/code/django/django/db/backends/utils.py", line 82, in _execute
    return self.cursor.execute(sql)
  File "/home/tim/code/spanner-orm/spanner/dbapi/cursor.py", line 144, in execute
    raise IntegrityError(e.details if hasattr(e, 'details') else e)
spanner.dbapi.exceptions.IntegrityError: 400 Foreign key constraint violation when deleting or updating referenced row(s): referencing row(s) found in table `many_to_one_district`.

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/tim/code/django/django/core/management/commands/flush.py", line 63, in handle
    connection.ops.execute_sql_flush(database, sql_list)
  File "/home/tim/code/django/django/db/backends/base/operations.py", line 404, in execute_sql_flush
    cursor.execute(sql)
  File "/home/tim/code/django/django/db/backends/utils.py", line 67, in execute
    return self._execute_with_wrappers(sql, params, many=False, executor=self._execute)
  File "/home/tim/code/django/django/db/backends/utils.py", line 76, in _execute_with_wrappers
    return executor(sql, params, many, context)
  File "/home/tim/code/django/django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)
  File "/home/tim/code/django/django/db/utils.py", line 89, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "/home/tim/code/django/django/db/backends/utils.py", line 82, in _execute
    return self.cursor.execute(sql)
  File "/home/tim/code/spanner-orm/spanner/dbapi/cursor.py", line 144, in execute
    raise IntegrityError(e.details if hasattr(e, 'details') else e)
django.db.utils.IntegrityError: 400 Foreign key constraint violation when deleting or updating referenced row(s): referencing row(s) found in table `many_to_one_district`.

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/tim/code/django/django/test/testcases.py", line 274, in __call__
    self._post_teardown()
  File "/home/tim/code/django/django/test/testcases.py", line 1009, in _post_teardown
    self._fixture_teardown()
  File "/home/tim/code/django/django/test/testcases.py", line 1173, in _fixture_teardown
    return super()._fixture_teardown()
  File "/home/tim/code/django/django/test/testcases.py", line 1044, in _fixture_teardown
    inhibit_post_migrate=inhibit_post_migrate)
  File "/home/tim/code/django/django/core/management/__init__.py", line 148, in call_command
    return command.execute(*args, **defaults)
  File "/home/tim/code/django/django/core/management/base.py", line 364, in execute
    output = self.handle(*args, **options)
  File "/home/tim/code/django/django/core/management/commands/flush.py", line 74, in handle
    ) from exc
django.core.management.base.CommandError: Database test_westbrook1 couldn't be flushed. Possible reasons:
  * The database isn't running or isn't configured correctly.
  * At least one of the expected database tables doesn't exist.
  * The SQL was invalid.
Hint: Look at the output of 'django-admin sqlflush'. That's the SQL this command wasn't able to run.

I haven't investigated in detail, but I think it suggests that when flushing the database after a test, we have to know the order in which to delete rows so as not to cause a foreign constraint violation. I don't think this is possible so if there's no DELETE CASCADE support, that may be a blocker.

@odeke-em
Copy link
Contributor Author

odeke-em commented Mar 6, 2020

@bvandiver @skuruppu please take a look at @timgraham's postulation

I haven't investigated in detail, but I think it suggests that when flushing the database after a test, we have to know the order in which to delete rows so as not to cause a foreign constraint violation. I don't think this is possible so if there's no DELETE CASCADE support, that may be a blocker.

@timgraham I think that as this is being investigated, please don't get blocked, you can keep working on other things like the unexplored Django features.

odeke-em added a commit that referenced this issue Mar 10, 2020
…elations

Implemented:
* DatabaseIntrospection.get_primary_key_column
* DatabaseIntrospection.get_relations

However, get_relations tests CANNOT yet be enabled because
Cloud Spanner doesn't yet have ON DELETE cascading for
FOREIGN KEYS which limits #313, which renders the method
useless as it will always return {} instead of being populated
with values that are derived from FOREIGN KEYs.

Fixes #312
Updates #311
odeke-em added a commit that referenced this issue Mar 10, 2020
…elations

Implemented:
* DatabaseIntrospection.get_primary_key_column
* DatabaseIntrospection.get_relations

However, get_relations tests CANNOT yet be enabled because
Cloud Spanner doesn't yet have ON DELETE cascading for
FOREIGN KEYS which limits #313, which renders the method
useless as it will always return {} instead of being populated
with values that are derived from FOREIGN KEYs.

Fixes #312
Updates #311
odeke-em added a commit that referenced this issue Mar 10, 2020
…elations

Implemented:
* DatabaseIntrospection.get_primary_key_column
* DatabaseIntrospection.get_relations

However, get_relations tests CANNOT yet be enabled because
Cloud Spanner doesn't yet have ON DELETE cascading for
FOREIGN KEYS which limits #313, which renders the method
useless as it will always return {} instead of being populated
with values that are derived from FOREIGN KEYs.

Fixes #312
Updates #311
odeke-em added a commit that referenced this issue Mar 10, 2020
…elations

Implemented:
* DatabaseIntrospection.get_primary_key_column
* DatabaseIntrospection.get_relations

However, get_relations tests CANNOT yet be enabled because
Cloud Spanner doesn't yet have ON DELETE cascading for
FOREIGN KEYS which limits #313, which renders the method
useless as it will always return {} instead of being populated
with values that are derived from FOREIGN KEYs.

Fixes #312
Updates #311
@timgraham timgraham changed the title django: add support for FOREIGN KEY constraint now that Cloud Spanner supports it django: add support for FOREIGN KEY constraints Mar 11, 2020
odeke-em added a commit that referenced this issue Mar 12, 2020
…elations

Implemented:
* DatabaseIntrospection.get_primary_key_column
* DatabaseIntrospection.get_relations

However, get_relations tests CANNOT yet be enabled because
Cloud Spanner doesn't yet have ON DELETE cascading for
FOREIGN KEYS which limits #313, which renders the method
useless as it will always return {} instead of being populated
with values that are derived from FOREIGN KEYs.

Fixes #312
Updates #311
odeke-em added a commit that referenced this issue Mar 13, 2020
…elations

Implemented:
* DatabaseIntrospection.get_primary_key_column
* DatabaseIntrospection.get_relations

However, get_relations tests CANNOT yet be enabled because
Cloud Spanner doesn't yet have ON DELETE cascading for
FOREIGN KEYS which limits #313, which renders the method
useless as it will always return {} instead of being populated
with values that are derived from FOREIGN KEYs.

Fixes #312
Updates #311
@odeke-em odeke-em added the blocked Issues that cannot be implemented because of some limit label Mar 13, 2020
@timgraham timgraham removed their assignment Mar 20, 2020
timgraham added a commit that referenced this issue Mar 24, 2020
It disables parts of tests that don't apply until support for
foriegn keys is added (refs #313).
timgraham added a commit that referenced this issue Mar 24, 2020
It disables parts of tests that don't apply until support for
foriegn keys is added (refs #313).
@yoshi-automation yoshi-automation added 🚨 This issue needs some love. triage me I really want to be triaged. labels Apr 7, 2020
@JustinBeckwith JustinBeckwith added the api: spanner Issues related to the googleapis/python-spanner-django API. label Apr 8, 2020
@odeke-em odeke-em added priority: p2 Moderately-important priority. Fix may not be included in next release. and removed triage me I really want to be triaged. labels Apr 16, 2020
@yoshi-automation yoshi-automation removed the 🚨 This issue needs some love. label Apr 16, 2020
@vi3k6i5 vi3k6i5 closed this as completed Jul 1, 2021
@vi3k6i5 vi3k6i5 reopened this Jul 1, 2021
@vi3k6i5 vi3k6i5 self-assigned this Oct 11, 2021
@asthamohta
Copy link
Contributor

This is a documented limitation that we do not plan on taking up any time soon, hence closing the issue:
https://github.com/googleapis/python-spanner-django/blob/main/docs/limitations.rst

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the googleapis/python-spanner-django API. blocked Issues that cannot be implemented because of some limit priority: p2 Moderately-important priority. Fix may not be included in next release.
Projects
None yet
Development

No branches or pull requests

6 participants