Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support of PostgreSQL schemas #163

Open
wuarmin opened this issue Mar 1, 2017 · 21 comments
Open

Support of PostgreSQL schemas #163

wuarmin opened this issue Mar 1, 2017 · 21 comments

Comments

@wuarmin
Copy link

wuarmin commented Mar 1, 2017

It would be an improvement if we can make use of PostgreSQL schemas in rom-sql. Sequel API supports it using the double underscore special notation.

Especially for legacy tables, it would be easier to migrate to rom-sql.

@flash-gordon
Copy link
Member

We'll be working on this, the only reason why this isn't here yet is that PG has the search_path setting which allows you to lookup several schemas at once.
https://www.postgresql.org/docs/9.6/static/runtime-config-client.html
You can use it either by providing a proper connection string, like postgres://localhost/rom_sql?search_path=schema1,schema2 or by configuring the connection with a proc, like

rom = ROM.container(:sql, DB_URL, after_connect: -> c { c.execute("set search_path to schema1,schema2") })

@solnic solnic closed this as completed in bc5c2e5 Mar 1, 2017
@solnic solnic reopened this Mar 1, 2017
@solnic
Copy link
Member

solnic commented Mar 1, 2017

ugh sorry I closed it by an accident, re-opened now

@solnic
Copy link
Member

solnic commented Mar 1, 2017

Do you want to do this as part of 1.x or 2.0?

@flash-gordon
Copy link
Member

I'd go with 2.0, there is a workaround for PG and in Oracle you can create a synonym. Supporting schemas may require some changes which I'd align with automigrations.

@solnic solnic added this to the v2.0.0 milestone Mar 2, 2017
@solnic solnic added the feature label Mar 2, 2017
@solnic solnic removed this from the v2.0.0 milestone Oct 23, 2017
@solnic solnic added the postgres label Nov 3, 2017
@wuarmin
Copy link
Author

wuarmin commented Oct 8, 2021

Hello,
what is the state of this issue? Is it planned to add the schema feature?
Thanks.
Best regards

@alex-lairan
Copy link

There is any way I can help with this issue?

All I see that can be problematic is that the schema waits for a symbol and allow Sequel::SQL::QualifiedIdentifier, but the has_many only allow symbol.

Also, the full name of the table is not found with the Sequel::SQL::QualifiedIdentifier.

This works for simple actions.

schema(Sequel::SQL::QualifiedIdentifier.new('x', 'foo'), as: :x_foo)

This does not works because the name is "'x'.'foo'".

schema(:'x.foo'), as: :x_foo)

@flash-gordon
Copy link
Member

it will require rom's identifiers to support schemas for a start, then these identifiers have to be used consistently across from and rom-sql. I tried to implement it once but I didn't succeed, it was too much work. Initially, I used search_path but then I ditched it in favor of a single schema.

@wuarmin
Copy link
Author

wuarmin commented Jan 20, 2022

I solve the schema issue meanwhile with the search_path and am satisfied with it.

@alex-lairan
Copy link

I solve the schema issue meanwhile with the search_path and am satisfied with it.

How does it works when you have x.foo and y.foo tables?

@wuarmin
Copy link
Author

wuarmin commented Jan 21, 2022

With my current projects, I have the situation that I design the db myself. so i can avoid this restriction. otherwise only a renaming helps, or a workaround with views.

@solnic
Copy link
Member

solnic commented Jan 24, 2022

@flash-gordon with a little bit of guidance I could try to make it work. My only worry is that it would make identifiers more complex everywhere, whereas we only need it in rom-sql.

@alex-lairan
Copy link

@flash-gordon with a little bit of guidance I could try to make it work. My only worry is that it would make identifiers more complex everywhere, whereas we only need it in rom-sql.

I found a solution to allow Sequel::SQL::QualifiedIdentifier for joins, maybe create a special type that will be able to transform the schema name to a QualifiedIdentifier and be able to communicate with non-SQL schemas.

I will open a PR today for the 1st part.

@solnic
Copy link
Member

solnic commented Jan 24, 2022

I will open a PR today for the 1st part.

@alex-lairan it'd be better to do it in 4.0.0. I'm wrapping up update-to-rom-6 branch and once it's in master, it'd be a better moment for you to work on this feature. Sounds good?

@alex-lairan
Copy link

I will open a PR today for the 1st part.

@alex-lairan it'd be better to do it in 4.0.0. I'm wrapping up update-to-rom-6 branch and once it's in master, it'd be a better moment for you to work on this feature. Sounds good?

I will have to do it anyway because it's a blocking issue for us :)

@solnic
Copy link
Member

solnic commented Jan 24, 2022

@alex-lairan OK so let's see what you have and I'll think about how it could be backported to 4.0.0!

@alex-lairan
Copy link

@solnic I have created the related PR #405

It works fine on our product, if you like it I will add tests to it.

@flash-gordon
Copy link
Member

My only worry is that it would make identifiers more complex everywhere, whereas we only need it in rom-sql.

@solnic I'm not sure if this is only applicable to SQL, I guess scopes like schemas/databases/etc could be found in other backends as well. Anyhow, SQL is a big part of persistence to justify the change. OTOH, rom-sql could have its own identifiers.

@alex-lairan
Copy link

I have other cases of join error with Schemas.
I will update my PR locally to handle them as I don't want it to mix with this kind of query.

Can anyone review my current PR?

@solnic
Copy link
Member

solnic commented Mar 13, 2022

@alex-lairan sounds good, please add tests and I'll take care of releasing it

@wuarmin
Copy link
Author

wuarmin commented Nov 10, 2022

@alex-lairan do you have some status/input?
Thanks

@alex-lairan
Copy link

@wuarmin sorry did not had time to do it.
I now have assigned time to make it pass!

Stay tuned :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants