Multiple hasAndBelongsToMany() relations on the same model don't work

[mindjuice]

I have a Class model defined as follows:

var Class = thinky.createModel('Class', {
  id:       type.string().options({enforce_missing: false}),
  name:     type.string(),
  isActive: type.boolean(),
  schoolId: type.string().options({enforce_missing: false})
});

// Relationships
Class.belongsTo(School, 'school', 'schoolId', 'id');

Class.hasAndBelongsToMany(User, 'teachers', 'id', 'id');
Class.hasAndBelongsToMany(User, 'students', 'id', 'id');

I see that this creates a Class_User table with fields for Class_id and User_id (and id) and when I save a new entry in the students array, Class_User gets updated with a new document.

However, when I later getJoin({teachers: true, students: true}) I get the same set of users in both teachers and students.

To make multiple relations on the same models work, I think you need to include the name of the relation field in the two ids:

• Class_teachers_id
• Class_students_id

Then, of course, you would need to change the getJoin() code to lookup the right relation based on the field name. Does that make sense?

Or perhaps simpler, include the name of the relation as a prefix to the id string and keep the field names as is:

• Class_id: teachers-ddb49718-9f0d-464a-9a7f-d4c7ec131e18
• Class_id: students-a3895a71-98e5-4089-bbec-d26a226c4993

I hope I'm not being too much of a bother with filing all these issues, but I really like RethinkDB and Thinky so far and want to use them for a long time and help make them better!

[neumino]

Arg, I have to admit that I never thought of this case when I designed hasAndBelongsToMany. So now it definitively doesn't work.

I'm not quite sure what a good work-around is for now. We definitively need to add information in the link table (in your example Class_School. The problem is that this table is created as soon as you call hasAndBelongsTo. Meaning that if you declare the mutual hasAndBelongsTo, the table will just be re-used. So thinky doesn't know the name of the field used in the mutual join.

I can think of a two solutions:

• Change the syntax of

A.hasAndBelongsToMany(B, b_field, key1, key2)
B.hasAndBelongsToMany(A, a_field, key2, key2)
// would just be:
A.hasAndBelongsToMany(B, b_field, a_field, key1, key2)

That way we can also put b_field and a_field in the name of the tables.

• Add a new argument to hasAndBelongsToMany to tag each type of link. So in your case it would be:

Class.hasAndBelongsToMany(User, 'teachers', 'id', 'id', 'teacher');
Class.hasAndBelongsToMany(User, 'students', 'id', 'id', 'user');

// So people can also create the mutual link
User.hasAndBelongsToMany(User, 'classes', 'id', 'id', 'teacher');
User.hasAndBelongsToMany(User, 'classes', 'id', 'id', 'user');

I kind of prefer the second relation because I can make it backward compatible. Implementing the first solution would require people to rename their table (as for now since you can't simply rename a table, you have to copy all its content).

@mindjuice -- again thank you for the feedback. This is really helping, especially in this case where I really never thought about the case of having different hasAndBelongsToMany relations between two models (that are not just two mutual relations).

[neumino]

For the second solution, if you do not provide the last argument, we just keep the current behavior. If you "tag" your relation, we'll use it in the table name.

[mindjuice]

The second solution seems better to me. In the second solution, where would the tag come into play? As a new field in the ModelA_ModelB table called tag? Like this:

• id
• ModelA_id
• ModelB_id
• tag

Or did you have something else in mind?

[tlvenn]

What a perfect timing, I am also facing the same issue and I tend to prefer the second solution as well. The field should probably named 'type' which would type the relation between those 2 entities.

[mindjuice]

Thanks @tlvenn for bringing up the naming. Names are very important to get right.

To me at least, 'type' implies an overall group of similar things that may or may not be related. But 'tag' implies more of a name (like a name tag). In this case, it's the name of a relation, and any two relations that have the same tag refer to the two sides of the same relation.

On the other hand, tag has a well known meaning in HTML/XML.

It's all related to how to perform the join as well, so perhaps being more explicit is an option too. Other possibilities: joinTag, joinOn, joinName, relationName

I'm okay with 'tag', but I'm open to a more explicit name if that's preferred (I like Apple's Objective-C naming style too though, so there's that!).

[neumino]

Err I wrote a long answer, but somehow GitHub (or Chromium) ate it :/

So now the syntax for hasAndBelongsToMany is actually:

ModelA.hasAndBelongsToMany(ModelB, field_containing_b, leftKey, rightKey, options)

options can be {init: <boolean>} I'll just look for a field type in the options that will be used to make the differences between multiple relations.

The type will not be stored in the table but will be used in the table name for a few reasons:

• It's easier to do. I just have to change the name of the table.
• While Support merging additional link fields into joined docs #122 is not merged yet, I plan to let people save data per relation. In this case, you don't want to store different types in the same table (as you'll have different schema)
• I believe it should be more efficient. Tables will be smaller, there's no need for a compound index (so the keys are smaller, and the B-tree is smaller too, meaning less RAM), and there's no need to repeat the field (mostly less space on disk).

[neumino]

This is released in 1.16.5 :)

[mindjuice]

Thanks for the fix! One thing though, the table it's creating is called "Class_UserClass_User_student". I think on line 683 of model.js you only need either link += "_" or link = link + "_", not both.

[neumino]

Arg good catch @mindjuice!

Just released 1.16.6 with the fix :) Thanks again!

[mindjuice]

I've patched line 683 locally and can confirm that multiple relations are working for me now!