Skip to content

Commit

Permalink
Fix batch methods for historical models
Browse files Browse the repository at this point in the history
In the default implementation, `cursor` defaults to `primary_key`, which
is 'id'.
However, for historical models, we need to use 'hid' instead of 'id'.

This patch addresses an issue where `with_hid_pkey` is called after the
cursor is already set, potentially leading to incorrect behavior.

Notes:
- `find_each` and `find_in_batches` use `in_batches` internally, however
  this is not enough anymore in the upcoming Rails 8.0 because there is
  a conditional branch using `enum_for`
- This approach prevents specifying 'id' as a cursor for historical
  models. If 'id' is needed, it must be handled separately.

Fix #321
  • Loading branch information
tagliala committed Aug 12, 2024
1 parent e6f3748 commit 04b7e5b
Showing 1 changed file with 35 additions and 1 deletion.
36 changes: 35 additions & 1 deletion lib/chrono_model/patches/batches.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,44 @@

module ChronoModel
module Patches
# Overrides the default batch methods for historical models
#
# In the default implementation, `cursor` defaults to `primary_key`, which is 'id'.
# However, for historical models, we need to use 'hid' instead of 'id'.
#
# This patch addresses an issue where `with_hid_pkey` is called after the cursor
# is already set, potentially leading to incorrect behavior.
#
# Notes:
# - `find_each` and `find_in_batches` use `in_batches` internally, however
# this is not enough anymore in the upcoming Rails 8.0 because there is
# a conditional branch using `enum_for`
# - This approach prevents specifying 'id' as a cursor for historical models.
# If 'id' is needed, it must be handled separately.
#
# See: ifad/chronomodel#321 for more context
module Batches
def in_batches(**)
def find_each(**options)
return super unless try(:history?)

options[:cursor] = 'hid' if options[:cursor] == 'id'

with_hid_pkey { super }
end

def find_in_batches(**options)
return super unless try(:history?)

options[:cursor] = 'hid' if options[:cursor] == 'id'

with_hid_pkey { super }
end

def in_batches(**options)
return super unless try(:history?)

options[:cursor] = 'hid' if options[:cursor] == 'id'

with_hid_pkey { super }
end
end
Expand Down

0 comments on commit 04b7e5b

Please sign in to comment.