Add Support for implicit_column_order to ActiveRecord::Batches finder methods

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Add Support for implicit_column_order to ActiveRecord::Batches finder methods

Patrick Schmitz
With PR #34480, Tekin Suleyman added implicit_column_order for use with ordered finder methods. This support did not extend to ActiveRecord::Batches which enforces ordering on primary_key.  The same logic/reasoning introduced for implicit_column_order or defining an ordering column can be extended to ordering on ActiveRecord::Batches finder methods.

Branch with my proposed changes and tests
https://github.com/rails/rails/compare/master...bullfight:imlicit_order_find_in_batches

--
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.
To view this discussion on the web visit https://groups.google.com/d/msgid/rubyonrails-core/0a2fb9dc-9975-4f32-8b0e-d8c865970908%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Add Support for implicit_column_order to ActiveRecord::Batches finder methods

Tekin Suleyman-2
Hey Patrick,

The use case for `implicit_order_column` is only intended for configuring the behaviour of finder methods where an ordering is implied, i.e. `first` and `last`, and where the default of using the primary key to order results in unpredictable behaviour, i.e. when the PK is a UUID. It wasn't the intention that this be used as a way to define a default order on finder methods in general.

With regard to the batch finders specifically, enabling the order column to be overridden would open them up to subtle bugs and undesirable behaviour, particularly if the order column is one that isn’t guaranteed to be unique, e.g. the `created_at` timestamp. This could result in records being skipped or processed more than once.

Tekin

On 12 Jul 2019, at 23:27, Patrick Schmitz <[hidden email]> wrote:

With PR #34480, Tekin Suleyman added implicit_column_order for use with ordered finder methods. This support did not extend to ActiveRecord::Batches which enforces ordering on primary_key.  The same logic/reasoning introduced for implicit_column_order or defining an ordering column can be extended to ordering on ActiveRecord::Batches finder methods.

Branch with my proposed changes and tests

--
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.
To view this discussion on the web visit https://groups.google.com/d/msgid/rubyonrails-core/0a2fb9dc-9975-4f32-8b0e-d8c865970908%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--
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 view this discussion on the web visit https://groups.google.com/d/msgid/rubyonrails-core/15C601CB-17C5-4E75-B03A-73CA068CD220%40tekin.co.uk.