Schema search path should not be included in schema.rb when a migration is executed.

[rapito]

When executing a migration the schema search path should not be included on the schema.rb since this is Environment dependant and will break on a different configuration:

Example of schema.rb generated because of scenic:

  create_view "my_schema_search_path.foo", materialized: true, sql_definition: <<-SQL
      SELECT * from foo;
  SQL
  add_index "my_schema_search_path.foo", ["bar"], name: "index_on_foo_bar"

Expected schema.rb appendix is:

  create_view "foo", materialized: true, sql_definition: <<-SQL
      SELECT * from foo;
  SQL
  add_index "foo", ["bar"], name: "index_on_foo_bar"

To be more specific, this breaks the following rake task for any env it runs with a different configuration than the one used to generate the schema.rb file.

rake db:schema:load

[derekprior]

Is it not reasonable to expect schema names to be consistent across environments?

[rapito]

There are several use cases where the schema_search_path is what changes across the environments and the db stays the same, thats why activerecord does not include the search_path on the schema.rb.

Is it not reasonable to expect schema names to be consistent across environments?

So in reality, adding the schema name makes it inconsistent because it changes every time it runs /is generated ​on a different schema_search path.

[jdejong]

I agree with this approach since it directly aligns with ActiveRecord's approach.

Additionally, many DBaaS providers use Schema or Database names as a way to uniquely identify databases and companies in their platform so you might not have control over either across your environments.

[derekprior]

This seems like it would be a breaking change given that the reason this exists in the first place was due to users taking the opposite approach. However, given that Rails now has native multiple database support, it may be worth revisiting. I haven't done anything personally with multi db support though.

@calebthompson what do you think?

[calebhearth]

many DBaaS providers use Schema or Database names as a way to uniquely

identify databases and companies I haven’t run into this before. Can you share a docs example for using schema names here? Heroku does database names for hobby-tier databases but schemas are new to me.

On April 15, 2021, GitHub ***@***.***> wrote: This seems like it would be a breaking change given that the reason this exists in the first place was due to users taking the opposite approach. However, given that Rails now has native multiple database support, it may be worth revisiting. I haven't done anything personally with multi db support though. @calebthompson <https://github.com/calebthompson> what do you think?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#325 (comment)- 820681280>, or unsubscribe <https://github.com/notifications/unsubscribe- auth/AAFNYJLRBOJHWZVN22F4BSDTI45RNANCNFSM4ZV3TQ3A>.

[jdejong]

Hi @calebthompson, yup Heroku uses database names, I think it was ClearDB that used unique schemas.

Regardless this breaks from normal rails conventions, and as @derekprior mentioned, will also break multi-database support in Rails 6/6.1.

Perhaps, making this an opt-out/in behavior with the default either enabled or disabled depending on whether it is opt-in or out?

[rapito]

@derekprior @calebthompson
This is an example on how the schema_search_path can be used on database yml configurations:

default_pg: &default_pg
  adapter: postgresql
  username: <%= ENV['PG_DB_USR'] %>
  password: <%= ENV['PG_DB_PW'] %>
  host: <%= ENV['PG_DB_HOST'] %>
  database: <%= ENV['PG_DB_DATABASE'] %>
  schema_search_path: <%= ENV['PG_DB_SCHEMA'] %>
  port: <%= ENV['PG_DB_PORT'] %>

Besides that, (as mentioned before) the way scenic is working right now is a breaking behavior since it forces the search path on the schema.rb and rails doesn't do this so it's also a breaking convention.

If you open up any schema.rb after running a scenic view migration you can verify that only the scenic entries are doing this:

create_table "foo", id: :serial, force: :cascade do |t|
  t.integer "bar"
  t.index ["bar"], name: "random_index_name_0"
end

create_view "this_is_development.my_view", materialized: true, sql_definition: <<-SQL
	SELECT foo.bar
 FROM foo foo;
SQL
add_index "this_is_development.my_view", ["bar"], name: "random_index_name_1"

As @jdejong proposed, at the very least I would like to push for an opt-out mechanism. 🙏

[derekprior]

I agree with this approach since it directly aligns with ActiveRecord's approach.

This from @jdejong is the most compelling argument that our current implementation is wrong. Can this be substantiated by anyone? It's not that I don't trust @jdejong but since we're going back on an earlier change, I'd love some verification.

[kopchickm]

@derekprior We'd love to have this functionality, too. Since Rails conventions don't include the schema when creating tables, the current implementation here is breaking for us since our different environments have different database (and schema) names. I'd love to see #327 merged. Thanks!

[rapito]

I agree with this approach since it directly aligns with ActiveRecord's approach.

This from @jdejong is the most compelling argument that our current implementation is wrong. Can this be substantiated by anyone? It's not that I don't trust @jdejong but since we're going back on an earlier change, I'd love some verification.

Hopefully the above comment serve as verification? @derekprior