DOMjudge / domjudge

DOMjudge programming contest jury system
https://www.domjudge.org
GNU General Public License v2.0
709 stars 248 forks source link

Problems Cannot be Deleted #243

Closed jGleitz closed 7 years ago

jGleitz commented 7 years ago

If I try to delete a problem, DOMJudge warns me:

Warning, this will: cascade to contestproblem cascade to event cascade to submission cascade to testcase

which sounds reasonable to me. However, if I confirm the deletion, I get this error:

error: SQL error, Error#1451: Cannot delete or update a parent row: a foreign key constraint fails (`domjudge`.`judging_run`, CONSTRAINT `judging_run_ibfk_1` FOREIGN KEY (`testcaseid`) REFERENCES `testcase` (`testcaseid`)), query: 'DELETE FROM problem WHERE `probid` = "18" LIMIT 1'

If I understand this right, the problem cannot be deleted because it would result in database constraints of judging runs being violated. But as the deletion cascades to submissions anyway, this judging runs could be deleted, too.

eldering commented 7 years ago

Indeed, see also #189. Clearly this has to be supported. It seems to me that the order of deleting referenced foreign keys should be fixed to allow this here. That is, if the problem first cascades to submissions, judgings, and judging_runs, then the testcases can be deleted afterwards.

FYI: I cannot reproduce this: I can successfully delete a problem that contains judged submissions. I suppose the order of cascading is not well-defined.

eldering commented 7 years ago

Note also that our check for foreign key dependencies in www/jury/delete.php is not complete because it doesn't search tables recursively.

eldering commented 7 years ago

@jGleitz could you confirm if this fixes the issue for you?

jGleitz commented 7 years ago

Sorry for the late reply.

@jGleitz could you confirm if this fixes the issue for you?

No, unfortunately not. Neither applying the changes of f7a63e6 nor using delete.php from master fixes the problem from me.

The error message is the same as in the initial report.

Do I have to upgrade DOMJudge completely to master to test the fix? I’m a bit reluctant to do that, as I don’t know an easy way to set up a DOMJudge test system, nor want to put my production system to an unstable version.

eldering commented 7 years ago

Which version of DOMjudge are you currently using? If 5.1, then either applying f7a63e6 or using the version from master should have worked. Could you do some manual testing on the database? (You can simply copy it to a different name or host.) In particular, if you execute directly in a mysql client or e.g. phpmyadmin the query

DELETE FROM problem WHERE probid = 'X' LIMIT 1

where X is an appropriate problem ID, does the query fail too? Then, if you first execute

DELETE FROM submission WHERE probid = 'X'

and after that the perform the delete problem query, does it work?

jGleitz commented 7 years ago

Once again, sorry for the late reply. And sorry for the German messages.

Which version of DOMjudge are you currently using?

5.1.2

DELETE FROM problem WHERE probid = 'X' LIMIT 1
#1451 - Kann Eltern-Zeile nicht löschen oder aktualisieren: eine Fremdschlüsselbedingung schlägt fehl (`domjudge_copy`.`judging_run`, CONSTRAINT `judging_run_ibfk_1` FOREIGN KEY (`testcaseid`) REFERENCES `testcase` (`testcaseid`))

(that’s the same as the message I get in the frontend)

Then, if you first execute

DELETE FROM submission WHERE probid = 'X'

and after that the perform the delete problem query, does it work?

Yes, it does.

eldering commented 7 years ago

Ok, so my intended fix seems to work when performed manually, but not in the code I wrote. Could you try to debug this? In etc/common-config.php enable DEBUG_SQL, i.e. add 4 to DEBUG and delete a problem in the jury interface. Which SQL queries are shown to be performed? It should include two delete queries: first on submissions, then on the problem. (I am seeing both)

jGleitz commented 7 years ago

I am very sorry!

I just realised that all the time, I was replacing files in my main domjudge folder, instead of the domserver subfolder (or re-running make install-domserver).

The fix does work, I only did not apply it correctly. Sorry for wasting your time!

eldering commented 7 years ago

Ok, no problem and thanks for confirming the fix.