Skip to content

Commit 0b0546c

Browse files
authored
Resolve historical join table via name/table_alias (#389)
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 and removes the emulation of `Arel::Table`-like readers from the SqlLiteral JoinNode. `respond_to?` guards in JoinNode have been removed because `Relation#chrono_join_history` only instantiates it for nodes that respond to `:name` (and `Arel::Table` provides `:table_alias` across all supported Rails versions), so this invariant is enforced at the call site. 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 0b0546c

File tree

2 files changed

+11
-42
lines changed

2 files changed

+11
-42
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: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -68,26 +68,18 @@ 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)
72+
join_left = join.left
73+
7374
# This case happens with nested includes, where the below
7475
# 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
76+
return if join_left.is_a?(ChronoModel::Patches::JoinNode)
8677

78+
model = ChronoModel.history_models[join_left.name] if join_left.respond_to?(:name)
8779
return unless model
8880

8981
join.left = ChronoModel::Patches::JoinNode.new(
90-
join.left, model.history, @_as_of_time
82+
join_left, model.history, @_as_of_time
9183
)
9284
end
9385

0 commit comments

Comments
 (0)