oracle-enhanced icon indicating copy to clipboard operation
oracle-enhanced copied to clipboard

ActiveRecord::ConnectionAdapters::SchemaStatements.remove_index does not work in Rails 6.1.2 migrations if both column and index name are given

Open rammpeter opened this issue 4 years ago • 2 comments

Steps to reproduce

This example ends in error like migrations do during rollback of add_index if a index name ist given

require 'bundler/inline'

gemfile(true) do
  gem 'activerecord', '6.1.2.1'
  gem 'activerecord-oracle_enhanced-adapter', '6.1.2'
end

require 'active_record'
require 'active_record/connection_adapters/oracle_enhanced_adapter'



ActiveRecord::Base.establish_connection(
  adapter:  :oracle_enhanced,
  host:     :localhost,
  port:     1521,
  database: '/orclpdb1',
  username: 'user',
  password: 'pw'
)

ActiveRecord::Schema.define do
  create_table :dummy, force: true do |t|
    t.string  :name
  end

  # Works with 6.1.2 if no columns are given
  add_index :dummy, :name, name: 'IX_DUMMY_NAME'
  remove_index :dummy, name: 'IX_DUMMY_NAME'

  # Works with 6.1.2 if no index_name is given
  add_index :dummy, :name, name: 'IX_DUMMY_NAME'
  remove_index :dummy, :name

  # Does not work if 6.1.2, but is what db:migrate now does while reversing "add_index :dummy, [:name], name: 'IX_DUMMY_NAME'"
  add_index :dummy, :name, name: 'IX_DUMMY_NAME'
  remove_index :dummy, :name, name: 'IX_DUMMY_NAME'
end

Expected behavior

Migration should be able to rollback like such one

class ExtendSchemaRights3 < ActiveRecord::Migration[6.0]
  def change
    add_index :schema_rights, [:user_id, :schema_id], name: 'IX_SCHEMA_RIGHTS_LOGICAL_PKEY'
  end
end

Actual behavior

ArgumentError: No indexes found on

with the options provided. is raised in db:rollback db:rollback calls remove_index in such cases with both column list and index name.

System configuration

Rails version: 6.1.2.1

Oracle enhanced adapter version: 6.1.2

Ruby version: jRuby 9.2.0.13

Oracle Database version: 9.3.0.0

rammpeter avatar Feb 14 '21 12:02 rammpeter

Reproduced locally.

$ ruby rep2148.rb
Fetching gem metadata from https://rubygems.org/.........
Resolving dependencies...
Using concurrent-ruby 1.1.8
Using minitest 5.14.3
Using zeitwerk 2.4.2
Using ruby-plsql 0.7.1
Using bundler 2.2.5
Using ruby-oci8 2.2.9
Using i18n 1.8.9
Using tzinfo 2.0.4
Using activesupport 6.1.2.1
Using activemodel 6.1.2.1
Using activerecord 6.1.2.1
Using activerecord-oracle_enhanced-adapter 6.1.2
-- create_table(:dummy, {:force=>true})
   -> 0.3040s
-- add_index(:dummy, :name, {:name=>"IX_DUMMY_NAME"})
   -> 0.2003s
-- remove_index(:dummy, {:name=>"IX_DUMMY_NAME"})
   -> 0.0109s
   -> 0 rows
-- add_index(:dummy, :name, {:name=>"IX_DUMMY_NAME"})
   -> 0.1827s
-- remove_index(:dummy, :name)
   -> 0.2926s
   -> 0 rows
-- add_index(:dummy, :name, {:name=>"IX_DUMMY_NAME"})
   -> 0.1882s
-- remove_index(:dummy, :name, {:name=>"IX_DUMMY_NAME"})
/home/yahonda/.rbenv/versions/3.0.0/lib/ruby/gems/3.0.0/gems/activerecord-6.1.2.1/lib/active_record/connection_adapters/abstract/schema_statements.rb:1411:in `index_name_for_remove': No indexes found on dummy with the options provided. (ArgumentError)
	from /home/yahonda/.rbenv/versions/3.0.0/lib/ruby/gems/3.0.0/gems/activerecord-oracle_enhanced-adapter-6.1.2/lib/active_record/connection_adapters/oracle_enhanced/schema_statements.rb:333:in `remove_index'
	from /home/yahonda/.rbenv/versions/3.0.0/lib/ruby/gems/3.0.0/gems/activerecord-6.1.2.1/lib/active_record/migration.rb:929:in `block in method_missing'
	from /home/yahonda/.rbenv/versions/3.0.0/lib/ruby/gems/3.0.0/gems/activerecord-6.1.2.1/lib/active_record/migration.rb:897:in `block in say_with_time'
	from /home/yahonda/.rbenv/versions/3.0.0/lib/ruby/3.0.0/benchmark.rb:293:in `measure'
	from /home/yahonda/.rbenv/versions/3.0.0/lib/ruby/gems/3.0.0/gems/activerecord-6.1.2.1/lib/active_record/migration.rb:897:in `say_with_time'
	from /home/yahonda/.rbenv/versions/3.0.0/lib/ruby/gems/3.0.0/gems/activerecord-6.1.2.1/lib/active_record/migration.rb:918:in `method_missing'
	from rep2148.rb:39:in `block in <main>'
	from /home/yahonda/.rbenv/versions/3.0.0/lib/ruby/gems/3.0.0/gems/activerecord-6.1.2.1/lib/active_record/schema.rb:50:in `instance_eval'
	from /home/yahonda/.rbenv/versions/3.0.0/lib/ruby/gems/3.0.0/gems/activerecord-6.1.2.1/lib/active_record/schema.rb:50:in `define'
	from /home/yahonda/.rbenv/versions/3.0.0/lib/ruby/gems/3.0.0/gems/activerecord-6.1.2.1/lib/active_record/schema.rb:46:in `define'
	from rep2148.rb:24:in `<main>'
$
$ more rep2148.rb
require 'bundler/inline'

gemfile(true) do
  source "https://rubygems.org"
  git_source(:github) { |repo| "https://github.com/#{repo}.git" }
  gem 'activerecord', '6.1.2.1'
  gem 'activerecord-oracle_enhanced-adapter', '6.1.2'
  gem 'ruby-oci8'
end

require 'active_record'
require 'active_record/connection_adapters/oracle_enhanced_adapter'


ActiveRecord::Base.establish_connection(
  adapter:  :oracle_enhanced,
  host:     :localhost,
  port:     1521,
  database: '/orclpdb1',
  username: 'oracle_enhanced',
  password: 'oracle_enhanced'
)

ActiveRecord::Schema.define do
  create_table :dummy, force: true do |t|
    t.string  :name
  end

  # Works with 6.1.2 if no columns are given
  add_index :dummy, :name, name: 'IX_DUMMY_NAME'
  remove_index :dummy, name: 'IX_DUMMY_NAME'

  # Works with 6.1.2 if no index_name is given
  add_index :dummy, :name, name: 'IX_DUMMY_NAME'
  remove_index :dummy, :name

  # Does not work if 6.1.2, but is what db:migrate now does while reversing "add_index :dummy, [:name], name: 'IX_DUMMY_NAME'"
  add_index :dummy, :name, name: 'IX_DUMMY_NAME'
  remove_index :dummy, :name, name: 'IX_DUMMY_NAME'
end
$

yahonda avatar Feb 16 '21 10:02 yahonda

It looks like this line does not return anything in this case. https://github.com/rails/rails/blob/130c128eae233bf71231c73b9c3c3b3f3ede918b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb#L1405

          matching_indexes = indexes(table_name).select { |i| checks.all? { |check| check[i] } }

@rammpeter Would you consider opening a pull request? I think we can have our own index_name_for_remove method.

yahonda avatar Feb 25 '21 09:02 yahonda