Avoid stack level too deep in predicate builder

[afrase]

Summary

If you have an association with the same name as the foreign key then a SystemStackError: stack level too deep error is raised.

This PR addresses the issue by checking if the query variable is different from attributes argument before recursively calling expand_from_hash. We have been running this fix in production for a number of years without issue at the company I work for.

This issue has been mentioned in a number of issues #26778, #34869, #31110.

Other Information

Something to note is the setter for the association can raise errors. I have another branch that fixes that but I'm not sure if using an association extension is the best way to handle it.

Thanks

[rafaelfranca]

I'm not sure if this is something we should be supporting. If the foreign key has the same name of the association the behavior is very confusing. Which behavior the reader and the setter should have? Should it always return the object of the id? Should it set the object or the id? What should be the behavior of the association on fixtures?

What advantages do you get in your application by defining the foreign key withe the same name of the association?

[afrase]

I'm not sure if this is something we should be supporting. If the foreign key has the same name of the association the behavior is very confusing. Which behavior the reader and the setter should have? Should it always return the object of the id? Should it set the object or the id? What should be the behavior of the association on fixtures?

What advantages do you get in your application by defining the foreign key withe the same name of the association?

Sorry, I need to make a correction, (it's been a while since I've dug into this issue) the error happens when the foreign key has the same name as the model it's defined in for has_many and has_one. For example

class User
  has_many :notes, foreign_key: :user # => results in SystemStackError
  has_one :supervisor, foreign_key: :user # => results in SystemStackError
end

class Supervisor
  belongs_to :user, foreign_key: :user # => works as expected
end

class Note
  belongs_to :user, foreign_key: :user # => works as expected
end

You are correct, in that Rails probably shouldn't necessarily support this but I think it should still work. Right now it's impossible to do without monkey patching expand_from_hash because of the recursive problem. Associations have the option to define the foreign key so I believe they should support having that key be named (almost) anything.

There are no advantages to doing it this way but we have a legacy database from a different application that didn't follow the rails convention of having the _id suffix for relationships.

[rafaelfranca]

It makes sense to not break with a stack level too deep. Can you squash your three commits in one?

[afrase]

@rafaelfranca Should now be all squashed to one commit!