Restore Rails edge compatibility

[dimvic]

Rails main (8.2.0.alpha) broke ChronoModel in two ways.

Schema readers answer for many tables at once: columns, primary keys and indexes go through the private fetch_column_definitions, fetch_primary_keys and fetch_indexes, which resolve unqualified names in the current search path and fall back to the single table readers only when nothing is found. ChronoModel's temporal schema overrides were bypassed, so the public view was read instead of the temporal table: no defaults were set on the view, omitted columns reached the INSERT trigger as NULL and NOT NULL columns raised NotNullViolation. The same readers feed the schema cache dump, which would have cached no primary key for temporal tables. Run them in the temporal schema for temporal tables.

PostgreSQL 18 reports search_path changes to the client, and the edge setter skips the SET when the server already reports the requested path. After an aborted transaction the server has already reverted the path, so restoring it in on_schema no longer raises "current transaction is aborted". The ensure block relied on that error and reset the memoized path only for nested calls, an off by one on the recursion counter, so it could stay stale after a rollback and make the new setter skip a needed SET. Reset it whenever the transaction is aborted and let the original error propagate instead of masking it with a failing SET.

Verified with Ruby 4.0.6 and Rails main a21cbefc04 against PostgreSQL 12 through 18.

Fixes ifad#414

Assistant: claude-fable-5-1 (max)

---

prompt:

Work on issue ifad#414
You must not read any other solution or branch than master and come up with your solution.
Only run specs against latest ruby and rails edge here, we'll rely on CI for older specs.
Validate fix against all major postgresql versions supported.
Work tirelessly until the solution is sound and all specs are green on all relevant postgres versions.
Use docker to spin multiple postgres versions.

[Copilot]

🟡 Changes recommended

The new Rails-version guard in specs uses lexicographic string comparison, which can misclassify versions (e.g., 8.10 vs 8.2) and should be replaced with a proper numeric/semantic comparison.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Restores compatibility with Rails main (8.2.x alpha) by ensuring multi-table schema readers operate against ChronoModel’s temporal tables (not public views) and by making on_schema’s search_path memoization resilient to aborted transactions/rollbacks.

Changes:

• Add overrides for Rails 8.2 multi-table schema reader internals (fetch_*) to read ChronoModel tables from the temporal schema.
• Adjust on_schema ensure behavior to reset memoized search_path whenever the transaction is aborted.
• Extend adapter specs to cover multi-table readers and rollback/search_path restoration.

File summaries

File	Description
spec/chrono_model/adapter/base_spec.rb	Adds coverage for Rails 8.2 multi-table schema reads and for search_path restoration behavior after rollback.
lib/chrono_model/adapter.rb	Implements temporal-schema execution for Rails 8.2 multi-table schema reader methods and updates aborted-transaction handling in on_schema.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[dimvic]

Comparison with the upstream candidates for ifad#414

All seven solutions (ifad#415, ifad#417, ifad#418, ifad#419, ifad#420, ifad#421 and this PR) fix the reported symptom the same way in spirit: read the temporal table instead of the public view. They differ in how much of the schema-reader surface they cover, whether they handle the PostgreSQL 18 search_path change in on_schema, and how they were verified.

Model	PR	+/−	Reader fix	on_schema on PG 18	Batching kept	Rails edge CI
GPT 5.6 Luna	ifad#415	+4/−2	DDL only, qualified name	memo nil via $ERROR_INFO	n/a	pass
GPT 6 Astra	ifad#417	+77/−8	columns, PKs, indexes, arrays	untouched, spec relaxed	no	pass
GPT 5.6 Sol	ifad#418	+52/−17	columns only, arrays	untouched, spec relaxed	yes	pass
Fable 5.1	ifad#419	+101/−15	columns, PKs, indexes, arrays	untouched, spec untouched	yes	fail
GPT 5.4	ifad#420	+7/−5	DDL only, qualified name	memo reset on abort	n/a	pass
Kimi K3	ifad#421	+44/−5	columns only, arrays	untouched, spec made conditional	yes	pass
Fable 5.1	this PR (b3f6d5a + c5d5cd4)	+105/−21	columns, PKs, indexes, arrays	memo reset on abort	yes	pass (see below)

GPT 5.6 Luna (ifad#415, +4/−2) and GPT 5.4 (ifad#420, +7/−5)

Smallest diffs. They pass the temporal table's qualified name when building the view, so the defaults get set. The adapter's readers still resolve bare names against the view on Rails edge. Measured on the ifad#420 checkout (same reader code as ifad#415) with Ruby 4.0.6, Rails main a21cbefc04 and PostgreSQL 18:

columns('wfoos') [name, default, null]: [["id", nil, true], ["name", "x", true]]
primary_keys(['wfoos']): {"wfoos" => []}
indexes(['wfoos']): {"wfoos" => []}

name is declared NOT NULL. The array forms are what db:schema:cache:dump uses on edge, so a dumped cache would carry no primary key for temporal models. ifad#420's on_schema change is identical to this PR's. ifad#415 instead nils the memo after a successful restore whenever an exception is in flight: that keeps the old spec shape, does not help when the restore itself raises (PostgreSQL < 18), and resets the memo needlessly on non-database exceptions.

GPT 6 Astra (ifad#417, +77/−8)

Same reader surface as this PR, and the broadest spec coverage (history-qualified names, empty lists, feature detection instead of a version check). Two costs: arrays are answered by looping the single-table readers, and fetch_column_definitions becomes a per-table loop for every table, so the one-query optimisation Rails added is lost for plain tables too. on_schema is untouched.

GPT 5.6 Sol (ifad#418, +52/−17)

Columns only, preserving order and batching, moved into a new Columns module (which also sidesteps the class-length cop). Primary keys and indexes in array form are not handled. on_schema is untouched; its spec re-opens a transaction after the rollback check.

Fable 5.1 (ifad#419, +101/−15)

Best reader design of the candidates: it overrides the public columns, primary_keys and indexes through one helper, keeps batching and input order, and does one is_chrono? per call. It leaves the on_schema spec untouched, so it fails on edge with PostgreSQL 18. Reproduced locally on its checkout, matching its four red edge jobs:

1) ChronoModel::Adapter.on_schema with default settings when errors occur
   expected Exception with message matching /current transaction is aborted/,
   got ActiveRecord::StatementInvalid: PG::SyntaxError: ERROR:  syntax error at or near "ERRORING"

Kimi K3 (ifad#421, +44/−5)

Columns only, through a feature-guarded fetch_column_definitions override that keeps batching (one query per group, plain tables first). Single-table columns is correct, including NOT NULL flags. The PR body states that the primary key and index overrides are not affected, which holds for single-table calls but not for the array forms the schema cache dump uses on edge. Measured on its checkout with the same setup:

columns('wfoos') [name, default, null]: [["id", nil, false], ["name", "x", false]]
primary_keys(['wfoos']): {"wfoos" => []}
indexes(['wfoos']): {"wfoos" => []}

Its PR body also gives the most precise account of the PostgreSQL 18 behaviour, pointing at rails/rails@e225c50, and its spec branches on Rails ≥ 8.2 plus a server that reports search_path, keeping the old expectation elsewhere. The on_schema code is untouched, so the stale-memo hazard below remains.

Fable 5.1, this PR (#3, +105/−21)

Two commits, history kept. b3f6d5a hooked Rails' private fetch_* readers and changed on_schema; it passed the whole fork matrix. c5d5cd4 reworked the readers to the design recommended below: public columns, primary_keys and indexes through one helper, batching and input order preserved, one is_chrono? per call, no dependency on Rails' month-old private methods. The on_schema change is unchanged: the memoized path is reset whenever the transaction is aborted and the original error propagates instead of being masked by a failing SET.

Validated at c5d5cd4 with Ruby 4.0.6 and Rails main a21cbefc04, full suite including the aruba specs: 518 examples, 0 failures on the local PostgreSQL 18 cluster and on the official postgres:12 through postgres:18 images. The fork's Rails 8.1 jobs went red at c5d5cd4 because a new spec asserted the boolean column default as the string "false", which Rails 8.1 reports as false; that is a spec-only issue, the core suite passes on 8.1 locally with the assertion corrected to the test column's default and NOT NULL flag.

The stale-memo hazard

Left open by ifad#417, ifad#418, ifad#419 and ifad#421. The edge setter returns early when the memoized path equals the requested one. With PostgreSQL < 18 a failing restore in an aborted transaction leaves the memo pointing at a schema the server has already reverted. The next on_schema for that schema then skips its SET and silently reads the public view. Only ifad#420 and this PR reset the memo on any aborted transaction.

Recommendation

The reader design of ifad#419 combined with the on_schema change and the aborted-transaction and post-rollback specs from this PR or ifad#420. This PR now embodies that combination.

Assistant: claude-fable-5-1 (max)

[dimvic]

Follow-up: readers reworked to the recommended design

Commit c5d5cd4 (Read temporal metadata via public schema readers) reworks the first commit along the lines recommended in the comparison: the reader design of ifad#419 combined with the on_schema change and specs from this PR. History is kept, nothing amended.

What changed

• The dynamic overrides now wrap the public primary_keys, indexes and default_sequence_name, and a public columns override replaces the column_definitions one. All four go through one private helper that evaluates the reader in the temporal schema for temporal tables and untouched for plain ones.
• The private fetch_column_definitions / fetch_primary_keys / fetch_indexes hooks are gone, so nothing depends on Rails' month-old private batched readers. A private hook is how the column_definitions override got bypassed in the first place.
• Arrays of tables (Rails ≥ 8.2) are partitioned into temporal and plain groups, read in one query per group, merged, and returned in the requested order. Single-table reads check is_chrono? once instead of twice.
• The on_schema aborted-transaction change from the first commit is unchanged: the memoized search path is reset whenever the transaction is aborted, and the original error propagates instead of being masked by a failing SET.

The three weaker points listed for this PR in the comparison (private hooks, double lookup, grouped order) no longer apply.

Specs

• Single-table columns and primary_key on temporal and plain tables, on every Rails version.
• On Rails ≥ 8.2: many-table columns, primary_keys and indexes with mixed temporal and plain tables, including key order, real indexes on both tables and the empty-list case.
• Aborted transaction inside nested on_schema: the original PG::SyntaxError propagates, the memoized path is reset, and the path is back to the default after ROLLBACK.

Validation

Ruby 4.0.6, Rails main a21cbefc04, full suite including the aruba integration specs:

Server	Result
Local PostgreSQL 18.4 cluster	518 examples, 0 failures
postgres:12 (12.22)	518 examples, 0 failures
postgres:13 (13.23)	518 examples, 0 failures
postgres:14 (14.24)	518 examples, 0 failures
postgres:15 (15.19)	518 examples, 0 failures
postgres:16 (16.15)	518 examples, 0 failures
postgres:17 (17.11)	518 examples, 0 failures
postgres:18 (18.6)	518 examples, 0 failures

RuboCop: 79 files inspected, no offenses. A sanity script over the schema cache dump path (SchemaCache#add_all) returns the temporal table's primary key, columns and indexes.

Assistant: claude-fable-5-1 (max)