ManyToMany decision

20 views
Skip to first unread message

Naftoli Gugenheim

unread,
Nov 10, 2009, 4:16:55 PM11/10/09
to liftweb
Hello.
When I wrote ManyToMany a couple of months ago, I designed it to internally hold a collection of join table records, and to act as a collection of elements of the child table.
For example, given Volunteer and VolunteerGroup where volunteers can be in multiple groups, Volunteer.volunteerGroups implements a collection of VolunteerGroups, but internally it's actually managing a collection of VolunteerVolunteerGroups (the join table).
The current implementation throws an error (Predef.error) when it tries to get the child element via the join record and it doesn't exist. Thus any page accessing corrupted data will not display if the error is not caught. I plan, G-d willing, to change the implementation to silently skip such records.
But the occurrence that reminded me of the defect also brought another point to my attention. To my knowledge Lift's schemifier does not correctly generate foreign key constraints for all databases (at least not at the point in time it schemified my H2 database... :) ) so we need a way for ManyToMany to keep things in sync. ManyToMany helps, to an extent, because when its MappedManyToMany members are initialized, it puts them in a list, and it propagates saves and deletes. So if you have a ManyToMany Mapper instance that contains a MappedManyToMany that has been initialized, and you call delete_! on the Mapper, it will delete all the associated join table entries. But it's not enough.
  1. That can only happen if both sides of the relationship use MappedManyToMany. Is there some way to enforce this? I was thinking of using a combination of (a) requiring the foreign MetaMapper to extends ManyToMany, and (b) when a MappedManyToMany is initialized, it should check that the foreign MetaMapper/ManyToMany actually contains a MappedManyToMany referring to the current MappedManyToMany. (a) is not sufficient without (b), and (b) has the same problem as #2 below, that objects are lazy.
  2. There is a basic problem, which is that since objects are lazy, if you haven't referenced the MappedManyToMany field, delete_! will not be able to propagate to the join entries.
#2 and #1.b can be solved by using reflection to initialize the MappedManyToMany members eagerly, just as MetaMapper does for all MappedFields. (MappedManyToMany is not a MappedField.) Now the advantage to having them lazy is that if you don't reference them they don't query the database. This advantage can be retained though by implementing the collection internally to populate lazily, much as MappedOneToMany already is.

Thoughts?

Thanks,
  Naftoli Gugenheim


Jim Barrows

unread,
Nov 11, 2009, 9:13:37 AM11/11/09
to lif...@googlegroups.com
On Tue, Nov 10, 2009 at 2:16 PM, Naftoli Gugenheim <nafto...@gmail.com> wrote:
Hello.
When I wrote ManyToMany a couple of months ago, I designed it to internally hold a collection of join table records, and to act as a collection of elements of the child table.
For example, given Volunteer and VolunteerGroup where volunteers can be in multiple groups, Volunteer.volunteerGroups implements a collection of VolunteerGroups, but internally it's actually managing a collection of VolunteerVolunteerGroups (the join table).

I don't think you really need to maintain the pivot table list (VolunteerVolunteerGroups) at all.  If you maintain the Many-to-Many as part of the object, and use a standard naming convention then you don't really need this extra list running around.  You can still build the SQL correctly knowing only the objects involved.
 
The current implementation throws an error (Predef.error) when it tries to get the child element via the join record and it doesn't exist. Thus any page accessing corrupted data will not display if the error is not caught. I plan, G-d willing, to change the implementation to silently skip such records.
But the occurrence that reminded me of the defect also brought another point to my attention. To my knowledge Lift's schemifier does not correctly generate foreign key constraints for all databases (at least not at the point in time it schemified my H2 database... :) ) so we need a way for ManyToMany to keep things in sync. ManyToMany helps, to an extent, because when its MappedManyToMany members are initialized, it puts them in a list, and it propagates saves and deletes. So if you have a ManyToMany Mapper instance that contains a MappedManyToMany that has been initialized, and you call delete_! on the Mapper, it will delete all the associated join table entries. But it's not enough.
  1. That can only happen if both sides of the relationship use MappedManyToMany. Is there some way to enforce this? I was thinking of using a combination of (a) requiring the foreign MetaMapper to extends ManyToMany, and (b) when a MappedManyToMany is initialized, it should check that the foreign MetaMapper/ManyToMany actually contains a MappedManyToMany referring to the current MappedManyToMany. (a) is not sufficient without (b), and (b) has the same problem as #2 below, that objects are lazy.

I think you're right here.  Both sides have to have the mapping.. however I don't think there is a good clean way to detect this without a compiler plugin of some kind.

 
  1. There is a basic problem, which is that since objects are lazy, if you haven't referenced the MappedManyToMany field, delete_! will not be able to propagate to the join entries.

As you traverse the deleteing side, doesn't the other side get initialized as well?
 
#2 and #1.b can be solved by using reflection to initialize the MappedManyToMany members eagerly, just as MetaMapper does for all MappedFields. (MappedManyToMany is not a MappedField.) Now the advantage to having them lazy is that if you don't reference them they don't query the database. This advantage can be retained though by implementing the collection internally to populate lazily, much as MappedOneToMany already is.

Thoughts?

I think your maintaining to much information that can be deduced from the context.
 

Thanks,
  Naftoli Gugenheim







--
James A Barrows

Naftoli Gugenheim

unread,
Nov 11, 2009, 12:31:55 PM11/11/09
to lif...@googlegroups.com, nafto...@gmail.com
To clarify: The fundamental purpose of ManyToMany, like OneToMany, is that rather than dealing with "children" of an entity as they are in the database at a given moment, instead, they should have similar semantics to a MappedField: You load it from the database, modify it to your hearts content, and then decide whether you want to save it or not. For example, calling set() on a MappedField does not write immediately to the database, and it's a good thing too--imagine a multi-page form where you can go back and forth to different screens, or a single page form that uses submit buttons that perform some action but you stay on the same page. You don't want the user's changes to go the database unless he clicks "Save." So too, one often wants to edit a list on one screen. There should be submit buttons to add records, etc., but if you don't click save it shouldn't go to the database.
So MappedOneToMany and MappedManyToMany act as collections, internally keeping track of which records were inserted or removed, but not performing a create or delete until the "field" is saved. So in other words they act as multi-valued fields.
There are two ways MappedManyToMany can do this. It can hold a list of join table records, or child table records. Either way it will have to look up the other at times.
Now in order for saves and deletes on the ManyToMany Mapper to be propagated to the MappedManyToMany fields, it has to have a list of them. The list only gets populated when code references the field, causing it to be initialized and add itself to the list of m-n fields.
So we make all of these proposed changes?

-------------------------------------
> 1. That can only happen if both sides of the relationship use
> MappedManyToMany. Is there some way to enforce this? I was thinking of using
> a combination of (a) requiring the foreign MetaMapper to extends ManyToMany,
> and (b) when a MappedManyToMany is initialized, it should check that the
> foreign MetaMapper/ManyToMany actually contains a MappedManyToMany referring
> to the current MappedManyToMany. (a) is not sufficient without (b), and (b)
> has the same problem as #2 below, that objects are lazy.
>
>
I think you're right here. Both sides have to have the mapping.. however I
don't think there is a good clean way to detect this without a compiler
plugin of some kind.



>
> 1. There is a basic problem, which is that since objects are lazy, if

glenn

unread,
Nov 12, 2009, 11:35:13 AM11/12/09
to Lift
Naftoli,

Isn't this really a cascading delete issue and not one isolated to
ManyToMany situations? Most ORM solutions allow for cascading deletes.
Such a feature could be added to the Mapper class, itself, and hold a
list of foreign key assignments that the delete function could iterate
over, if turned on.

Glenn

On Nov 11, 9:31 am, Naftoli Gugenheim <naftoli...@gmail.com> wrote:
> To clarify: The fundamental purpose of ManyToMany, like OneToMany, is that rather than dealing with "children" of an entity as they are in the database at a given moment, instead, they should have similar semantics to a MappedField: You load it from the database, modify it to your hearts content, and then decide whether you want to save it or not. For example, calling set() on a MappedField does not write immediately to the database, and it's a good thing too--imagine a multi-page form where you can go back and forth to different screens, or a single page form that uses submit buttons that perform some action but you stay on the same page. You don't want the user's changes to go the database unless he clicks "Save." So too, one often wants to edit a list on one screen. There should be submit buttons to add records, etc., but if you don't click save it shouldn't go to the database.
> So MappedOneToMany and MappedManyToMany act as collections, internally keeping track of which records were inserted or removed, but not performing a create or delete until the "field" is saved. So in other words they act as multi-valued fields.
> There are two ways MappedManyToMany can do this. It can hold a list of join table records, or child table records. Either way it will have to look up the other at times.
> Now in order for saves and deletes on the ManyToMany Mapper to be propagated to the MappedManyToMany fields, it has to have a list of them. The list only gets populated when code references the field, causing it to be initialized and add itself to the list of m-n fields.
> So we make all of these proposed changes?
>
> -------------------------------------
>
> Jim Barrows<jim.barr...@gmail.com> wrote:

glenn

unread,
Nov 12, 2009, 11:38:44 AM11/12/09
to Lift
There is a related issue that if implemented, would certainly make the
Mapper framework more robust - that of marking table rows for
deletion,
rather than deleting them directly. Some RDBMS's have a built-in
marking
function, and another function that actually removes rows that have
been
so marked.

Glenn

Naftoli Gugenheim

unread,
Nov 12, 2009, 12:04:18 PM11/12/09
to lif...@googlegroups.com, nafto...@gmail.com
I guess it's about two separate things: cascading delete, as you said; and that it should silently skip dead-end joins rather than throwing an error. Note that I'm not aware of an issue with OneToMany cascading deletes (although in order to tell it to, you have to mix in Cascade).
What we have is three possible ways to support cascading deletes.
1. Schemifier should tell the database to cascade deletes. This doesn't seem to be fully implemented, and anyway wouldn't help for MySQL MyISAM tables.
2. As you suggest, implement it on the Mapper level. That means that a given MetaMapper must know about what other foreign keys in other mappers link to it. It could be implemented as follows: MetaMapper will contain a list of say MappedForeignKeys to notify when it's deleted (or PK edited). Then you can mix in a trait to the MappedForeignKey like "CascadeDelete" or "CascadeSetNull." Either the MappedForeignKey or its MetaMapper would have to add it to the PK's MetaMapper's list. Then when the PK Mapper is deleted, it should delegate to the FK to perform some action. I like this approach. It would require the join table FKs to implement the traits, but would take some responsibility of ManyToMany's shoulders, obviating the need to require bothe sides to use it.
3. My approach.



-------------------------------------

Naftoli Gugenheim

unread,
Nov 12, 2009, 12:16:56 PM11/12/09
to lif...@googlegroups.com
Interesting. Did you see mapper.view.ItemsList?

-------------------------------------
glenn<gl...@exmbly.com> wrote:


There is a related issue that if implemented, would certainly make the
Mapper framework more robust - that of marking table rows for
deletion,
rather than deleting them directly. Some RDBMS's have a built-in
marking
function, and another function that actually removes rows that have
been
so marked.

Glenn

Naftoli Gugenheim

unread,
Nov 12, 2009, 9:28:57 PM11/12/09
to lif...@googlegroups.com
In any case, the major problem with silently dropping children not in the join table is that it becomes necessary to take indexes into the children table and convert them to indexes into the join table. Any ideas how to go about doing so, without scanning the join table for broken entries each time?

Naftoli Gugenheim

unread,
Nov 12, 2009, 11:17:47 PM11/12/09
to lif...@googlegroups.com
http://reviewboard.liftweb.net/r/101/
From my the end of my description:

Please advise me as to what testing is appropriate.
Also, when it finds broken joins should it log them?

Naftoli Gugenheim

unread,
Nov 23, 2009, 7:17:22 PM11/23/09
to liftweb
It turns out that foreign keys not being created by Schemifier was DB specific -- to my knowledge only H2. Hopefully that will get fixed. Although it doesn't specify an action - cascade etc.
So do we want a way to tell Schemifier what action to use for FK constraints? Or do we want Mapper to manage cascades etc. directly?

2009/11/12 Naftoli Gugenheim <nafto...@gmail.com>
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Lift" group.
To post to this group, send email to lif...@googlegroups.com
To unsubscribe from this group, send email to liftweb+u...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/liftweb?hl=en
-~----------~----~----~----~------~----~------~--~---




Reply all
Reply to author
Forward
0 new messages