Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

better explanation for optional fk values in rels #37

Closed
wants to merge 5 commits into from
Closed

better explanation for optional fk values in rels #37

wants to merge 5 commits into from

Conversation

davewood
Copy link
Contributor

No description provided.

@ribasushi
Copy link
Collaborator

The original discussion on this was:

<davewood> can the "problem" be simplified to: "if your foreign key column is not nullable you want join_type => left for complex resultsets to work correctly"?
<ribasushi> davewood: itym s/not nullable/nullable/
<ribasushi> and "for relatonship traversal to work consistently in all situations" (nothing complex about the resultset I gave as example)
<ribasushi> davewood: but yes, that'd be much better - convert it to a final patch and show again?

The current patch however simply rewords two sentences, while keeping the original confusing semantic structure intact. Is there a reason you didn't go for the much simpler explanation as in the IRC log above?
I could apply the patch as-is, but it imho makes no difference whatsoever wrt the problem you were originally trying to solve.

@ribasushi
Copy link
Collaborator

So, all patches in this PR have been incorporated except for davewood@5911039b

<ribasushi> davewood: about the cookbook entry
<ribasushi> davewood: it needs no changes, however on first read it seems to imply that DBIC will take care of the nulling and all that jazz, which is not true
<ribasushi> that is - it will work as described *only* if you deploy() from DBIC
<ribasushi> davewood: I think something needs added to this effect
<ribasushi> davewood: on a tangent - I actually have correctly working code to do dbic-side on_X emulation, but it has no tests (part of a larger thing)
<ribasushi> davewood: if you want to spin it into a separate dist - I would be very very happy
<ribasushi> davewood: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=dbsrgits/DBIx-Class-Shadow.git;a=blob;f=lib/DBIx/Class/Relationship/Cascade/Rekey.pm;hb=HEAD
<davewood> ribasushi: I wouldnt mind helping out with unit tests, will get back to you in 2014.

<ribasushi> davewood: so what do you think wrt the cookbook thing?
<davewood> ribasushi: the least would be to add a note saying that this only works if deployed from DBIC and I can also add the SQL needed for those working the other way around.
<ribasushi> davewood: cool, I will keep it in the branch for the time being then, poke me when you have a writeup

Currently commit is in https://github.com/dbsrgits/dbix-class/tree/topic/nullable_belongsto_recipe

@ribasushi
Copy link
Collaborator

@davewood I will close this PR for the time being. The branch is still available https://github.com/dbsrgits/dbix-class/tree/topic/nullable_belongsto_recipe for further improvement as described in #37 (comment)

Looking forward to another PR with the discussed changes in place.

Cheers and thanks!

@ribasushi ribasushi closed this May 23, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants