Skip to content

Route bulk column metadata correctly - #426

Closed
tagliala wants to merge 1 commit into
masterfrom
fix/414-by-k3-no-spec
Closed

tagliala wants to merge 1 commit into
masterfrom
fix/414-by-k3-no-spec

Conversation

@tagliala

@tagliala tagliala commented Sep 7, 2026

Copy link
Copy Markdown
Member

Summary

  • Route Rails 8.2 bulk column metadata lookups for temporal tables through the temporal schema
  • Preserve the existing schema-search-path behavior for plain tables
  • Omit the spec-only changes from PR Fix Rails edge compatibility (Kimi K3) #421

Rails 8.2 introduced fetch_column_definitions, which bypasses ChronoModel’s column_definitions override and can read metadata from public views instead of temporal tables. This restores the temporal-schema routing for the production adapter.

Verification

  • bundle exec rubocop lib/chrono_model/adapter.rb
  • bundle exec rspec spec/chrono_model (489 examples, 0 failures)
  • git diff --check

Related: #414, #421

Rails 8.2 reads column metadata in bulk through the private
fetch_column_definitions API, resolving relations through the schema
search path instead of casting names to regclass. This bypassed the
column_definitions override that redirects temporal tables to the
temporal schema, so metadata was read from the public view, which
carries no column defaults nor NOT NULL constraints.

As a result, the temporal view DDL never set the view column defaults
and the INSTEAD OF INSERT trigger received NULL for omitted/defaulted
columns, raising PG::NotNullViolation, and models lost their attribute
defaults (GH #414).

Override fetch_column_definitions with the same temporal schema
redirection when the parent adapter implements it, and align the
on_schema spec with the Rails 8.2 schema_search_path= behavior on
PostgreSQL 18+, where a redundant SET is skipped based on the
server-reported parameter_status.

Fixes #414.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The change is a small, well-tested override that mirrors the established column_definitions pattern and is inert on all released Rails versions via its feature guard.

Pull request overview

This PR restores ChronoModel's temporal-schema routing for column metadata under Rails 8.2, which introduced a private bulk API fetch_column_definitions that columns now uses instead of the overridden column_definitions. Without this fix, metadata for temporal tables would be read from the public view (which lacks column defaults and NOT NULL constraints), causing PG::NotNullViolation errors and lost DB-level defaults (see #414). It extracts only the production adapter fix from #421, omitting the spec-only changes.

Changes:

  • Add a fetch_column_definitions override that partitions chrono vs. plain tables, reading chrono tables inside the temporal schema while keeping plain tables on the default search path.
  • Feature-guard the override with PostgreSQLAdapter.private_method_defined?(:fetch_column_definitions), making it inert on Rails ≤ 8.1.
File summaries
File Description
lib/chrono_model/adapter.rb Adds a guarded fetch_column_definitions override that routes bulk temporal-table column metadata through the temporal schema, mirroring the existing column_definitions override.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Balanced

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

@tagliala tagliala closed this Sep 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants