Skip to content

Commit a17edcb

Browse files
committed
Resolve historical join table via name/table_alias
All supported Rails versions consistently respond to `#name` and `#table_alias`, so branching on `#table_name` is unnecessary. This change standardizes on the stable API by resolving models via `join.left.name` and, in JoinNode, computing the virtual table name as `join_node.table_alias || join_node.name`. It also retains the early return in chrono_join_history when the join has already been wrapped (responds to `:as_of_time`) and removes the emulation of `Arel::Table`-like readers from the SqlLiteral JoinNode. The generated SQL semantics are unchanged; the refactor simply aligns our code with the methods that are available across Rails supported rails versions and reduces maintenance Close #388
1 parent 22ca930 commit a17edcb

File tree

2 files changed

+10
-44
lines changed

2 files changed

+10
-44
lines changed

lib/chrono_model/patches/join_node.rb

Lines changed: 6 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -2,40 +2,17 @@
22

33
module ChronoModel
44
module Patches
5-
# This class supports the AR < 7.1 code that expects to receive an
6-
# Arel::Table as the left join node. We need to replace the node
7-
# with a virtual table that fetches from the history at a given
8-
# point in time, we replace the join node with a SqlLiteral node
9-
# that does not respond to the methods that AR expects.
10-
#
11-
# This class provides AR with an object implementing the methods
12-
# it expects, yet producing SQL that fetches from history tables
13-
# as-of-time.
14-
#
15-
# TODO: Remove when dropping Rails < 7.1 compatibility
5+
# Replaces the left side of an Arel join (table or table alias) with a SQL
6+
# literal pointing to the history virtual table at the given as_of_time,
7+
# preserving any existing table alias.
168
class JoinNode < Arel::Nodes::SqlLiteral
17-
attr_reader :name, :table_name, :table_alias, :as_of_time
9+
attr_reader :as_of_time
1810

1911
def initialize(join_node, history_model, as_of_time)
20-
# Handle both Arel::Table (which has .name method) and other join node types
21-
# (which have .table_name method) to extract the table name consistently
22-
table_name =
23-
if join_node.respond_to?(:table_name)
24-
join_node.table_name
25-
elsif join_node.respond_to?(:name)
26-
join_node.name
27-
else
28-
raise ArgumentError, "Cannot determine table name from #{join_node.class}: expected :table_name or :name method"
29-
end
30-
31-
@name = table_name
32-
@table_name = table_name
33-
@table_alias = join_node.table_alias if join_node.respond_to?(:table_alias)
34-
3512
@as_of_time = as_of_time
3613

37-
virtual_table = history_model
38-
.virtual_table_at(@as_of_time, table_name: @table_alias || @table_name)
14+
table_name = join_node.table_alias || join_node.name
15+
virtual_table = history_model.virtual_table_at(@as_of_time, table_name: table_name)
3916

4017
super(virtual_table)
4118
end

lib/chrono_model/patches/relation.rb

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -68,26 +68,15 @@ def build_arel(*)
6868

6969
# Replaces a join with the current data with another that
7070
# loads records As-Of time against the history data.
71-
#
7271
def chrono_join_history(join)
73-
# This case happens with nested includes, where the below
74-
# code has already replaced the join.left with a JoinNode.
75-
#
76-
return if join.left.respond_to?(:as_of_time)
77-
78-
# Handle both Arel::Table (which has .name method) and other join node types
79-
# (which have .table_name method) to extract the table name consistently
80-
model =
81-
if join.left.respond_to?(:table_name)
82-
ChronoModel.history_models[join.left.table_name]
83-
elsif join.left.respond_to?(:name)
84-
ChronoModel.history_models[join.left.name]
85-
end
72+
join_left = join.left
73+
return if join_left.respond_to?(:as_of_time)
8674

75+
model = ChronoModel.history_models[join_left.name] if join_left.respond_to?(:name)
8776
return unless model
8877

8978
join.left = ChronoModel::Patches::JoinNode.new(
90-
join.left, model.history, @_as_of_time
79+
join_left, model.history, @_as_of_time
9180
)
9281
end
9382

0 commit comments

Comments
 (0)