diff --git a/.gitignore b/.gitignore index cc979da25..7b6b58d7d 100644 --- a/.gitignore +++ b/.gitignore @@ -24,4 +24,5 @@ notebook .vscode __main__.py jupyter_custom.js -apk_requirements.txt \ No newline at end of file +apk_requirements.txt +.eggs \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b23df8eb..35e78375a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ ## Release notes -### 0.13.0 -- Mar 19, 2021 +### 0.13.0 -- Mar 24, 2021 * Re-implement query transpilation into SQL, fixing issues (#386, #449, #450, #484). PR #754 * Re-implement cascading deletes for better performance. PR #839. * Add table method `.update1` to update a row in the table with new values PR #763 @@ -11,6 +11,7 @@ * Default enable_python_native_blobs to True * Bugfix - Regression error on joins with same attribute name (#857) PR #878 * Bugfix - Error when `fetch1('KEY')` when `dj.config['fetch_format']='frame'` set (#876) PR #880, #878 +* Bugfix - Error when cascading deletes in tables with many, complex keys (#883, #886) PR #839 * Drop support for Python 3.5 ### 0.12.8 -- Jan 12, 2021 diff --git a/LNX-docker-compose.yml b/LNX-docker-compose.yml index b7edad942..c91f4c01e 100644 --- a/LNX-docker-compose.yml +++ b/LNX-docker-compose.yml @@ -32,7 +32,7 @@ services: interval: 1s fakeservices.datajoint.io: <<: *net - image: datajoint/nginx:v0.0.15 + image: datajoint/nginx:v0.0.16 environment: - ADD_db_TYPE=DATABASE - ADD_db_ENDPOINT=db:3306 diff --git a/datajoint/table.py b/datajoint/table.py index 36319b5d0..53da2dc3c 100644 --- a/datajoint/table.py +++ b/datajoint/table.py @@ -15,16 +15,27 @@ from . import blob from .utils import user_choice from .heading import Heading -from .errors import DuplicateError, AccessError, DataJointError, UnknownAttributeError, IntegrityError +from .errors import (DuplicateError, AccessError, DataJointError, UnknownAttributeError, + IntegrityError) from .version import __version__ as version logger = logging.getLogger(__name__) -foregn_key_error_regexp = re.compile( +foreign_key_error_regexp = re.compile( r"[\w\s:]*\((?P`[^`]+`.`[^`]+`), " r"CONSTRAINT (?P`[^`]+`) " - r"FOREIGN KEY \((?P[^)]+)\) " - r"REFERENCES (?P`[^`]+`(\.`[^`]+`)?) \((?P[^)]+)\)") + r"(FOREIGN KEY \((?P[^)]+)\) " + r"REFERENCES (?P`[^`]+`(\.`[^`]+`)?) \((?P[^)]+)\)[\s\w]+\))?") + +constraint_info_query = ' '.join(""" + SELECT + COLUMN_NAME as fk_attrs, + CONCAT('`', REFERENCED_TABLE_SCHEMA, '`.`', REFERENCED_TABLE_NAME, '`') as parent, + REFERENCED_COLUMN_NAME as pk_attrs + FROM INFORMATION_SCHEMA.KEY_COLUMN_USAGE + WHERE + CONSTRAINT_NAME = %s AND TABLE_SCHEMA = %s AND TABLE_NAME = %s; + """.split()) class _RenameMap(tuple): @@ -344,23 +355,31 @@ def _delete_cascade(self): try: delete_count += self.delete_quick(get_count=True) except IntegrityError as error: - match = foregn_key_error_regexp.match(error.args[0]) - assert match is not None, "foreign key parsing error" + match = foreign_key_error_regexp.match(error.args[0]).groupdict() + if "`.`" not in match['child']: # if schema name missing, use self + match['child'] = '{}.{}'.format(self.full_table_name.split(".")[0], + match['child']) + if match['pk_attrs'] is not None: # fully matched, adjusting the keys + match['fk_attrs'] = [k.strip('`') for k in match['fk_attrs'].split(',')] + match['pk_attrs'] = [k.strip('`') for k in match['pk_attrs'].split(',')] + else: # only partially matched, querying with constraint to determine keys + match['fk_attrs'], match['parent'], match['pk_attrs'] = list(map( + list, zip(*self.connection.query(constraint_info_query, args=( + match['name'].strip('`'), + *[_.strip('`') for _ in match['child'].split('`.`')] + )).fetchall()))) + match['parent'] = match['parent'][0] # restrict child by self if # 1. if self's restriction attributes are not in child's primary key # 2. if child renames any attributes # otherwise restrict child by self's restriction. - child = match.group('child') - if "`.`" not in child: # if schema name is not included, take it from self - child = self.full_table_name.split("`.")[0] + child - child = FreeTable(self.connection, child) + child = FreeTable(self.connection, match['child']) if set(self.restriction_attributes) <= set(child.primary_key) and \ - match.group('fk_attrs') == match.group('pk_attrs'): + match['fk_attrs'] == match['pk_attrs']: child._restriction = self._restriction - elif match.group('fk_attrs') != match.group('pk_attrs'): - fk_attrs = [k.strip('`') for k in match.group('fk_attrs').split(',')] - pk_attrs = [k.strip('`') for k in match.group('pk_attrs').split(',')] - child &= self.proj(**dict(zip(fk_attrs, pk_attrs))) + elif match['fk_attrs'] != match['pk_attrs']: + child &= self.proj(**dict(zip(match['fk_attrs'], + match['pk_attrs']))) else: child &= self.proj() delete_count += child._delete_cascade() @@ -375,8 +394,10 @@ def _delete_cascade(self): def delete(self, transaction=True, safemode=None): """ Deletes the contents of the table and its dependent tables, recursively. + :param transaction: if True, use the entire delete becomes an atomic transaction. - :param safemode: If True, prohibit nested transactions and prompt to confirm. Default is dj.config['safemode']. + :param safemode: If True, prohibit nested transactions and prompt to confirm. Default + is dj.config['safemode']. """ safemode = config['safemode'] if safemode is None else safemode diff --git a/datajoint/version.py b/datajoint/version.py index 91ba72298..4ac209b43 100644 --- a/datajoint/version.py +++ b/datajoint/version.py @@ -1,3 +1,3 @@ -__version__ = "0.13.dev6" +__version__ = "0.13.dev7" assert len(__version__) <= 10 # The log table limits version to the 10 characters diff --git a/docs-parts/intro/Releases_lang1.rst b/docs-parts/intro/Releases_lang1.rst index db326b405..36ad0c8f5 100644 --- a/docs-parts/intro/Releases_lang1.rst +++ b/docs-parts/intro/Releases_lang1.rst @@ -1,4 +1,4 @@ -0.13.0 -- Mar 19, 2021 +0.13.0 -- Mar 24, 2021 ---------------------- * Re-implement query transpilation into SQL, fixing issues (#386, #449, #450, #484). PR #754 * Re-implement cascading deletes for better performance. PR #839. @@ -10,6 +10,7 @@ * Default enable_python_native_blobs to True * Bugfix - Regression error on joins with same attribute name (#857) PR #878 * Bugfix - Error when `fetch1('KEY')` when `dj.config['fetch_format']='frame'` set (#876) PR #880, #878 +* Bugfix - Error when cascading deletes in tables with many, complex keys (#883, #886) PR #839 * Drop support for Python 3.5 0.12.8 -- Jan 12, 2021 diff --git a/local-docker-compose.yml b/local-docker-compose.yml index 9831c29c9..ff0dda0e6 100644 --- a/local-docker-compose.yml +++ b/local-docker-compose.yml @@ -34,7 +34,7 @@ services: interval: 1s fakeservices.datajoint.io: <<: *net - image: datajoint/nginx:v0.0.15 + image: datajoint/nginx:v0.0.16 environment: - ADD_db_TYPE=DATABASE - ADD_db_ENDPOINT=db:3306 diff --git a/tests/schema.py b/tests/schema.py index e2d29f917..1fd187637 100644 --- a/tests/schema.py +++ b/tests/schema.py @@ -366,3 +366,16 @@ class Child(dj.Lookup): name: varchar(30) """ contents = [(1, 12, 'Dan')] + +# Related to issue #886 (8), #883 (5) +@schema +class ComplexParent(dj.Lookup): + definition = '\n'.join(['parent_id_{}: int'.format(i+1) for i in range(8)]) + contents = [tuple(i for i in range(8))] + + +@schema +class ComplexChild(dj.Lookup): + definition = '\n'.join(['-> ComplexParent'] + ['child_id_{}: int'.format(i+1) + for i in range(1)]) + contents = [tuple(i for i in range(9))] diff --git a/tests/test_cascading_delete.py b/tests/test_cascading_delete.py index 76f91b998..6988d432a 100644 --- a/tests/test_cascading_delete.py +++ b/tests/test_cascading_delete.py @@ -1,6 +1,7 @@ from nose.tools import assert_false, assert_true, assert_equal import datajoint as dj from .schema_simple import A, B, D, E, L +from .schema import ComplexChild, ComplexParent class TestDelete: @@ -78,3 +79,20 @@ def test_delete_lookup_restricted(): deleted_count = len(rel) rel.delete() assert_true(len(L()) == original_count - deleted_count) + + @staticmethod + def test_delete_complex_keys(): + # https://github.com/datajoint/datajoint-python/issues/883 + # https://github.com/datajoint/datajoint-python/issues/886 + assert_false(dj.config['safemode'], 'safemode must be off for testing') + parent_key_count = 8 + child_key_count = 1 + restriction = dict({'parent_id_{}'.format(i+1): i + for i in range(parent_key_count)}, + **{'child_id_{}'.format(i+1): (i + parent_key_count) + for i in range(child_key_count)}) + assert len(ComplexParent & restriction) == 1, 'Parent record missing' + assert len(ComplexChild & restriction) == 1, 'Child record missing' + (ComplexParent & restriction).delete() + assert len(ComplexParent & restriction) == 0, 'Parent record was not deleted' + assert len(ComplexChild & restriction) == 0, 'Child record was not deleted'