Allow per-database schema format #53666

tsvallender · 2024-11-18T21:52:41Z

Motivation / Background

This PR allows applications with multiple databases to set a schema_format independently for each one.

I've personally found a desire for this when converting a legacy application to use the standard Rails methods one piece at a time, but it would also be useful when there is a need to move a database to use structure.sql (for some specific functionality) but no desire to do this for all databases.

Detail

The schema_format can now be set to ruby or sql in the database configuration. If this does not exist, there is no change in behaviour and the application-wide configuration is used, or the environment variable if present and appropriate.

Additional information

Checklist

Before submitting the PR make sure the following are checked:

This Pull Request is related to one change. Unrelated changes should be opened in separate PRs.
Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
Tests are added or updated if you fix a bug or add a feature.
CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

flavorjones

👋 I'm a member of the triage team who has some familiarity with the database tasks and config code.

Currently the use of SCHEMA_FORMAT is limited to the database tasks, and I think we'd prefer to keep it out of the config classes if possible. I've made specific comments in the code.

Please also be careful about changing the method signatures of public methods (or more specifically, public methods that aren't flagged as internal-only with # :nodoc:). I've made a comment there as well.

activerecord/lib/active_record/database_configurations/hash_config.rb

activerecord/lib/active_record/railties/databases.rake

activerecord/lib/active_record/tasks/database_tasks.rb

tsvallender

Thanks for the review and sorry this has taken me a little while to get back to. I've actioned all the points you've raised.

activerecord/lib/active_record/database_configurations/hash_config.rb

activerecord/lib/active_record/railties/databases.rake

activerecord/lib/active_record/tasks/database_tasks.rb

activerecord/lib/active_record/railties/databases.rake

activerecord/lib/active_record/tasks/database_tasks.rb

tsvallender

I have a failing test on the changelog but I can't figure out why sorry.

activerecord/lib/active_record/railties/databases.rake

activerecord/CHANGELOG.md

flavorjones · 2024-12-12T22:18:50Z

@tsvallender Can I ask you to please "resolve" all the threads you've addressed? I realized that I've been procrastinating another review because of the anticipated cognitive overhead of re-reading each thread to determine if something is still needed. (It may not be obvious, but as the reviewer, github will not allow me to resolve the threads myself.)

Thanks, and sorry for my delay in responding.

tsvallender · 2024-12-13T08:29:18Z

@tsvallender Can I ask you to please "resolve" all the threads you've addressed? I realized that I've been procrastinating another review because of the anticipated cognitive overhead of re-reading each thread to determine if something is still needed. (It may not be obvious, but as the reviewer, github will not allow me to resolve the threads myself.)

Thanks, and sorry for my delay in responding.

Apologies, I've been used to a workflow where the other person resolves them when they confirm they're happy - the permissions must be different on that repo. I've resolved them all now. Thanks for your time reviewing! 🙏

activerecord/CHANGELOG.md

activerecord/lib/active_record/database_configurations/hash_config.rb

guides/source/configuring.md

railties/test/application/rake/multi_dbs_test.rb

Checks the database configuration for a schema_format key, allowing applications with multilpe databases to set schema format individually for each one. Falls back to standard configuration attribute when this is not present.

tsvallender · 2025-03-05T09:44:39Z

Just rebased this and all checks are now passing, confirming the previous BuildKit issue was unrelated 👍

* Fix style * Fix documentation * Use fetch like the other methods

* main: Cleanup #53666 Allow per-database schema format in database config Move test helpers into test_helpers (#54688) Add `except_on:` option for validation callbacks Fix capture view helper to pass keyword arguments Stop generating bundler binstub: Fix channel generator import statement: Optimize `String#parameterize` Add anchor links to FormOptionsHelper [ci skip] Fix creating fsm visualizer with lazy routes

Considering how we go forward in rails#53666, this has been an oversight when introducing the `SCHEMA_FORMAT` environment variable.

rails-bot bot added activerecord docs railties labels Nov 18, 2024

tsvallender force-pushed the per-database-schema-format branch 2 times, most recently from 601b968 to 41359a9 Compare November 19, 2024 07:23

flavorjones reviewed Nov 20, 2024

View reviewed changes

tsvallender force-pushed the per-database-schema-format branch from 41359a9 to 9486dda Compare November 29, 2024 14:51

tsvallender commented Nov 29, 2024

View reviewed changes

tsvallender force-pushed the per-database-schema-format branch from 9486dda to 8fa2802 Compare November 29, 2024 14:53

flavorjones suggested changes Nov 29, 2024

View reviewed changes

tsvallender force-pushed the per-database-schema-format branch 3 times, most recently from 345468d to 16a5bdf Compare December 1, 2024 15:49

tsvallender force-pushed the per-database-schema-format branch 2 times, most recently from e2bd1b8 to 93275d8 Compare December 9, 2024 19:43

tsvallender commented Dec 9, 2024

View reviewed changes

activerecord/lib/active_record/railties/databases.rake Show resolved Hide resolved

skipkayhil reviewed Dec 9, 2024

View reviewed changes

activerecord/CHANGELOG.md Outdated Show resolved Hide resolved

tsvallender force-pushed the per-database-schema-format branch from 93275d8 to 804aca2 Compare December 9, 2024 20:57

tsvallender requested a review from flavorjones December 9, 2024 21:42

flavorjones approved these changes Jan 19, 2025

View reviewed changes

flavorjones added the ready PRs ready to merge label Jan 19, 2025

eileencodes requested changes Feb 6, 2025

View reviewed changes

tsvallender force-pushed the per-database-schema-format branch 2 times, most recently from 989d051 to 1c1fea3 Compare February 6, 2025 18:12

tsvallender force-pushed the per-database-schema-format branch 4 times, most recently from 5955ba7 to 822871f Compare February 14, 2025 09:04

tsvallender force-pushed the per-database-schema-format branch 2 times, most recently from 379f519 to 0c961d4 Compare February 15, 2025 10:14

tsvallender requested a review from eileencodes February 15, 2025 10:23

Allow per-database schema format in database config

3081fc9

Checks the database configuration for a schema_format key, allowing applications with multilpe databases to set schema format individually for each one. Falls back to standard configuration attribute when this is not present.

tsvallender force-pushed the per-database-schema-format branch from 0c961d4 to 3081fc9 Compare March 5, 2025 09:32

eileencodes approved these changes Mar 5, 2025

View reviewed changes

eileencodes merged commit b80bc2f into rails:main Mar 5, 2025
3 checks passed

eileencodes added a commit that referenced this pull request Mar 5, 2025

Cleanup #53666

bd16ad7

* Fix style * Fix documentation * Use fetch like the other methods

schmijos added a commit to schmijos/rails that referenced this pull request May 13, 2025

Respect SCHEMA_FORMAT in db:schema:load

2a3d218

Considering how we go forward in rails#53666, this has been an oversight when introducing the `SCHEMA_FORMAT` environment variable.

schmijos mentioned this pull request May 13, 2025

Respect SCHEMA_FORMAT in db:schema:load #55059

Merged

rnestler mentioned this pull request Jun 20, 2025

Allow :schema_format in database configuration to override ActiveRecord default #45597

Closed

tsvallender mentioned this pull request Jul 3, 2025

rename schema method tsvallender/rails#3

Closed

4 tasks

This was referenced Jul 21, 2025

solid_* gems were unable to migrate the database properly if config.active_record.schema_format = :sql set. #55372

Closed

[8-0-stable] [Fix #55372] Allow per-database schema format in database config #55377

Merged

salzig pushed a commit to salzig/rails that referenced this pull request Sep 18, 2025

Respect SCHEMA_FORMAT in db:schema:load

ea77ec1

Considering how we go forward in rails#53666, this has been an oversight when introducing the `SCHEMA_FORMAT` environment variable.

Allow per-database schema format #53666

Allow per-database schema format #53666

Uh oh!

Conversation

tsvallender commented Nov 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation / Background

Detail

Additional information

Checklist

Uh oh!

flavorjones left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tsvallender left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tsvallender left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

flavorjones commented Dec 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tsvallender commented Dec 13, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tsvallender commented Mar 5, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tsvallender commented Nov 18, 2024 •

edited

Loading

flavorjones commented Dec 12, 2024 •

edited

Loading