Issue: Multiple ForeignKeyField in one Model cause failure on deletion
Author: johnmave126
State: closed
Created: 2013-07-15 05:04:29

Reproduction:
Using MySQL innoDB (With Foreign Key Constraint check on)
peewee version: HEAD of master branch

``` python
class SampleA(Model):
    pass

class SampleB(Model):
    s1 = ForeignKeyField(SampleA, null=True)
    s2 = ForeignKeyField(SampleA, null=True)

instance1 = SampleA.create()
instance2 = SampleB.create(s1=instance1, s2=instance2)
instance1.delete_instance(recursive=True) # SQL fail on this statement
```

Cause:
in class ForeignKeyField -> add_to_class

``` python
        model_class._meta.fields[self.name] = self
        model_class._meta.columns[self.db_column] = self

        self.related_name = self._related_name or '%s_set' % (model_class._meta.name) # this statement
```

``` python

        model_class._meta.rel[self.name] = self
        self.rel_model._meta.reverse_rel[self.related_name] = self # this statement
```

Two ForeignKeyField without related_name assigned will share the same related_name. The latter one will override the former one in meta definition.
On deletion, 
in class Model -> dependencies

``` python
            for rel_name, fk in klass._meta.reverse_rel.items(): # this statement
                rel_model = fk.model_class
```

The transversal will ignore the former field, thus not update it.
The internal foreign key check by MySQL innoDB will fail on such condition.

Possible Fix:
1. Make reverse_rel something like MultiDict instead of a Dict
2. Using different naming scheme for different ForeignKeyField in the same Model

P.S.:
If your fix might change the structure of current database, wish that you could release a tool to update the current structure.


Following the validation that ensures a related_name cannot override a field on the related model, I have made it raise an `AttributeError` if you attempt to create two foreign keys with the same related_name.

Thank you for finding this!


But the case provided is that two foreign keys with no related_name assigned on `__init__`.
That is, they will both be assigned `'%s_set' % (model_class._meta.name)` as their related_name
Also, I don't think giving two foreign keys with no specified related_name in one Model is something weird...


@johnmave126 -- If I understand correctly you do not like this fix.  The rationale is:

Currently if you set the `related_name` attribute to the value of a field on the related model, it will raise an `AttributeError`, e.g.:

``` python

class A(Model):
    foo = CharField()
    bs = CharField()

class B(Model):
    a = ForeignKeyField(related_name='bs')  # AttributeError
```

It made sense to me to put the same requirement on related names.  Thus, if one `related_name` stomps on another `related_name`, raise an `AttributeError`.  To me, this is symmetrical and clean.

The following will result in `AttributeError`:

``` python

class A(Model):
    pass
class B(Model):
    a1 = ForeignKeyField(A)
    a2 = ForeignKeyField(A)  # AttributeError since related name stomps on related name for a1

### similarly
class A(Model):
    pass
class B(Model):
    a1 = ForeignKeyField(related_name='bs')
    a2 = ForeignKeyField(related_name='bs')   # AttributeError
```


@coleifer 
I don't mean I dislike it. I merely mean it might not be a perfect fix on semantical view.
Currently I understand it in this way (might not be appropriate, please correct me if I am wrong)
Setting an explicit related_name means that the rel_model cares about this relationship, or, the entry is meaningful to the rel_model. 
Setting an implicit related_name means that rel_model does not care about this relationship, or, the foreign key is a unilateral referrer from model to rel_model. This relationship is hidden to rel_model (although in fact it is not).
I don't know how others look at the meaning of foreign keys, my understanding may not be a well accepted one. So please clarify it a little bit if I've gone too far. Thank you for your great peewee!


Ah, that makes sense.  Unfortunately, peewee will always create the backref.  That has been the default behavior since the initial release so I can't change it now without worrying about breaking code that relies on the auto-generated related_name.

Glad you're liking peewee, thank you for bringing this to my attention!


@coleifer 
I thought about this issue these days and I have some ideas. These ideas are so primitive that I just post them here FYI.
I think the following behaviour is acceptable and will not cause compatibility problem.
1. Allow multiple `ForeignKeyField` without explicit `related_name`
2. Forbid multiple `ForeignKeyField` with same explicit `related_name`

This behaviour will not cause compatibility problem because the behaviour of explicit `related_name` is unchanged. The change of anonymous `ForeignKeyField` will not affect the execution of current projects based on peewee because these project have no way to and should not back reference these `ForeignKeyField` in the `rel_model`.

However, I could not think of an elegant and unified implementation for this behaviour.
I thought of a dirty solution to construct a list specially for implicit `ForeignKeyField`, but apparently it is too dirty and break the simplicity in the definition of `ForeignKeyField`.
I also thought of alter the naming scheme on backref of implicit `ForeignKeyField`, but this seems too naive.
If anyone notices this and has a better patch, it would be very nice.