Issue: Deriving model classes with ManyToManyField
Author: mried
State: closed
Created: 2018-12-24 20:08:07

Hi!

I recently switched from peewee 2.8.5 to 3.8.0 (and from Python 2.7 to Python 3.7 but I don't think, this is the reason for the problem described below). After doing so, I've got a problem with derived model classes, if they contain a `ManyToManyField`. This is a super simple (and stupid) example, which raises an `IndexError`:
```python
class User(BaseModel):
    pass


class Project(BaseModel):
    users = ManyToManyField(User, backref='projects')
    rating = IntegerField()


class VProject(Project):  # This line throws the exception
    # users = ManyToManyField(User, backref='projects')  # Needed in peewee 2.8.5 for a reason I don't remember
    is_top_rated = BooleanField()

    @classmethod
    def create_view(cls):
        db.execute_sql('''CREATE VIEW vproject AS
                          SELECT project.*, project.rating >= 5 AS is_top_rated
                            FROM project''')
```
As you can see, I use the derived class to access a view which has basically the same layout as a table - it only adds a column (the real case is a bit more complicated, of course). It worked perfectly using peewee 2.8.5, but now I get an `IndexError`:
```
Traceback (most recent call last):
  File "peeweetest.py", line 20, in <module>
    class VProject(Project):
  File "venv\lib\site-packages\peewee.py", line 5421, in __new__
    cls._meta.add_field(name, field)
  File "venv\lib\site-packages\peewee.py", line 5239, in add_field
    field.bind(self.model, field_name, set_attribute)
  File "venv\lib\site-packages\peewee.py", line 4702, in bind
    super(ManyToManyField, self).bind(model, name, set_attribute)
  File "venv\lib\site-packages\peewee.py", line 3963, in bind
    setattr(model, name, self.accessor_class(model, self, name))
  File "venv\lib\site-packages\peewee.py", line 4651, in __init__
    self.src_fk = self.through_model._meta.model_refs[self.model][0]
IndexError: list index out of range
```
If I understand the peewee code which throws the exception correctly, it tries to get some information about the through model of the derived class (`VProject` in my example), but the model has the infos for the base class (hence `Project`) only.

Is there a way to get around this error without removing the `ManyToManyField`?

Ahh, yeah, `ManyToManyField` may not play nicely with inheritance without some additional effort. Can you not just replicate the definition of the `ManyToManyField`, as you have it commented-out currently?

Whoops... I must admit, the super simple example works after removing the comment-`#`. The example was wrong and didn't show the problem I've got in real code... The following produces the error again:
```python
class Project(BaseModel):
    rating = IntegerField()


class User(BaseModel):
    projects = ManyToManyField(Project, backref='users')


class VProject(Project):
    # users = ManyToManyField(User, backref='projects')  # Comment 1
    is_top_rated = BooleanField()

    @classmethod
    def create_view(cls):
        db.execute_sql('''CREATE VIEW vproject AS
                          SELECT project.*, project.rating >= 5 AS is_top_rated
                            FROM project''')
```
I placed the `ManyToManyField` at the `User` class which is basically the "other end" of the relationship of my initial example. `VProject` still derives from `Project` which has no `ManyToManyField` explicitly. This gives the exact same exception as written in my first post.
Since replicating the `ManyToManyField` definitions helped so far, I added the line which is commented out here (Comment 1). After adding that, it seems to work but, I'm afraid this might have side effects I don't see right now.

Thanks for the comment, this is now fixed.

Thanks for the fix.

This seems to fix the problem I mentioned in my first post, but not the one in the second post. Here is a test which still produces the `IndexError`:
```python
    def test_manytomany_inheritance_2(self):
        class BaseModel(TestModel):
            class Meta:
                database = self.database
        class Project(BaseModel):
            name = TextField()
        class User(BaseModel):
            username = TextField()
            projects = ManyToManyField(Project, backref='users')
        class VProject(Project):
            pass

        PThrough = Project.users.through_model
        self.assertTrue(PThrough.project.rel_model is Project)
        self.assertTrue(PThrough.user.rel_model is User)

        VPThrough = VProject.users.through_model
        self.assertTrue(VPThrough.vproject.rel_model is VProject)
        self.assertTrue(VPThrough.user.rel_model is User)
```
(can be placed at line 205 in `tests/manytomany.py`)

Is it possible to fix this, too?

Yeah...this is all very weird, though, right? I mean, I think it's a bug and I have a one-line fix. But it struck me as rather confusing trying to think about inheriting many-to-many. A many-to-many between two tables implies a third table that has foreign-keys to the two sources:

Student >-- StudentCourse --< Course

So `StudentCourse` has a foreign-key `student_id` and `course_id`.

If you want to subclass either `Student` or `Course`, then presumably the junction table would need to be "subclassed" as well. So if `HonorStudent` is a subclass of `Student`, we need a new junction table as well:

HonorStudent >-- HonorStudentCourse --< Course

For auto-generated through models, which is what the test-case I added deals with, I think this is OK (if confusing).

But what if the through model is user-defined? Should it automatically be subclassed, and have its foreign-key modified?

I think the safest and most correct approach is really not to ever inherit many-to-many fields...because we're not really talking about inheriting a "field"...we're talking about the creation of a completely new table to store the junction -- and that seems to get into confusing territory quickly.

I'm going to instead favor a slightly better error message and a strong admonishment to not subclass many-to-many.

I totally agree because there is another thing that came into my mind: the `backref` fields.

Given the following code:
```python
class User(BaseModel):
    pass


class Project(BaseModel):
    users = ManyToManyField(User, backref='projects')
```
Having a `User` instance `user`, I can access `user.projects` and get a list of `Project` instances. All fine until now.

Now we derive `VProject` from `Project` and access `user.projects` again. What do we get? A list of `Project` instances? A list of `VProject` instances? This is absolutely not clear to the user (at least to me 😉).

To sum up: Don't derive `ManyToManyField`s. It may produce unforeseen consequences 🔥. So an explicit warning like you suggested sounds like the best solution.

Yes, exactly...the backrefs were what was causing half of the trouble anyways, since in "many-to-many"-land, a backref is exactly the same as an ordinary `ManyToManyField`.