Closed (fixed)
Project:
Relation
Version:
7.x-1.x-dev
Component:
API
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
14 Aug 2011 at 12:14 UTC
Updated:
21 Aug 2012 at 20:31 UTC
Jump to comment: Most recent file
Comments
Comment #1
chx commentedCurrently
After the change above:
There is probably some relation between T1 and T2 but for sure it's a different relation than between S and Tx. Needs label?
Comment #2
naught101 commentedEr.. yes? I think I might be missing something here (including why you'd want a 1:n relation - why not model it as n 1:1 relations?)
But my understanding of the current setup is more like:
And everything outside of that (ie. interpretation of directionality) is just sugar coating.. So yeah, if it doesn't change anything for other relation types, and doesn't change the schema, then go for it.
Comment #3
chx commentedThat's a very cool graph, thanks. Yeah, it doesn't really change anything will do later
Comment #4
chx commentedStarted rewriting with CONCEPTS.txt
Comment #5
salvisYes, I need this. I'd like to implement hourly reservations of meeting rooms and equipment (e.g. a beamer or flipchart). Sources would be users or events, and targets would be rooms and/or equipment, definitely 1:n.
Comment #6
itangalo commented+1 for introducing 1:n directional relations.
Comment #7
naught101 commentedsalvis:
relation type 1: booking: from {users, events} to {rooms}
relation type 2: booking requirements: from {bookings} to {equipment}
Would that work? Personally, I'd be very interested in getting complex relations like that working, but that's a separate issue.
related: #1259738: n->m relations (mutant squids)
Comment #8
salvisYes, in a big organization that might make sense.
The real-life operation is quite small in this case, but there needs to be a way to book multiple rooms without having to enter multiple bookings. There are only two or three pieces of relevant equipment, and introducing a second relation type seems to be overkill. I think rooms and equipment can just be bookable units. That has the added advantage that you can book a beamer (for example to take somewhere else) without having to book a room.
Comment #9
alexiswatson commentedTo add an additional +1 to this, our organization has a number of use cases for 1:n directional relationships. The parent-child pattern occurs with entities so often in our data model it makes my head spin (festival has-a series, series has-a event, etc.), and modeling them as n 1:1 relationships--while certainly possible--seems less than ideal or intuitive.
CONCEPTS.txt looks solid. Still in the process of grokking Relation, though I'd be happy to help review patches and the like in the meantime.
Comment #10
martin_q+1 from me too. 1:n is a definite need; this seems to be a sound way of doing it. Just one extra thing to throw into the mix - I'd like to be able to order the targets. Presumably that's a matter of being able to manipulate the r_index?
Comment #11
alexiswatson commentedI'm honestly not sure how I feel about tracking order like that, at least by sanctioning the use of r_index for it. At its heart, it seems like the weight/order of each child should be a property of the children, not of the relationship itself. For instance, a parent has 1:n children. Children each have an age (represented by a numeric field). If I wanted to order Mary's children Alice, Bob, and Caroline by age, I would first look to see who Mary's children are, and then sort them by age.
If there's a use case where we legitimately need to store this sort of information in the relationship, I propose making it a separate issue.
As for the original issue, what else needs to happen to make this possible? This seems like largely a documentation effort, though it also seems like we would need to relax any hard restrictions that exist on directionality for non-binary relations.
Comment #12
martin_qThanks, davidwatson - you are right, and I have filed a separate issue at #1359244: Ordering for items in 1:n relationship with a use-case description.
Comment #13
martin_qAt present, if one checks the 'directional' box then the 'minimum arity' and 'maximum arity' boxes are locked. I presume the intention was for them to be locked at the value of 2, but it is possible to uncheck the 'directional' box, set the 'maximum arity' to 'infinite' and then re-check the 'directional' box. I haven't been able to test further yet whether the rest of it works as I've run into bugs which may or may not be at all related.
Comment #14
alexiswatson commentedAs #13 mentioned, validation handling really needs to be more polished. The reason it gets away with simply disabling boxes is because min-arity and max-arity is forced to 2 on submit if it's directional, which makes me feel dirty.
evNN and I sat down and outlined this and other sub-issues that need to be addressed (including documentation on d.o after this gets pushed through) so we can work out a first pass at this. Should have something by Monday at the latest.
Comment #15
naught101 commentedYeah, arity handling isn't great.
I don't particularly like the current use of r_index - it's basically just an extra column so that the relation endpoints table can have unique primary keys (rid+r_index). It's also used to define directionality, as 0 is assumes to be source, and anything else is a target. Not a particularly flexible system.
What we need is
I'm not even sure that we need a unique key. The weighting can probably be done just with Field API's weighting system (field delta==r_index always, currently), and source/sink would be nice to do with a simple boolean flag, except that that would make #1360230: Merge this module and relation_endpoints more difficult
Comment #16
alexiswatson commentedFor the sake of having an extensible, usable Relation API, I see the merits of having a unique key for endpoints and tracking weight internally (say, if the endpoints aren't always being leveraged by Field API). I personally don't see the need to sacrifice the quality of the schema just to make merging a submodule with another module easier, especially since it's unclear at this point whether that should happen or not. Having that flag would make dealing with m->n relations that much easier later on.
That having been said, these all seem to be separate issues (#1304196: How should we do weighted Relations? for ordering, #1259738: n->m relations (mutant squids) for multiple sources), and both have implications for the schema that extend beyond the scope of this issue. Of course this is up to the maintainers, though I'd suggest following through as outlined by chx originally, since it seems like a fairly easy win in the meantime. We can always iterate on it later as the module continues to evolve.
EDIT: Mea culpa. Just occurred to me that we're actually storing endpoints via Field API anyway; didn't realize this changed since the last time I looked at the module right away. Seems like a sane way of handling weight, but even so, should probably be left to another issue. :]
Comment #17
alexiswatson commentedA possible starting point, including chx's rewrite of CONCEPTS.txt.
Tested this using the latest dev of Relation and CTools, creating directional 1:n relations using the UI, using the collector to create parent-child 1:n relations (parent first), and using dummy_field to display them. You'll note that for directional 1:n relations, only the parent is shown for each target when a reverse label is printed for natural language formatting. Otherwise you are left with "Target 1 is a child of Source, Target 2, Target 3" -- which is obviously incorrect. Sources correctly display all children, as do n-ary nondirectional relations.
Credit should also go to evNN for the patch, if we are handling notes attribution per http://groups.drupal.org/node/161659.
Review and feedback welcome!
Comment #18
chx commented@davidwatson, thanks, I will review. As your patch and my experiments show, if you are using ctools-exported relations noone stops you from doing this already. I have seen it working by just changing max_arity to 0.
Comment #20
steveoliver commentedRe-rolled @davidwatson's #17 patch against the latest 7.x-1.x (8cb812f). Works for me.
Comment #21
alexiswatson commentedThanks for the reroll! Had to shift to another solution for the time being, but I'm still very interested in getting this up and running for other projects. Setting back to CNR for testbot; one more for RTBC?
Comment #22
mikran commented#20: 1248916--nonbinary-directional-relations-20.patch queued for re-testing.
Comment #23
mikran commentedCommitted, thanks to all involved
Comment #24
chx commentedHrm, are we sure the UI change is good? It will be very confusing for symmetrical relation IMO.
Comment #25
mikran commentedAnd #1559354: allow unary relations: arity of 1 might confuse even more, maybe we need a new issue to improve the UI.
Comment #26
alexiswatson commentedI agree that the UI improvements are probably necessary, and merit their own issue, though I'm knee-deep in migration and deployments for the next month or so. If someone else wants to create the issue and outline the improvements that need to be made, I'd be happy to weigh in whenever I can steal a few cycles.