Index: tests/modeltests/delete/models.py =================================================================== --- tests/modeltests/delete/models.py (revision 11696) +++ tests/modeltests/delete/models.py (working copy) @@ -46,7 +46,7 @@ ## First, test the CollectedObjects data structure directly ->>> from django.db.models.query import CollectedObjects +>>> from django.db.models.query_utils import CollectedFields, CollectedObjects >>> g = CollectedObjects() >>> g.add("key1", 1, "item1", None) @@ -112,10 +112,12 @@ >>> d1 = D(c=c1, a=a1) >>> d1.save() ->>> o = CollectedObjects() ->>> a1._collect_sub_objects(o) +>>> o, f = CollectedObjects(), CollectedFields() +>>> a1._collect_sub_objects(o, f) >>> o.keys() [, , , ] +>>> f.keys() +[] >>> a1.delete() # Same again with a known bad order @@ -131,10 +133,12 @@ >>> d2 = D(c=c2, a=a2) >>> d2.save() ->>> o = CollectedObjects() ->>> a2._collect_sub_objects(o) +>>> o, f = CollectedObjects(), CollectedFields() +>>> a2._collect_sub_objects(o, f) >>> o.keys() [, , , ] +>>> f.keys() +[] >>> a2.delete() ### Tests for models E,F - nullable related fields ### @@ -163,21 +167,14 @@ # Since E.f is nullable, we should delete F first (after nulling out # the E.f field), then E. ->>> o = CollectedObjects() ->>> e1._collect_sub_objects(o) +>>> o, f = CollectedObjects(), CollectedFields() +>>> e1._collect_sub_objects(o, f) >>> o.keys() [, ] +>>> f.keys() +[] -# temporarily replace the UpdateQuery class to verify that E.f is actually nulled out first ->>> import django.db.models.sql ->>> class LoggingUpdateQuery(django.db.models.sql.UpdateQuery): -... def clear_related(self, related_field, pk_list): -... print "CLEARING FIELD",related_field.name -... return super(LoggingUpdateQuery, self).clear_related(related_field, pk_list) ->>> original_class = django.db.models.sql.UpdateQuery ->>> django.db.models.sql.UpdateQuery = LoggingUpdateQuery >>> e1.delete() -CLEARING FIELD f >>> e2 = E() >>> e2.save() @@ -188,15 +185,13 @@ # Same deal as before, though we are starting from the other object. ->>> o = CollectedObjects() ->>> f2._collect_sub_objects(o) +>>> o, f = CollectedObjects(), CollectedFields() +>>> f2._collect_sub_objects(o, f) >>> o.keys() -[, ] +[] +>>> f.keys() +[] >>> f2.delete() -CLEARING FIELD f - -# Put this back to normal ->>> django.db.models.sql.UpdateQuery = original_class """ } Index: django/db/models/base.py =================================================================== --- django/db/models/base.py (revision 11696) +++ django/db/models/base.py (working copy) @@ -8,10 +8,11 @@ from django.db.models.fields import AutoField, FieldDoesNotExist from django.db.models.fields.related import OneToOneRel, ManyToOneRel, OneToOneField from django.db.models.query import delete_objects, Q -from django.db.models.query_utils import CollectedObjects, DeferredAttribute +from django.db.models.query_utils import CollectedFields, CollectedObjects, DeferredAttribute from django.db.models.options import Options -from django.db import connection, transaction, DatabaseError +from django.db import connection, transaction, DatabaseError, IntegrityError from django.db.models import signals +from django.db.models.fields.related import CASCADE, PROTECT, SET_NULL, SET_DEFAULT, DO_NOTHING from django.db.models.loading import register_models, get_model from django.utils.functional import curry from django.utils.encoding import smart_str, force_unicode, smart_unicode @@ -513,7 +514,7 @@ save_base.alters_data = True - def _collect_sub_objects(self, seen_objs, parent=None, nullable=False): + def _collect_sub_objects(self, seen_objs, fields_to_set, parent=None, nullable=False): """ Recursively populates seen_objs with all objects related to this object. @@ -525,6 +526,33 @@ pk_val = self._get_pk_val() if seen_objs.add(self.__class__, pk_val, self, parent, nullable): return + + def _handle_sub_obj(related, sub_obj): + on_delete = related.field.rel.on_delete + if on_delete is None: + #If no explicit on_delete option is specified, use the old + #django behavior as the default: SET_NULL if the foreign + #key is nullable, otherwise CASCADE. + if related.field.null: + on_delete = SET_NULL + else: + on_delete = CASCADE + if on_delete == DO_NOTHING: + return + elif on_delete == CASCADE: + sub_obj._collect_sub_objects(seen_objs, fields_to_set, self.__class__) + elif on_delete == SET_NULL: + fields_to_set.add(sub_obj, related.field, None) + elif on_delete == SET_DEFAULT: + fields_to_set.add(sub_obj, related.field, related.field.get_default()) + elif on_delete == PROTECT: + msg = '[Django] Cannot delete a parent object: a foreign key constraint fails (ForeignKey `%s` (pk=`%s`) references `%s` (pk=`%s`))' % ( + sub_obj.__class__, + sub_obj._get_pk_val(), + self.__class__, + pk_val, + ) + raise IntegrityError(msg) for related in self._meta.get_all_related_objects(): rel_opts_name = related.get_accessor_name() @@ -534,7 +562,7 @@ except ObjectDoesNotExist: pass else: - sub_obj._collect_sub_objects(seen_objs, self.__class__, related.field.null) + _handle_sub_obj(related, sub_obj) else: # To make sure we can access all elements, we can't use the # normal manager on the related object. So we work directly @@ -547,7 +575,7 @@ raise AssertionError("Should never get here.") delete_qs = rel_descriptor.delete_manager(self).all() for sub_obj in delete_qs: - sub_obj._collect_sub_objects(seen_objs, self.__class__, related.field.null) + _handle_sub_obj(related, sub_obj) # Handle any ancestors (for the model-inheritance case). We do this by # traversing to the most remote parent classes -- those with no parents @@ -562,17 +590,18 @@ continue # At this point, parent_obj is base class (no ancestor models). So # delete it and all its descendents. - parent_obj._collect_sub_objects(seen_objs) + parent_obj._collect_sub_objects(seen_objs, fields_to_set) def delete(self): assert self._get_pk_val() is not None, "%s object can't be deleted because its %s attribute is set to None." % (self._meta.object_name, self._meta.pk.attname) # Find all the objects than need to be deleted. seen_objs = CollectedObjects() - self._collect_sub_objects(seen_objs) + fields_to_set = CollectedFields() + self._collect_sub_objects(seen_objs, fields_to_set) # Actually delete the objects. - delete_objects(seen_objs) + delete_objects(seen_objs, fields_to_set) delete.alters_data = True Index: django/db/models/fields/related.py =================================================================== --- django/db/models/fields/related.py (revision 11696) +++ django/db/models/fields/related.py (working copy) @@ -20,6 +20,18 @@ pending_lookups = {} +class CASCADE(object): + pass +class PROTECT(object): + pass +class SET_NULL(object): + pass +class SET_DEFAULT(object): + pass +class DO_NOTHING(object): + pass +ALLOWED_ON_DELETE_ACTION_TYPES = set([None, CASCADE, PROTECT, SET_NULL, SET_DEFAULT, DO_NOTHING]) + def add_lazy_relation(cls, field, relation, operation): """ Adds a lookup on ``cls`` when a related field is defined using a string, @@ -218,6 +230,7 @@ # object you just set. setattr(instance, self.cache_name, value) setattr(value, self.related.field.get_cache_name(), instance) + class ReverseSingleRelatedObjectDescriptor(object): # This class provides the functionality that makes the related-object @@ -628,7 +641,8 @@ class ManyToOneRel(object): def __init__(self, to, field_name, related_name=None, - limit_choices_to=None, lookup_overrides=None, parent_link=False): + limit_choices_to=None, lookup_overrides=None, parent_link=False, + on_delete=None): try: to._meta except AttributeError: # to._meta doesn't exist, so it must be RECURSIVE_RELATIONSHIP_CONSTANT @@ -641,6 +655,7 @@ self.lookup_overrides = lookup_overrides or {} self.multiple = True self.parent_link = parent_link + self.on_delete = on_delete def get_related_field(self): """ @@ -655,10 +670,12 @@ class OneToOneRel(ManyToOneRel): def __init__(self, to, field_name, related_name=None, - limit_choices_to=None, lookup_overrides=None, parent_link=False): + limit_choices_to=None, lookup_overrides=None, parent_link=False, + on_delete=None): super(OneToOneRel, self).__init__(to, field_name, related_name=related_name, limit_choices_to=limit_choices_to, - lookup_overrides=lookup_overrides, parent_link=parent_link) + lookup_overrides=lookup_overrides, parent_link=parent_link, + on_delete=on_delete) self.multiple = False class ManyToManyRel(object): @@ -697,7 +714,8 @@ related_name=kwargs.pop('related_name', None), limit_choices_to=kwargs.pop('limit_choices_to', None), lookup_overrides=kwargs.pop('lookup_overrides', None), - parent_link=kwargs.pop('parent_link', False)) + parent_link=kwargs.pop('parent_link', False), + on_delete=kwargs.pop('on_delete', None)) Field.__init__(self, **kwargs) self.db_index = True @@ -742,6 +760,16 @@ target = self.rel.to._meta.db_table cls._meta.duplicate_targets[self.column] = (target, "o2m") + on_delete = self.rel.on_delete + if on_delete not in ALLOWED_ON_DELETE_ACTION_TYPES: + raise ValueError("Invalid value 'on_delete=%s' specified for %s %s.%s." % (on_delete, type(self).__name__, cls.__name__, name)) + if on_delete == SET_NULL and not self.null: + specification = "'on_delete=SET_NULL'" + raise ValueError("%s specified for %s '%s.%s', but the field is not nullable." % (specification, type(self).__name__, cls.__name__, name)) + if on_delete == SET_DEFAULT and not self.has_default(): + specification = "'on_delete=SET_DEFAULT'" + raise ValueError("%s specified for %s '%s.%s', but the field has no default value." % (specification, type(self).__name__, cls.__name__, name)) + def contribute_to_related_class(self, cls, related): setattr(cls, related.get_accessor_name(), ForeignRelatedObjectsDescriptor(related)) Index: django/db/models/__init__.py =================================================================== --- django/db/models/__init__.py (revision 11696) +++ django/db/models/__init__.py (working copy) @@ -11,6 +11,7 @@ from django.db.models.fields.subclassing import SubfieldBase from django.db.models.fields.files import FileField, ImageField from django.db.models.fields.related import ForeignKey, OneToOneField, ManyToManyField, ManyToOneRel, ManyToManyRel, OneToOneRel +from django.db.models.fields.related import CASCADE, PROTECT, SET_NULL, SET_DEFAULT from django.db.models import signals # Admin stages. Index: django/db/models/query.py =================================================================== --- django/db/models/query.py (revision 11696) +++ django/db/models/query.py (working copy) @@ -5,8 +5,9 @@ from copy import deepcopy from django.db import connection, transaction, IntegrityError from django.db.models.aggregates import Aggregate +from django.db.models.sql.constants import GET_ITERATOR_CHUNK_SIZE from django.db.models.fields import DateField -from django.db.models.query_utils import Q, select_related_descend, CollectedObjects, CyclicDependency, deferred_class_factory +from django.db.models.query_utils import Q, select_related_descend, CollectedFields, CollectedObjects, CyclicDependency, deferred_class_factory from django.db.models import signals, sql # Used to control how many objects are worked with at once in some cases (e.g. @@ -384,12 +385,13 @@ # Collect all the objects to be deleted in this chunk, and all the # objects that are related to the objects that are to be deleted. seen_objs = CollectedObjects(seen_objs) + fields_to_set = CollectedFields() for object in del_query[:CHUNK_SIZE]: - object._collect_sub_objects(seen_objs) + object._collect_sub_objects(seen_objs, fields_to_set) if not seen_objs: break - delete_objects(seen_objs) + delete_objects(seen_objs, fields_to_set) # Clear the result cache, in case this QuerySet gets reused. self._result_cache = None @@ -1000,7 +1002,7 @@ setattr(obj, f.get_cache_name(), rel_obj) return obj, index_end -def delete_objects(seen_objs): +def delete_objects(seen_objs, fields_to_set): """ Iterate through a list of seen classes, and remove any instances that are referred to. @@ -1021,6 +1023,8 @@ obj_pairs = {} try: + fields_to_set.execute_updates() + for cls in ordered_classes: items = seen_objs[cls].items() items.sort() @@ -1034,17 +1038,6 @@ del_query = sql.DeleteQuery(cls, connection) del_query.delete_batch_related(pk_list) - update_query = sql.UpdateQuery(cls, connection) - for field, model in cls._meta.get_fields_with_model(): - if (field.rel and field.null and field.rel.to in seen_objs and - filter(lambda f: f.column == field.rel.get_related_field().column, - field.rel.to._meta.fields)): - if model: - sql.UpdateQuery(model, connection).clear_related(field, - pk_list) - else: - update_query.clear_related(field, pk_list) - # Now delete the actual data. for cls in ordered_classes: items = obj_pairs[cls] @@ -1054,14 +1047,12 @@ del_query = sql.DeleteQuery(cls, connection) del_query.delete_batch(pk_list) - # Last cleanup; set NULLs where there once was a reference to the - # object, NULL the primary key of the found objects, and perform - # post-notification. - for pk_val, instance in items: - for field in cls._meta.fields: - if field.rel and field.null and field.rel.to in seen_objs: - setattr(instance, field.attname, None) + fields_to_set.update_instances() + for cls in ordered_classes: + items = obj_pairs[cls] + items.reverse() + for pk_val, instance in items: signals.post_delete.send(sender=cls, instance=instance) setattr(instance, cls._meta.pk.attname, None) Index: django/db/models/query_utils.py =================================================================== --- django/db/models/query_utils.py (revision 11696) +++ django/db/models/query_utils.py (working copy) @@ -124,6 +124,56 @@ """ return self.data.keys() +class CollectedFields(object): + """ + A container that stores the model object and field + for fields that need to be set to enforce on_delete=SET_NULL + and on_delete=SET_DEFAULT ForeigKey constraints. + """ + + def __init__(self): + # {model: {(field, value): set([instances])}} + self.data = {} + + def add(self, obj, field, value): + """ + Adds an item. + model is the class of the object being added, + field is the field to be set, + value is the value it needs to be set to. + """ + d = self.data.setdefault(obj.__class__, dict()) + instances = d.setdefault((field, value), set()) + instances.add(obj) + + def execute_updates(self): + from django.db.models import connection, sql + for model, instances_for_fieldvalues in self.data.iteritems(): + for (field, value), instances in instances_for_fieldvalues.iteritems(): + pk_field = model._meta.pk + pk_list = [obj.pk for obj in instances] + for offset in range(0, len(pk_list), sql.constants.GET_ITERATOR_CHUNK_SIZE): + query = sql.UpdateQuery(model, connection) + query.where = query.where_class() + query.where.add((sql.where.Constraint(None, pk_field.column, pk_field), 'in', pk_list), sql.where.AND) + query.add_update_values({field.name: value}) + query.execute_sql() + + def update_instances(self): + for model, instances_for_fieldvalues in self.data.iteritems(): + for (field, value), instances in instances_for_fieldvalues.iteritems(): + for obj in instances: + setattr(obj, field.attname, value) + + def __nonzero__(self): + return bool(self.data) + + # FIXME: used by the tests, but not really needed: + def keys(self): + return self.data.keys() + + + class QueryWrapper(object): """ A type that indicates the contents are an SQL fragment and the associate