Refactor ActiveRecord's WhereClause to group predicates and binds

20 views
Skip to first unread message

Maxime Handfield Lapointe

unread,
Jul 15, 2017, 3:05:07 PM7/15/17
to Ruby on Rails: Core
I would like to do a pull request to refactor the WhereClause class of ActiveRecord. However, before doing it, I would like to have the approval that this is something that could be interesting.

The refactor would group one predicate and its binds into a sub-struct, something like WherePredicate.

Doing so would solve lots of complexities in the code of WhereClause, as it often needs to interact between predicates and their associated binds, which in the current code, means doing iteration and counting the number of BindParam in the predicates, and keeping track of such a thing. This is also the cause of a few bugs / needed in some solutions:
* https://github.com/rails/rails/pull/29780
* https://github.com/rails/rails/pull/29621.
* I am working on another pull request which also needs to do this tracking of predicates/binds.

With the refactor, there would be a single array with one struct instance per predicate and zero to many binds. This way, you can easily iterate over the predicates and filter them along with their binds without having to always grep through the nodes and count, or any other operations. The attr_reader :binds would be replaced by a simple .flat_map(&:binds)

So, is this something that I can expect to have a positive feedback?

Reply all
Reply to author
Forward
0 new messages