Rails: ActiveRecord::StatementInvalid in after_touch callback when using associations.

[yukideluxe]

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 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 https://github.com/rails/rails/pull/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.

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

1. This happens only when the callbacks are specified before the has_many association and when we try to autosave associated records.
2. 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 😮
3. 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 😂)
4. It does not happen using 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

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

This issue is also happening in the master branch, you can check that out here https://github.com/rails/rails/compare/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! ❤️

[kamipo]

I confirmed #25877 fixes the issue.

[yukideluxe]

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!

[maclover7]

Closing since #25877 was merged.