Hey Jeremy,

Thanks for the fast and thorough response! That makes sense and explains 
why it is being validated multiple times.

We actually ran into this issue in separate places: both in validate and 
before_save hooks on the associated objects, but I was trying to keep my 
example from becoming too convoluted and thought they were related.

Since we've been digging some more it seems like there are actually two 
separate things happening in our case: Not being able to skip the 2nd 
validation as you mentioned, which validating once would solve, and then 
the release_id on the track changing from nil to the actual value on save, 
automatically clearing the reciprocal association due 
to association_autoreloading (which it looks like is now baked in), which 
then causes the association to be reloaded by the time any _save hooks are 
run on the associated objects.

We haven't figured out yet if the 2nd case is just how it needs to work or 
if there is somewhere we can add a special case for nested_attributes when 
creating the reciprocal object at the same time as the related objects -- 
in which case I *think* we know that the cached reciprocal reference is 
still valid. You bring up a good point though: it seems like if it does 
actually make sense, we should try to contain it to the nested_attributes 
plugin.

Thanks again for details!,
Ben

On Monday, November 5, 2018 at 1:28:32 PM UTC-8, Jeremy Evans wrote:
>
> On Monday, November 5, 2018 at 12:55:57 PM UTC-8, Ben Alavi wrote:
>>
>> This gist shows the whole issue we're having: 
>> https://gist.github.com/benalavi/35429576f87c1bd675e81bc19fb80174
>>
>> It looks like when you create an object along w/ multiple related objects 
>> via nested attributes through a one_to_many association validations are run 
>> multiple times (which may be on purpose?) and the reciprocal association 
>> cache on the to_many side is cleared before the 2nd validation (and before 
>> any _save hooks).
>>
>> So if you reference the reciprocally associated object in a validation or 
>> save hook it is then queried again for every associated object being saved, 
>> which in our case ended up leading to a substantial performance issue 
>> because we assumed the reciprocal association was preserved.
>>
>> Is this a bug? If so I can dig deeper and work on a patch (of course any 
>> advice would be great :))
>>
>> Or is there maybe an option somewhere to change this behavior? Or is this 
>> simply expected behavior for nested attributes? (it doesn't seem to happen 
>> when using adder methods).
>>
>> The output from that gist I get is (showing the object_id of the 
>> reciprocal object):
>>
>> Creation w/ nested_attributes
>> Track validate 70306570765160
>> Track validate 
>> Track before_save 
>>
>> Creation using adder w/ given reciprocal reference
>> Release id: 70306570435020
>> Track validate 70306570435020
>> Track before_save 70306570435020
>>
>
> First, thank you very much for the example you posted and your analysis.  
> It made it very easy to see the problem.
>
> The underlying reason for the behavior is that nested_attributes has to 
> validate the associated object during the primary object's validation 
> phase.  In general validating the associated object during the saving of 
> the associated object should be skipped, and nested_attributes does do that 
> in the cases where it is calling Model#save manually.  However, when 
> creating the associated object, it uses the add_* association method.  The 
> add_* association method does not support passing the :validate=>false 
> option to #save, so there isn't a way to tell the object to not validate on 
> save.  It can't be added to the nested_attributes plugin, because the 
> nested attributes plugin is generally only added into the classes that need 
> it (the primary object's class), not the associated classes that would need 
> it.
>
> To fix the issue you were having, we would have to do one of the following:
>
> 1) Add a new plugin that nested_attributes would load into associated 
> classes.
>
> 2) Add a method to Sequel::Model itself to skipping validation in 
> Model#save.
>
> I try to avoid adding methods to Sequel::Model itself, but in this case I 
> think that's the best approach.  This issue is not isolated to 
> nested_attributes, any time you are validating the object before the save 
> and the latter calling #save where you want to skip validating again could 
> benefit from it.  I've done a short test of that approach and it passes all 
> existing tests and fixes your issue to avoid the unnecessary "SELECT * FROM 
> `releases` WHERE `id` = ..." queries.  I'll add some more tests for the 
> behavior and try to get it committed later today.
>
> Thanks,
> Jeremy
>

-- 
You received this message because you are subscribed to the Google Groups 
"sequel-talk" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to sequel-talk+unsubscr...@googlegroups.com.
To post to this group, send email to sequel-talk@googlegroups.com.
Visit this group at https://groups.google.com/group/sequel-talk.
For more options, visit https://groups.google.com/d/optout.

Reply via email to