many-to-many relationship
Opened this issue · 5 comments
@schettino72 It looks like the secondary support I just added is not working as expected, so I've been debugging it.
Now that I am really digging into the code and looking at the tests - there is something that doesn't quite make sense to me here.
I would have expected Group.members to be a list of GroupMember references instead of Profile references. Can you explain the current implementation?
The model is based on Association-Object pattern [1]. A different approach then what you used with a secondary table.
See the commit where it was added 06f8a09
There is quite a bit of magic there... (sorry).
I guess my reasoning was that if you want to have GroupMember with extra attributes you would add entries of GroupMember's on its own. But for simple case where Association-Object is only a reference it provides a bit of magic for convenience (and it would work same as the secondary table you have just implemented).
Are you just trying to understand the code or this is causing trouble for you?
[1] http://docs.sqlalchemy.org/en/latest/orm/basic_relationships.html#association-object
It's not causing trouble necessarily - but it just seems like a confusing API for the developer.
The "magic" part is quite confusing when just looking at the YAML. For the most part, relationships are declared with references to the model, except for this one case where it can be declared with a reference to a completely different model inferred from the association (?)
SQLAlchemy indicates that the use case for an Association Object should be for any extra columns beyond the foreign keys. The secondary argument of the relationship should be used otherwise. With that being supported now, I don't think we should be doing this indirect referencing for the Association Object.
I can fix the bug with secondary for a 0.9.1 release, but I propose a breaking change for 1.0.0 to remove the magic of the association table.
The secondary argument of the relationship should be used otherwise.
where does it say that? just because it mentions a use-case it doesn't mean the only reason is that use-case.
it doesn't say that, but i think WE (this library) should say that as it will make it less confusing by only allowing direct relationship references.
I personally prefer to use the Association-Object pattern. I think it has some advantages... I think it makes the ORM code easier to read as all tables have a mapper.
- How you query all associations when using a secondary-table pattern?
- You can use a single pattern regardless if the association has extra data or not.
Also, fixtures for many-to-many being spelled the same way regardless of how you choose to represent the relation with SQLAlchemy is IMO a very neat feature.
If you think it is confusing, maybe it is just the case of adding some notes to docs. Not removing the feature.
And as your own code shows, it is possible for both to co-exist without interfering with each other.