You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
It took me a while to track this bug down and finding a way to reproduce it but finally I have it! I will try to explain my findings as best as I can.
After switching from 5.0.0.1 to 5.0.1 an ActiveRecord::StatementInvalid exception is raised in my tests and it looks like that ActiveRecord is not building the query as expected when doing a mininum – and per extension maximum, average and others – calculation. I couldn't figure which was the problem out until I used git bisect and I detected that the commit that introduced the regression is #25976
Then I tried to reproduce the issue so I could open a PR to fix it and that took me a long time and I still can not explain why the ActiveRecord::StatementInvalid is happening because the issue gets weirder hahaha. I could fix the issue on my side when I read http://api.rubyonrails.org/classes/ActiveRecord/AutosaveAssociation.html and specifically Placing your callbacks after associations is usually a good practice.
This happens only when the callbacks are specified before the has_many association and when we try to autosave associated records.
In the test case, the error is raised when running the after_touch callback but only if the after_create has been also run. In the after_create the SQL query is SELECT MIN(`comments`.`id`) FROM `comments` WHERE `comments`.`post_id` IS NULL I would understand this for the before_create but in the after_create we already have a post_id 😮
bound_attributes of the comments association for ActiveRecord::Associations::CollectionProxy and its scope ActiveRecord::AssociationRelation are empty but I've seen that for associations they should have the foreign_key of the relation (maybe not related but just saying... you cannot imagine how much I've debugged here 😂)
It does not happen using sqlite!!! All the queries look as expected.
In my opinion, even if the callbacks are run before the autosave callbacks and the results we get are not the expected ones, the SQL query should be valid? What do you think?
ActiveRecord::StatementInvalid: Mysql2::Error: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '' at line 1: SELECT MIN(`comments`.`id`) FROM `comments` WHERE `comments`.`post_id` =
Using postgresql
ARCONN=postgresql ruby test_case.rb
Raises the exception
ActiveRecord::StatementInvalid: PG::ProtocolViolation: ERROR: bind message supplies 0 parameters, but prepared statement "" requires 1
: SELECT MIN("comments"."id") FROM "comments" WHERE "comments"."post_id" = $1
Awesome @kamipo! I'm not in a hurry because I could workaround this by putting the callbacks in the "recommended" position but would be nice to have your PR merged! Thanks!
Hola 👋
It took me a while to track this bug down and finding a way to reproduce it but finally I have it! I will try to explain my findings as best as I can.
After switching from 5.0.0.1 to 5.0.1 an
ActiveRecord::StatementInvalid
exception is raised in my tests and it looks like thatActiveRecord
is not building the query as expected when doing amininum
– and per extensionmaximum
,average
and others – calculation. I couldn't figure which was the problem out until I usedgit bisect
and I detected that the commit that introduced the regression is #25976Then I tried to reproduce the issue so I could open a PR to fix it and that took me a long time and I still can not explain why the
ActiveRecord::StatementInvalid
is happening because the issue gets weirder hahaha. I could fix the issue on my side when I read http://api.rubyonrails.org/classes/ActiveRecord/AutosaveAssociation.html and specificallyPlacing your callbacks after associations is usually a good practice.
I'll tell you what I've found out and let the experts try to solve it. Check the test case example first, please https://gist.github.com/yukideluxe/36b4092748310bc8ec020f5e4dad8ea9
has_many
association and when we try to autosave associated records.after_touch
callback but only if theafter_create
has been also run. In theafter_create
the SQL query isSELECT MIN(`comments`.`id`) FROM `comments` WHERE `comments`.`post_id` IS NULL
I would understand this for thebefore_create
but in theafter_create
we already have apost_id
😮bound_attributes
of thecomments
association forActiveRecord::Associations::CollectionProxy
and its scopeActiveRecord::AssociationRelation
are empty but I've seen that for associations they should have theforeign_key
of the relation (maybe not related but just saying... you cannot imagine how much I've debugged here 😂)sqlite
!!! All the queries look as expected.My first idea about how to fix this was just reverting this https://github.com/rails/rails/blob/master/activerecord/lib/active_record/associations/collection_proxy.rb#L753-L755 because if doesn't save any call which was the idea of introducing that lines in the first place but I prefer you too have a look in case this is a more serious issue or not issue at all ✌️
In my opinion, even if the callbacks are run before the autosave callbacks and the results we get are not the expected ones, the SQL query should be valid? What do you think?
Steps to reproduce it
Run this test case https://gist.github.com/yukideluxe/36b4092748310bc8ec020f5e4dad8ea9
Using sqlite
ruby test_case.rb
No exception
Using mysql2
ARCONN=mysql2 ruby test_case.rb
Raises the exception
Using postgresql
ARCONN=postgresql ruby test_case.rb
Raises the exception
This issue is also happening in the master branch, you can check that out here master...yukideluxe:callbacks-calculation-bug
Sorry for the long post, I didn't want to miss anything! I hope I explained myself. If you have any questions, let me know! ❤️
The text was updated successfully, but these errors were encountered: