TalentBox / sequel-rails

A gem for using Sequel with Rails 5.x, 6.x, 7.x
http://talentbox.github.io/sequel-rails/
MIT License
324 stars 81 forks source link

:ruby schema format type not taking enums into account #115

Closed HoneyryderChuck closed 8 years ago

HoneyryderChuck commented 8 years ago

I have an enum created in a migration:

create_enum(:addressing_protocol, %w(itchy scratchy))
....

When I perform db:schema:dump, this enum is not recreated, hence my appliance of `db:schema:load`` to the test database will fail because:

PG::UndefinedObject: ERROR:  type "addressing_protocol" does not exist 

:sql as schema_format strategy at least runs.

JonathanTron commented 8 years ago

Hi @TiagoCardoso1983, thanks for reporting this issue.

Unfortunately we're using Sequel's schema dumper, and it doesn't handle all db features when exporting. If you use a feature not supported by the :ruby schema you have to use the :sql one.

HoneyryderChuck commented 8 years ago

@JonathanTron see #116 , which is the reason I can't really do it (sql structure also dumps search path)

JonathanTron commented 8 years ago

I understand, if you need it, maybe you can propose a PR to Sequel to handle it.

HoneyryderChuck commented 8 years ago

I don't think this comes from sequel: https://github.com/TalentBox/sequel-rails/blob/ad1a51b3ee45f90c99e28413bcc9d45c1eaf2eb8/lib/sequel_rails/storage/abstract.rb#L124-L141

So the dump_schema_information from SequelRails::Storage::Postgres calls this abstract class method (why abstract class? I thought search path was a sequel only feature). I'd say, one could these lines and it would magically work, as the migrations already run under the search_path defined within the db connection (and this, yes, is sequel feature).

What do you think? do you see any implications @JonathanTron ? Could you reopen the ticket?

JonathanTron commented 8 years ago

From what I understood, when using the ruby schema, the enum definition is not exported correctly, is that I right?

If it's the case, then it's a feature which need to be implemented in Sequel's schema dumper.

What you describe now seems to be what is in #116, which I agree is a problem in your use-case.

Do I misunderstood something ?

HoneyryderChuck commented 8 years ago

Ups. You are right, it has to do with #116.