bug in bulk delete
Luke Plant
I think I've found a bug in bulk delete. It doesn't occur with mysql,
only with Postgres (perhaps because Django doesn't output the same
constraints as it does with Postgres).
A simplified but hopefully adequate version of my model is below:
class Person(models.Model):
name = models.CharField("Name", maxlength=40)
class Camp(models.Model):
number = models.PositiveSmallIntegerField("number")
chaplain = models.ForeignKey(Person,
related_name="camp_as_chaplain",
verbose_name="chaplain",
null=True, blank=True)
leaders = models.ManyToManyField(Person,
related_name="camp_as_leader",
verbose_name="leaders",
null=True, blank=True)
Notice the two relationships from Camp to Person - a ForeignKey and a
ManyToManyField.
When trying to do Person.objects.all().delete(), I get:
ERROR: update or delete on "cciw_person" violates foreign key
constraint "chaplain_id_referencing_cciw_person_id" on "cciw_camp"
DETAIL: Key (id)=(23) is still referenced from table "cciw_camp".
DELETE FROM "cciw_person" WHERE "id" IN
(44,43,42,41,40,39,38,37,36,35,34,33,32,31,30,29,28,27,26,25,24,23)
(I had already set up the relationships between 'chaplains' (Person) and
camps for all the camps).
I haven't tried doing just this test case -- I can provide the whole
thing if this isn't enough to reproduce the bug.
Regards,
Luke
--
"My capacity for happiness you could fit into a matchbox without taking
out the matches first." (Marvin the paranoid android)
Luke Plant || L.Plant.98 (at) cantab.net || http://lukeplant.me.uk/
Russell Keith-Magee
On 2/19/06, Luke Plant <luke....@gmail.com> wrote:
>
> I think I've found a bug in bulk delete.
This looks very similar to a bug I have been chasing where the query
system mistakes a m2m field for a m2o field (because of names reported
by get_accessor_name()).
What SVN revision are you working on? Does it still happen with r2310 or later?
Other than that - I'll look into it.
Russ Magee %-)
Russell Keith-Magee
>
> On 2/19/06, Luke Plant <luke....@gmail.com> wrote:
> >
> > I think I've found a bug in bulk delete.
>
Hi Luke,
I have put together a test case using your model - my test case works
fine using the up-to-date stream, but broke pre r2310. It looks like
the problem was what I thought it was.
Pre r2310, the find_field method in query.py used
OLD_get_accessor_name(), which was a holdover from the pre-descriptor
work. This method worked most of the time in the post-descriptor
world, but was ambiguous (and therefore problematic) if there were
multiple related fields to the same table in a single model.
Can you confirm that your problem is also resolved?
Russ Magee %-)
Luke Plant
> On 2/19/06, Russell Keith-Magee <freakb...@gmail.com> wrote:
> > On 2/19/06, Luke Plant <luke....@gmail.com> wrote:
> > > I think I've found a bug in bulk delete.
> >
> > Other than that - I'll look into it.
>
> Hi Luke,
>
> I have put together a test case using your model - my test case works
> fine using the up-to-date stream, but broke pre r2310. It looks like
> the problem was what I thought it was.
>
> Can you confirm that your problem is also resolved?
Thanks for working on this, but unfortunately it's still not resolved.
However, I've found I've got the same problem with deleting a single
object, and also that my model is more complex than my simplified
example. I've created tests for my model and they pass, so it must be
to do actual data in the models and some quite complex links between
tables.
I'll investigate more when I can and let you know.
Cheers,
Luke
--
Parenthetical remarks (however relevant) are unnecessary
Russell Keith-Magee
>
> On Friday 24 February 2006 23:34, Russell Keith-Magee wrote:
> > Can you confirm that your problem is also resolved?
>
> Thanks for working on this, but unfortunately it's still not resolved.
> However, I've found I've got the same problem with deleting a single
> object, and also that my model is more complex than my simplified
> example. I've created tests for my model and they pass, so it must be
> to do actual data in the models and some quite complex links between
> tables.
Interesting... by way of diagnosis, it might be helpful to dump the
contents of seen_objs that is collected just before the call to
delete_objects() at line 216 of django/core/db/query.py. That
structure determines the order in which circular dependencies are
resolved; the error you are describing certainly sounds like a
circular dependency problem.
Let me know if you have any luck narrowing the problem down at all.
Yours,
Russ Magee %-)
Luke Plant
> Interesting... by way of diagnosis, it might be helpful to dump the
> contents of seen_objs that is collected just before the call to
> delete_objects() at line 216 of django/core/db/query.py. That
> structure determines the order in which circular dependencies are
> resolved; the error you are describing certainly sounds like a
> circular dependency problem.
>
> Let me know if you have any luck narrowing the problem down at all.
Thanks for the tip, it sent me down the right road. But it was a long
and winding round that led to this fix. Would you believe it if I told
you that the following patch fixes it?
--- django/db/models/loading.py (revision 2459)
+++ django/db/models/loading.py (working copy)
@@ -17,7 +17,7 @@
_app_list = []
for app_name in settings.INSTALLED_APPS:
try:
- _app_list.append(__import__(app_name + '.models', '', '', ['']))
+ _app_list.append(__import__(app_name + '.models', '', '', ['*']))
except ImportError, e:
pass
return _app_list
@@ -26,7 +26,7 @@
"Returns the module containing the models for the given app_label."
for app_name in settings.INSTALLED_APPS:
if app_label == app_name.split('.')[-1]:
- return __import__('%s.models' % app_name, '', '', [''])
+ return __import__('%s.models' % app_name, '', '', ['*'])
raise ImproperlyConfigured, "App with label %s could not be found"
% app_label
===
(I mangled that patch a bit to make it fit, BTW)
The existing usage of __import__ is a bit strange (with an empty string
as the name to import), and seems to cause the problem. Basically
those __import__ calls were somehow causing the model classes to get
re-evaluated, rather than just imported, which eventually meant that
this function failed:
def get_all_related_objects(self):
try: # Try the cache first.
return self._all_related_objects
except AttributeError:
rel_objs = []
for klass in get_models():
for f in klass._meta.fields:
if f.rel and self == f.rel.to._meta:
rel_objs.append(RelatedObject(f.rel.to, klass,
f))
self._all_related_objects = rel_objs
return rel_objs
The line:
if f.rel and self == f.rel.to._meta:
failed as the Options() instances did not have the same identity.
Phew! That's a weird one! I'm guessing it's also to do with the fact
that my models don't live in models.py.
Would Adrian or Jacob or someone like to comment on the original usage
of __import__ and whether this patch is OK? An alternative way to do
the imports is this:
__import__(app_name, '', '', ['models']).models
and that also seems to work.
One more thing: In the admin I'm not seeing the list of related objects
to be deleted (but they are actually deleted). Is anyone else seeing
this? If not there is another bug to track down.
Luke
--
The early bird gets the worm, but the second mouse gets the cheese.
--Steven Wright