Cascading delete fails with non-congruent non-pk foreign refs

[eywalker]

Follow set of tables lead to incorrect cascading delete:

Image

%{
test.Image (manual) # collection of all image data
image_id          :int          #unique image id
-----
image_data=null   :longblob     #image data
%}
classdef Image < dj.Relvar
end

MonkeyImage

%{
test.MonkeyImage (manual) # collection of monkey images
subj_id         :int      #unique id of the monkey
-----
-> test.Image
%}
classdef MonkeyImage < dj.Relvar
end

ModifiedMonkeyImage

%{
test.ModifiedMonkeyImage (manual) # collection of modified monkey images
-> test.MonkeyImage
modification_type    :varchar(255)      #type of modification
-----
-> test.Image
%}
classdef ModifiedMonkeyImage < dj.Relvar
end

In the above three tables, Image is where all the images are actually stored as longblob, with each image given a unique image_id. MonkeyImage is a convenient collection of all images for monkeys, with unique entry for each monkey identified by subj_id. Finally, ModifiedMonkeyImage is a collection of modified images for the monkey, uniquely identified by the subj_id of the monkey as the modification_type descriptor. Note that although ModifiedMonkeyImage is a child of MonkeyImage and that both have non-pk foreign ref back to Image, they will point to different image entries!

With the three tables defined as above, the follow code illustrates the issue:

% insert two images, one for monkey unmodified, and one for cropped monkey image
insert(test.Image, struct('image_id', 1);
insert(test.Image, struct('image_id', 2);

% insert the image under MonkeyImage
insert(test.MonkeyImage, struct('subj_id', 100, 'image_id', 1);

% insert the modified image for "subj_id = 100", where "crop" has been applied
% note that here the image_id=2, not 1!
insert(test.ModifiedMonkeyImage, struct('subj_id', 100, 'modification_type', 'crop', 'image_id', 2);

% now try to delete "image_id=2" from "Image", this should cause cascading delete in 
% test.ModifiedMonkeyImage

del(test.Image & 'image_id =  2')

% OUTPUT BELOW
ABOUT TO DELETE:
       1 tuples from `edgar_sandbox`.`image` (manual)

Proceed to delete? (yes/no) > yes
Deleting from test.Image

 ** delete rolled back due to to error
Error using mym
Cannot delete or update a parent row: a foreign key constraint fails (`edgar_sandbox`.`modified_monkey_image`, CONSTRAINT
`modified_monkey_image_ibfk_2` FOREIGN KEY (`image_id`) REFERENCES `image` (`image_id`) ON UPDATE CASCADE)

This issue has been mentioned on datajoint/datajoint-python#15 and I am not sure if it has been appropriately dealt with in datajoint-python either. This is caused by an inappropriate cascading when there exists multiple paths from a parent table (e.g. Image) to the target (ModifiedMonkeyImage, here paths are Image -> ModifiedMonkeyImage and Image -> MonkeyImage -> ModifiedMonkeyImage).

[dimitri-yatsenko]

I think the problem can be reduced to a simpler one:

delete(test.Monkey & 'image_id=2')

will fail because datajoint does not cascade deletes up the hierarchy by design. I think the error above is expected behavior. However, we need to improve the documentation.

[dimitri-yatsenko]

I changed the title of the issue because it gave the impression that erroneous deletes were performed. Failure to delete due to a referential constraint is expected behavior.

[eywalker]

I obviously didn't get back to this issue for quite some time, but no, this issue was not caused by erroneous cascading up the hierarchy but rather by incorrect cascading down of the restriction down the hierarchy chain, and this is certainly not the expected behavior. Now that we have modified a lot of things in DataJoint, it'll be good to test whether this is still an issue.

[eywalker]

I have just verified that this issue still remains in MATLAB (but haven't tested in Python). Basically in the following code snippet from Relvar.del method:

datajoint-matlab/+dj/Relvar.m

Lines 74 to 84 in e4fe19b

	for i=1:length(rels)
	% iterate through all tables that reference rels(i)
	for ix = cellfun(@(child) find(strcmp(self.schema.conn.tableToClass(child),list)), rels(i).children)
	% and restrict them by it or its restrictions
	if restrictByMe(i)
	rels(ix).restrict(pro(rels(i))) % TODO: handle renamed attributes self.conn.foreignKeys(fullTableName).aliased
	else
	rels(ix).restrict(rels(i).restrictions{:});
	end
	end
	end

for i=1:length(rels)
    % iterate through all tables that reference rels(i)
    for ix = cellfun(@(child) find(strcmp(self.schema.conn.tableToClass(child),list)), rels(i).children)
        % and restrict them by it or its restrictions
        if restrictByMe(i)
            rels(ix).restrict(pro(rels(i)))  % TODO: handle renamed attributes  self.conn.foreignKeys(fullTableName).aliased
        else
            rels(ix).restrict(rels(i).restrictions{:});
        end
    end
end

the restrictions are ANDed whereas the correct thing to do is to OR every restriction path.