I haven't had a chance to look at the patch yet, but what you describe here sounds good. I don't have any problem with you "hijacking" the work.
Did your patch deal at all with the unit tests in my patch that intentionally failed to expose things that weren't working right yet? Mike Johannes Dollinger wrote: > There's a new patch on the ticket[1], based on Michael Glassford's > work, that solves a few remaining issues and should be fully backwards > compatible. I haven't painted the bikeshed yet, so the API still is > `on_delete=CASCADE | SET_NONE | SET_DEFAULT | PROTECT | DO_NOTHING | > SET(value)`. Two minor additions: > > `DO_NOTHING` does nothing. Let the db handle it or resolve the > dependency in pre_delete (see: #10829 [2]) > > `SET(value)` sets the foreign key to an arbitrary value. `value` > may > be a callable, `SET(None)` is equivalent to `SET_NULL`. > > To make `on_delete` work on m2m intermediary models > `DeleteQuery.delete_batch_related()` had to go. Intermediary models > now use (almost) the same related-objects-collection code path as > every other model (thanks to m2m-refactor). Because that would have > lead to lots of SELECT queries for related objects, I refactored the > collection algorithm to collect batches of objects instead of > individual objects. That reduced the overhead to > `#INTERMEDIARY_INSTANCES / GET_ITERATOR_CHUNK_SIZE` queries. This > refactoring has a nice side-effect: Given the following code > > class A(models.Model): pass > class B(models.Model): a = models.ForeignKey(A) > class C(models.Model): b = models.ForeignKey(B) > > a = A.objects.create() > for i in xrange(100): B.objects.create(a=a) > a.delete() > > the `delete()` call results in 103 queries with trunk, and only 4 > queries with my patch applied. > > Finally, collecting related objects for auto_created intermediary > models is short-circuited to avoid the extra SELECTs completely. The > same could be done for any model that has no related objects, if we > didn't need the instances to send signals (Someday/Maybe: > Meta.send_signals = bool or tuple). > > Since the constants used for `on_delete` are just callables, it's > possible to do any kind of calculation to decide what should happen to > related instances, e.g.: > > def delete_or_setnull(collector, field, objects): > setnull = [] > cascade = [] > for obj in objects: > if can_delete(obj): > delete.append(obj) > else: > setnull.append(obj) > SET_NULL(collector, field, setnull) > CASCADE(collector, field, cascade) > > fk = ForeignKey(To, on_delete=delete_or_setnull, null=True) > > This should probably not be documented at first, but it would be a > nice feature once it's clear the on_delete handler signature will > remain stable. > > > FIXME: > * I'd like to introduce `DatabaseFeatures.can_defer_constraint_checks` > to disable nulling out foreign keys when it's not necessary. This > would save a couple of UPDATE queries. > > * There are ugly contrib.contenttypes imports in > `DeleteQuery.delete_batch_related()`. I left all contenttypes related > code there (just renamed the method, it's still called). Someday/Maybe > this could be removed and handled as a custom `on_delete` argument on > GenericForeignKey. > > * There are no docs. > > > Any feedback welcome. > > @glassfordm: If you're still working on this patch, I'd like to hear > what you think (and get those tests you mentioned). I'm sorry I > somewhat hijacked your work. > > __ > Johannes > > [1] http://code.djangoproject.com/ticket/7539 > [2] http://code.djangoproject.com/ticket/10829 > > > > > > > --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~----------~----~----~----~------~----~------~--~---
