Skip to content

Commit 923e1db

Browse files
yahondaclaude
andcommitted
Honor foreign_keys: false database.yml option
Rails 7.1 (rails/rails#45301) introduced an adapter option that lets applications opt out of foreign-key constraints via database.yml: production: adapter: oracle_enhanced ... foreign_keys: false The Rails helper use_foreign_keys? (= supports_foreign_keys? && @config.fetch(:foreign_keys, true)) is then used to gate add_foreign_key, remove_foreign_key, foreign_key_for, and SchemaCreation FK emission. oracle-enhanced's SchemaCreation#visit_TableDefinition already routes through use_foreign_keys?, and remove_foreign_key/foreign_key_for are inherited unchanged so Rails core's gates apply. Two oracle-enhanced overrides needed adjustment: * OracleEnhanced::SchemaDumper#tables overrides Rails' base method and emitted add_foreign_key lines unconditionally. Apply the gate inline next to divergent_unique_constraints(tbl, stream) so the original per-table interleave (FK_a -> UC_a -> FK_b -> UC_b) is preserved. This matches PR #45301's original dumper-gate intent; rails/rails#48731 (Feb 2024) inadvertently reverted that gate to supports_foreign_keys? upstream, so adopting use_foreign_keys? here lets Oracle users actually benefit from foreign_keys: false. * OracleEnhanced::SchemaStatements#add_foreign_key ran assert_valid_deferrable before delegating to super, so an invalid deferrable: value still raised when FKs were globally disabled. return unless use_foreign_keys? at the top of the override matches Rails core's contract (silent no-op) and mirrors the first line of Rails' own add_foreign_key in abstract/schema_statements.rb. Specs stub @conn.use_foreign_keys? rather than re-establishing the connection with foreign_keys: false. Pool replacement leaks a stale @conn into other describe blocks under randomized order, which otherwise surfaced as a tablespace-dump failure in schema_dumper_spec.rb:571. The integration-style proof that foreign_keys: false flows through to use_foreign_keys? is Rails core's responsibility (foreign_keys_enabled? = @config.fetch(:foreign_keys, true)) and is covered upstream. The new specs here cover the four oracle-enhanced surfaces that must honor the flag: dumper output, add_foreign_key no-op, deferrable validation skip, and SchemaCreation inline FK suppression with an Oracle-invalid deferrable value. structure.sql is intentionally left untouched. Rails core's per-adapter Tasks::*DatabaseTasks#structure_dump (pg_dump / mysqldump / sqlite3) does not gate on foreign_keys_enabled? / use_foreign_keys? because structure dumps reflect live DB state, not adapter config. Oracle's structure_dump (lib/.../oracle_enhanced/structure_dump.rb) is left likewise: migrations run with foreign_keys: false create no FKs so the structure dump has nothing to emit; if FKs already exist in the DB they surface in structure.sql exactly as pg_dump would. Targets master only (Rails 8.2.alpha). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent ac83019 commit 923e1db

3 files changed

Lines changed: 80 additions & 1 deletion

File tree

lib/active_record/connection_adapters/oracle_enhanced/schema_dumper.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ def tables(stream)
2727
# add foreign keys if table has them
2828
sorted_tables.each do |tbl|
2929
next if ignored? tbl
30-
foreign_keys(tbl, stream)
30+
foreign_keys(tbl, stream) if @connection.use_foreign_keys?
3131
divergent_unique_constraints(tbl, stream)
3232
end
3333

lib/active_record/connection_adapters/oracle_enhanced/schema_statements.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -735,6 +735,7 @@ def tablespace(table_name)
735735
end
736736

737737
def add_foreign_key(from_table, to_table, **options)
738+
return unless use_foreign_keys?
738739
assert_valid_deferrable(options[:deferrable])
739740

740741
super

spec/active_record/connection_adapters/oracle_enhanced/schema_dumper_spec.rb

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,84 @@ def drop_test_posts_table
440440
end
441441
end
442442

443+
describe "with use_foreign_keys? returning false (foreign_keys: false in database.yml)" do
444+
before(:each) do
445+
schema_define do
446+
drop_table :test_comments, if_exists: true
447+
drop_table :test_posts, if_exists: true
448+
create_table :test_posts, force: true do |t|
449+
t.string :title
450+
end
451+
create_table :test_comments, force: true do |t|
452+
t.string :body, limit: 4000
453+
t.references :test_post
454+
end
455+
end
456+
end
457+
458+
after(:each) do
459+
schema_define do
460+
remove_foreign_key :test_comments, :test_posts, if_exists: true
461+
drop_table :test_comments, if_exists: true
462+
drop_table :test_posts, if_exists: true
463+
end
464+
end
465+
466+
it "schema dump omits add_foreign_key statements" do
467+
schema_define do
468+
add_foreign_key :test_comments, :test_posts
469+
end
470+
allow(@conn).to receive(:use_foreign_keys?).and_return(false)
471+
expect(standard_dump).not_to match(/add_foreign_key/)
472+
end
473+
474+
it "add_foreign_key is a silent no-op" do
475+
allow(@conn).to receive(:use_foreign_keys?).and_return(false)
476+
expect {
477+
schema_define do
478+
add_foreign_key :test_comments, :test_posts
479+
end
480+
}.not_to raise_error
481+
expect(@conn.foreign_keys("test_comments")).to be_empty
482+
end
483+
484+
it "add_foreign_key skips deferrable validation" do
485+
allow(@conn).to receive(:use_foreign_keys?).and_return(false)
486+
expect {
487+
schema_define do
488+
add_foreign_key :test_comments, :test_posts, deferrable: :always
489+
end
490+
}.not_to raise_error
491+
expect(@conn.foreign_keys("test_comments")).to be_empty
492+
end
493+
494+
it "create_table with inline t.references foreign_key: true does not create the FK" do
495+
allow(@conn).to receive(:use_foreign_keys?).and_return(false)
496+
schema_define do
497+
drop_table :test_comments, if_exists: true
498+
create_table :test_comments, force: true do |t|
499+
t.string :body, limit: 4000
500+
t.references :test_post, foreign_key: true
501+
end
502+
end
503+
expect(@conn.foreign_keys("test_comments")).to be_empty
504+
end
505+
506+
it "create_table inline t.references with an Oracle-invalid deferrable does not raise" do
507+
allow(@conn).to receive(:use_foreign_keys?).and_return(false)
508+
expect {
509+
schema_define do
510+
drop_table :test_comments, if_exists: true
511+
create_table :test_comments, force: true do |t|
512+
t.string :body, limit: 4000
513+
t.references :test_post, foreign_key: { deferrable: :always }
514+
end
515+
end
516+
}.not_to raise_error
517+
expect(@conn.foreign_keys("test_comments")).to be_empty
518+
end
519+
end
520+
443521
describe "synonyms" do
444522
after(:each) do
445523
schema_define do

0 commit comments

Comments
 (0)