search

EmpaticoOrg / scenic-mysql_adapter

MySQL adapter for thoughtbot/scenic
MIT License
30 stars 15 forks source link

schema.rb contains both `create_table` and `create_view` for newly created views #2

Closed josepegea closed 6 years ago

josepegea commented 6 years ago

How to reproduce

Add a new view using a migration with create_view as suggested in the docs.

Review db/schema.rb and you'll find that 2 new sections have been added to the file:

 create_table "supply_sets", id: false, force: :cascade do |t|
   t.integer "id",            limit: 4,   default: 0,  null: false
   (rest of columns here)
 end

and

 create_view "supply_sets",  sql_definition: <<-SQL
     (SQL view definition here)
 SQL

Why is this a problem?

If you try to prepare the test DB you get an error, as it first creates a table and then it tries to create a view with the same name.

graymatter:pfm161-v4 jes$ spring rake db:test:prepare
Running via Spring preloader in process 10807
rake aborted!
ActiveRecord::StatementInvalid: Mysql2::Error: Table 'supply_sets' already exists: CREATE VIEW `supply_sets` AS       (rest of view definition here)

What can be done?

The problem seems to come from MySQL including views when you ask for a list of tables.

Some tweak on SchemaDumper#ignore? could help to omit the views when generating the tables section of the schema.rb file.

cainlevy commented 6 years ago

I wonder if this could be a Rails version issue? We've only used 5.x with this adapter so far.

josepegea commented 6 years ago

Good point! I created a barebones Rails 5 project and the schema.rb file is generated correctly when adding a view.

I'll look further to see if it's a problem with Rails 4.2 or something else related to our project.

cainlevy commented 6 years ago

I found this in the Rails 5.1 release notes: https://github.com/rails/rails/commit/5973a984c369a63720c2ac18b71012b8347479a8

Looks like the MySQL adapter previously mixed views and tables together, but the PostgreSQL adapter did not. Hence, thoughtbot/scenic didn't need any extra patches to support 4.x or 5.0 but this extension might.

josepegea commented 6 years ago

You nailed it! That seems to be the case! I'll try to add some fix for previous versions

cesargomez89 commented 6 years ago

Same issue here, any update?

cainlevy commented 6 years ago

I haven't been working on a patch for this issue. I'd be happy to look at a pull request that either:

josepegea commented 6 years ago

I ended up adding a fix directly in my project.

I have a PR for the gem half in the works. I'll try to find some time during the weekend to finish it.

On Thu, May 17, 2018, 18:58 Lance Ivy notifications@github.com wrote:

I haven't been working on a patch for this issue. I'd be happy to look at a pull request that either:

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/EmpaticoOrg/scenic-mysql_adapter/issues/2#issuecomment-389937097, or mute the thread https://github.com/notifications/unsubscribe-auth/AWIrlvGiJqXV_mmoCEaQvS_rtpgPOaCWks5tzaw2gaJpZM4SpB8C .

-- This is a PRIVATE message. If you are not the intended recipient, please  delete without copying and kindly advise us by e-mail of the mistake in  delivery. NOTE: Regardless of content, this e-mail shall not operate to bind Platform161 to any order or other contract unless pursuant to explicit written agreement or government initiative expressly permitting the use of e-mail for such purpose.