Refactor ActiveRecord's WhereClause to group predicates and binds

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Refactor ActiveRecord's WhereClause to group predicates and binds

Maxime Handfield Lapointe
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?

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.
Loading...