imdrasil / jennifer_sqlite3_adapter

SQLite3 adapter for Jennifer ORM
MIT License
13 stars 7 forks source link

Problem with dropping an indexed column in a migration #35

Closed richardboehme closed 3 years ago

richardboehme commented 3 years ago

Dropping an indexed column fails with the message:

no such column: column.
Original query was:
CREATE UNIQUE INDEX table_column ON table (column)

Example:

change_table :table do |t|
  t.drop_index :column
  t.drop_column :column
end

The drop_column method in the SQLite3 adapter was just recently added and tries to create all indexes on the new table. However, if the dropped column has an index, this fails. I see two solutions to this:

  1. Change the order of execution in a change_table migration block, so that an index is dropped before the columns are dropped (this would be a fix in Jennifer, I think).
  2. Tell the Jennifer::SQLite3::SchemaProcessor#drop_column to create each index except the ones that contain the dropped column. However this would automatically drop all indexes related to a dropped column when calling #drop_column. I'm not sure how other ORMs handle this situation but I think this may be too implicit.

Possible workaround: Use #drop_index in a separate call in the migration like this:

drop_index(:table, :column)
change_table :table do |t|
  t.drop_column :column
end

Environment:

Crystal: 1.1.1 jennifer: 0.11.1 jennifer_sqlite3_adapter: 0.3.2

imdrasil commented 3 years ago

Hi, the error message you provided looks a bit weird but I see the problem. It is a bug in Jennifer (at least taking into account current "architecture" used for table manipulations). Will address it tomorrow (likely)

richardboehme commented 3 years ago

Hi, the error message you provided looks a bit weird but I see the problem. It is a bug in Jennifer (at least taking into account current "architecture" used for table manipulations). Will address it tomorrow (likely)

Yeah I was confused from the error message at first. But after looking at the way #change_column in Jennifer and #drop_column in the SQLite3 adapter work it made sense to me. Thanks for looking into this!

imdrasil commented 3 years ago

Jennifer received a fix for this issue