Skip to content

Commit 9068dfa

Browse files
yahondaclaude
andcommitted
Sweep rescue nil cleanup paths across remaining specs
Continues the cleanup begun in PR #2687. The previous PR replaced `@conn.execute "DROP …" rescue nil` in `connection_spec.rb` with `drop_table` / `drop_if_exists`, narrowing the rescue to the per-kind "object does not exist" ORA codes so privilege failures, ORA-00972 identifier-too-long on 11g, syntax errors, etc. propagate instead of being silently absorbed. This sweep covers the remaining spec files that used the same pattern: oracle_enhanced_data_types_spec.rb oracle_enhanced/context_index_spec.rb oracle_enhanced/dbms_metadata_structure_dump_spec.rb oracle_enhanced/quoting_spec.rb oracle_enhanced/schema_dumper_spec.rb oracle_enhanced/schema_statements_spec.rb oracle_enhanced/structure_dump_spec.rb The replacements follow the same per-call-site review used in #2687: - DROP TABLE (+ implicit default `<table>_seq`) → `drop_table(name, if_exists: true)`. The standard ActiveRecord public API also cleans up the default sequence, so the explicit DROP SEQUENCE line in `oracle_enhanced_data_types_spec.rb` is redundant and drops out. - DROP <other-object> (VIEW, MATERIALIZED VIEW, SYNONYM, PACKAGE, FUNCTION, PROCEDURE, TYPE, TRIGGER, SEQUENCE) → `drop_if_exists("<TYPE>", name)`. - `remove_foreign_key … rescue nil` → `remove_foreign_key …, if_exists: true` (already supported by AR). - `Object.send(:remove_const, "X") rescue nil` → `Object.send(:remove_const, "X") if Object.const_defined?("X")`. `structure_dump_spec.rb`'s `after(:each)` had a long pile of `ALTER TABLE … DROP CONSTRAINT` and `ALTER TABLE … DROP COLUMN` lines, also wrapped in `rescue nil`. They were redundant: every constraint and column they removed was on `test_posts` or `foos`, both of which the same hook drops a few lines later — the constraints and columns disappear with their parent table. A few of the names (`UK_FOOZ_BAZ`, `fooz_id`) were not even created by any test in the file. Simplified to a flat sequence of view/table/sequence/trigger/type drops in dependency order. For `remove_context_index`, ActiveRecord has no equivalent helper, so this commit also extends the Oracle-Enhanced adapter helper itself to accept `if_exists:`. `remove_context_index(name:, if_exists: true)` now calls `drop_if_exists("INDEX", index_name, if_exists: …)` internally and routes the auxiliary trigger / procedure cleanup through `drop_if_exists` too (replacing the inline `rescue nil` those private helpers had themselves). The auxiliary `drop_ctx_preference` PL/SQL call is left as-is for now — Oracle Text returns DRG-* codes, not ORA-*, and narrowing it requires a separate per-code audit. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 68aa2aa commit 9068dfa

8 files changed

Lines changed: 32 additions & 37 deletions

File tree

lib/active_record/connection_adapters/oracle_enhanced/context_index.rb

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -126,18 +126,21 @@ def add_context_index(table_name, column_name, options = {})
126126
execute sql
127127
end
128128

129-
# Drop full text index with Oracle specific CONTEXT index type
129+
# Drop full text index with Oracle specific CONTEXT index type.
130+
# Pass `if_exists: true` to tolerate the index already being absent
131+
# (rescues only ORA-01418 "specified index does not exist").
130132
def remove_context_index(table_name, options = {})
131133
unless Hash === options # if column names passed as argument
132134
options = { column: Array(options) }
133135
end
136+
if_exists = options[:if_exists]
134137
index_name = options[:name] || index_name(table_name,
135138
column: options[:index_column] || options[:column], identifier_max_length: 25)
136-
execute "DROP INDEX #{index_name}"
139+
drop_if_exists("INDEX", index_name, if_exists: if_exists)
137140
drop_ctx_preference(default_datastore_name(index_name))
138141
drop_ctx_preference(default_storage_name(index_name))
139142
procedure_name = default_datastore_procedure(index_name)
140-
execute "DROP PROCEDURE #{quote_table_name(procedure_name)}" rescue nil
143+
drop_if_exists("PROCEDURE", procedure_name, if_exists: true)
141144
drop_index_column_trigger(index_name)
142145
end
143146

@@ -291,7 +294,7 @@ def create_index_column_trigger(table_name, index_name, index_column, index_colu
291294

292295
def drop_index_column_trigger(index_name)
293296
trigger_name = default_index_column_trigger_name(index_name)
294-
execute "DROP TRIGGER #{quote_table_name(trigger_name)}" rescue nil
297+
drop_if_exists("TRIGGER", trigger_name, if_exists: true)
295298
end
296299

297300
def default_datastore_procedure(index_name)

spec/active_record/connection_adapters/oracle_enhanced/context_index_spec.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -204,8 +204,8 @@ class ::Comment < ActiveRecord::Base
204204
end
205205

206206
after(:each) do
207-
@conn.remove_context_index :posts, name: "post_and_comments_index" rescue nil
208-
@conn.remove_context_index :posts, index_column: :all_text rescue nil
207+
@conn.remove_context_index :posts, name: "post_and_comments_index", if_exists: true
208+
@conn.remove_context_index :posts, index_column: :all_text, if_exists: true
209209
Post.destroy_all
210210
end
211211

spec/active_record/connection_adapters/oracle_enhanced/dbms_metadata_structure_dump_spec.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,11 +178,11 @@
178178
describe "structure_dump_db_stored_code (DBMS_METADATA path)" do
179179
before(:each) do
180180
skip "requires Oracle 12.1+ for the DBMS_METADATA path" unless @conn.use_dbms_metadata_dump?
181-
@conn.execute "DROP PACKAGE test_dbms_metadata_pkg" rescue nil
181+
@conn.drop_if_exists("PACKAGE", "test_dbms_metadata_pkg")
182182
end
183183

184184
after(:each) do
185-
@conn.execute "DROP PACKAGE test_dbms_metadata_pkg" rescue nil
185+
@conn.drop_if_exists("PACKAGE", "test_dbms_metadata_pkg")
186186
end
187187

188188
it "emits a PACKAGE BODY only once" do

spec/active_record/connection_adapters/oracle_enhanced/quoting_spec.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,8 +200,8 @@ def create_camel_case_table
200200
drop_table "CamelCase", if_exists: true
201201
end
202202
end
203-
Object.send(:remove_const, "WarehouseThing") rescue nil
204-
Object.send(:remove_const, "CamelCase") rescue nil
203+
Object.send(:remove_const, "WarehouseThing") if Object.const_defined?("WarehouseThing")
204+
Object.send(:remove_const, "CamelCase") if Object.const_defined?("CamelCase")
205205
end
206206

207207
it "should allow creation of a table with non alphanumeric characters" do

spec/active_record/connection_adapters/oracle_enhanced/schema_dumper_spec.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,8 @@ def drop_test_posts_table
130130

131131
after(:each) do
132132
schema_define do
133-
remove_foreign_key :test_comments, :test_posts rescue nil
134-
remove_foreign_key :test_comments, name: "comments_posts_baz_fooz_fk" rescue nil
133+
remove_foreign_key :test_comments, :test_posts, if_exists: true
134+
remove_foreign_key :test_comments, name: "comments_posts_baz_fooz_fk", if_exists: true
135135
end
136136
end
137137

@@ -326,7 +326,7 @@ def drop_test_posts_table
326326

327327
describe "materialized views" do
328328
after(:each) do
329-
@conn.execute "DROP MATERIALIZED VIEW test_posts_mv" rescue nil
329+
@conn.drop_if_exists("MATERIALIZED VIEW", "test_posts_mv")
330330
drop_test_posts_table
331331
end
332332

spec/active_record/connection_adapters/oracle_enhanced/schema_statements_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1459,7 +1459,7 @@ class ::TestEmployee < ActiveRecord::Base; end
14591459
end
14601460

14611461
after(:all) do
1462-
@conn.execute("drop materialized view sum_test_employees") rescue nil
1462+
@conn.drop_if_exists("MATERIALIZED VIEW", "sum_test_employees")
14631463
schema_define do
14641464
drop_table :sum_test_employees, if_exists: true
14651465
drop_table :test_employees, if_exists: true

spec/active_record/connection_adapters/oracle_enhanced/structure_dump_spec.rb

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -36,21 +36,14 @@ class ::TestPost < ActiveRecord::Base
3636
end
3737

3838
after(:each) do
39+
@conn.drop_if_exists("VIEW", "test_posts_view_a")
40+
@conn.drop_if_exists("VIEW", "test_posts_view_z")
3941
@conn.drop_table :test_posts
4042
@conn.drop_table :foos
41-
@conn.execute "DROP SEQUENCE test_posts_seq" rescue nil
42-
@conn.execute "ALTER TABLE test_posts drop CONSTRAINT fk_test_post_foo" rescue nil
43-
@conn.execute "DROP TRIGGER test_post_trigger" rescue nil
44-
@conn.execute "DROP TYPE TEST_TYPE" rescue nil
45-
@conn.execute "DROP TABLE bars" rescue nil
46-
@conn.execute "ALTER TABLE foos drop CONSTRAINT UK_BAZ" rescue nil
47-
@conn.execute "ALTER TABLE foos drop CONSTRAINT UK_FOOZ_BAZ" rescue nil
48-
@conn.execute "ALTER TABLE foos drop column fooz_id" rescue nil
49-
@conn.execute "ALTER TABLE foos drop column baz_id" rescue nil
50-
@conn.execute "ALTER TABLE test_posts drop column fooz_id" rescue nil
51-
@conn.execute "ALTER TABLE test_posts drop column baz_id" rescue nil
52-
@conn.execute "DROP VIEW test_posts_view_z" rescue nil
53-
@conn.execute "DROP VIEW test_posts_view_a" rescue nil
43+
@conn.drop_table :bars, if_exists: true
44+
@conn.drop_if_exists("SEQUENCE", "test_posts_seq")
45+
@conn.drop_if_exists("TRIGGER", "test_post_trigger")
46+
@conn.drop_if_exists("TYPE", "TEST_TYPE")
5447
Object.send(:remove_const, "TestPost") if defined?(TestPost)
5548
ActiveRecord::Base.clear_cache!
5649
end
@@ -470,15 +463,15 @@ class ::TestPost < ActiveRecord::Base
470463
end
471464

472465
after(:each) do
466+
@conn.drop_if_exists("VIEW", "full_drop_test_view")
467+
@conn.drop_if_exists("MATERIALIZED VIEW", "full_drop_test_mview")
468+
@conn.drop_if_exists("SYNONYM", "full_drop_test_synonym")
469+
@conn.drop_if_exists("PACKAGE", "full_drop_test_package")
470+
@conn.drop_if_exists("FUNCTION", "full_drop_test_function")
471+
@conn.drop_if_exists("PROCEDURE", "full_drop_test_procedure")
472+
@conn.drop_if_exists("TYPE", "full_drop_test_type")
473473
@conn.drop_table :full_drop_test
474474
@conn.drop_table :full_drop_test_temp
475-
@conn.execute "DROP VIEW FULL_DROP_TEST_VIEW" rescue nil
476-
@conn.execute "DROP MATERIALIZED VIEW FULL_DROP_TEST_MVIEW" rescue nil
477-
@conn.execute "DROP SYNONYM FULL_DROP_TEST_SYNONYM" rescue nil
478-
@conn.execute "DROP PACKAGE FULL_DROP_TEST_PACKAGE" rescue nil
479-
@conn.execute "DROP FUNCTION FULL_DROP_TEST_FUNCTION" rescue nil
480-
@conn.execute "DROP PROCEDURE FULL_DROP_TEST_PROCEDURE" rescue nil
481-
@conn.execute "DROP TYPE FULL_DROP_TEST_TYPE" rescue nil
482475
end
483476

484477
it "should contain correct sql" do
@@ -540,7 +533,7 @@ def test_identity_dump_block(dump)
540533
end
541534

542535
after do
543-
@conn.execute("DROP SYNONYM test_synonym") rescue nil
536+
@conn.drop_if_exists("SYNONYM", "test_synonym")
544537
@conn.drop_table :test_synonym_target, if_exists: true
545538
end
546539

spec/active_record/connection_adapters/oracle_enhanced_data_types_spec.rb

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,7 @@
44
before(:all) do
55
ActiveRecord::Base.establish_connection(CONNECTION_PARAMS)
66
@conn = ActiveRecord::Base.lease_connection
7-
@conn.execute "DROP TABLE test_employees" rescue nil
8-
@conn.execute "DROP SEQUENCE test_employees_seq" rescue nil
7+
@conn.drop_table "test_employees", if_exists: true
98
@conn.execute <<~SQL
109
CREATE TABLE test_employees (
1110
employee_id NUMBER(6,0) PRIMARY KEY,

0 commit comments

Comments
 (0)