tortoise
Delete with related field query fails due to invalid SQL generation
Describe the bug Using a filter query to select rows for deletion fails if the filter query is a related field query (i.e. across a foreign key).
To Reproduce
class Device(Model):
device_id = fields.IntField(pk=True)
name = fields.CharField(max_length=16)
class LogMessage(Model):
device = fields.ForeignKeyFIeld('models.Device', related_name='log_messages')
date_created = fields.DatetimeField(auto_now_add=True)
message = fields.CharField(max_length=1024)
# [...]
await LogMessage.filter(device__name="test").delete()
Expected behavior In the example above, all "LogMessage" rows referencing devices having the name "test" should be deleted.
Instead, an invalid SQL DELETE query is generated.
Thanks for the detailed bug report. I'll add this as a test case (and fix) soon. (hopefully tomorrow)
I actually haven't had time to look into this any further, but it may require the ForeignKeyField to be nullable as well, and I should mention this happens with PostgreSQL specifically. In any case, a query like DELETE FROM LogMessage x LEFT JOIN Device y was generated, and as far as I can tell, it should be DELETE x FROM LogMessage x LEFT JOIN Device y or DELETE LogMessage FROM LogMessage LEFT JOIN Device (i.e., specify specifically which table is being deleted from). Hope that helps.
Standard DELETE FROM sql statement doesn't support LEFT JOIN, every DB has a different way of doing it.
:thinking: (Why would you LEFT JOIN when deleting, then all relations becomes optional, and therefore the delete becomes practically unconditional)
Both MySQL and PostgreSQL supports DELETE FROM LogMessage USING Device WHERE LogMessage.fk = Device.pk
SQLite does not, a subqery is pretty much required, e.g. DELETE FROM LogMessage WHERE LogMessage fk IN (SELECT pk FROM Device)
From Django delete method:
# The delete is actually 2 queries - one to find related objects,
# and one to delete. Make sure that the discovery of related
# objects is performed on the same database as the deletion.
We can think of a similar implementation here as well.
@grigi I have done the fix for this one. Since it is my very first commit, I would like to get it reviewed from someone who has more experience it this project. After having the feedback will modify accordingly.
Right now I facing issues while creating PR, once that issue is resolved I will create a PR for the fix.
