Association being loaded when assigning assoc
Reported by Sam Carr | June 5th, 2008 @ 11:25 AM
Assuming Foo and Bar are DM resources and Foo has 1 Bar:
bar = Bar.new
foo = Foo.new
foo.bar = bar
This results in the following pseudo SQL (from memory):
INSERT INTO bar...
SELECT id, foo_id FROM bar WHERE foo_id IS NULL
INSERT INTO foo...
There are two problems with this:
- That select shouldn't be there at all!
- That select is intending to use the actual id of foo, but foo hasn't been saved yet so doesn't have one.
Solving the first problem will obviously make the second moot.
DKubb said that he can see the bug in the OneToOne module. Apparently "the code makes the assumption that the association was loaded and replaces it. The bug is that in the code it uses a kicker method, which will attempt to pull in the association prior to replacing it.. that shouldn't happen"
Comments and changes to this ticket
- Milestone cleared.
- State changed from new to open
- Assigned user changed from Sam Smoot to Dan Kubb (dkubb)
I think this problem should be resolved with the following commit:
Sam, can you re-install extlib and see if the problem has been resolved on your end?
Sorry - hasn't made any difference to my test case, which is attached (you'll probably just need to change DB connect string to get it to run). It's not a minimal repro I'm afraid, since I was testing out a bunch of stuff, but it's pretty simple and there are comments that indicate where the problem gets fired.
BTW, the SQL I see that I think is wrong, as output by that test file of mine:
~ SELECT `id`, `a`, `b`, `c`, `d`, `bar_id` FROM `sub` WHERE `bar_id` IS NULL
~ SELECT `id`, `name`, `bar_id` FROM `foo` WHERE `bar_id` IS NULL
~ INSERT INTO `bar` (`name`) VALUES ('bar1')
~ UPDATE `sub` SET `bar_id` = 1 WHERE `id` = 1
~ UPDATE `foo` SET `bar_id` = 1 WHERE `id` = 1
~ UPDATE `foo` SET `bar_id` = 1 WHERE `id` = 2
It's those two selects that shouldn't be there as far as I'm concerned.
I think this is caused by Relationship#get_children. It currently performs the all() method with the parent_key, even when the parent key is unknown.. as in the case when the parent is a new record and hasn't been assigned a key yet.
In order for the OneToMany::Proxy code to work get_children should return an empty Array when the parent_key is unknown.
One issue this will bring to light is that some operations cannot be performed on a OneToMany::Proxy when the parent key is unknown. Assertions will need to be added to warn the users that the parent needs to be saved first. When the parent is saved we should also replace the plain Array (that was returned by get_children) with a Collection populated with the Array's entries.
- State changed from open to resolved
I have committed a fix that should resolve this specific problem:
I still have more work to do on the assertions side, where people will be notified when performing operations not supported by the underlying Array. As of right now it'll raise NotImplementedError whcih is ok (and technically correct) but I'd like to be more explicit in the future.
I have added a note to the Proxy object to add the assertions in the future.
That seems to have fixed it. If the item is not saved when I update the assoc, it doesn't issue an extra query. If it is saved, it issues a query for all currently associated items, then another to perform the update. I don't understand why it needs to select all the previously associated items at all, but I'll assume that it's correct and necessary behaviour.