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

Add schema support #222

Closed
lafriks opened this issue Oct 12, 2021 · 12 comments · Fixed by go-rel/sql#51
Closed

Add schema support #222

lafriks opened this issue Oct 12, 2021 · 12 comments · Fixed by go-rel/sql#51

Comments

@lafriks
Copy link
Contributor

lafriks commented Oct 12, 2021

Currently there is no way at least for migrations to create/drop schemas for databases that support them like PostgreSQL

@Fs02
Copy link
Member

Fs02 commented Oct 12, 2021

databases that support them like PostgreSQL

did you do research other than PostgreSQL?

I would love to have this if it's possible

@lafriks
Copy link
Contributor Author

lafriks commented Oct 12, 2021

I think only PostgreSQL and MSSQL support this, tho I have not used MSSQL for quite some time already :)

@Fs02
Copy link
Member

Fs02 commented Oct 12, 2021

Looks like MySQL driver also support db creation
https://github.com/go-sql-driver/mysql/blob/6cf3092b0e12f6e197de3ed6aa2acfeac322a9bb/driver_test.go#L227-L228

for sqlite3, maybe we can just connect to db and schema will be created, for deleting schema, we can delete the file

@lafriks
Copy link
Contributor Author

lafriks commented Oct 12, 2021

First step would be probably just implement for ones who support schemas and for others just print warnings

Maybe for MySQL and sqlite3 use schema as prefix for table names?

@Fs02
Copy link
Member

Fs02 commented Oct 13, 2021

ah, looks like I miss understood schema for database 🤦

Maybe for MySQL and sqlite3 use schema as prefix for table names?

if there's a way to make this API not available in this adapter maybe better, like making the CreateSchema function adapter specific 🤔

how about application usage? how will it refer to correct schema?
I think we can exec SET search_path TO schema during adpater initialization, but I'm wondering if it's a good idea 🤔

@lafriks
Copy link
Contributor Author

lafriks commented Oct 13, 2021

If you want to use multiple schemas in single app search_path is not very useful. Migration is just one part - if adapter does not support it can just ignore these commands.
Other part is table names need to prefixed with <schema>.<table> so either we need new function to interface that would like there is Table() string to also have Schema() string and adapter than could decide on how to concatenate them like for postgresql it would generate: "schema"."table" for mssql - [schema].[table] and for mysql and sqlite3 it could generate just table name schema_table

@Fs02
Copy link
Member

Fs02 commented Oct 25, 2021

sorry for late respond 🙏

what do you think if we use Table() string to specify schema? simply returns <schema>.<table>
seems, similar approach is used in activerecord https://stackoverflow.com/a/35945716/7678169

@lafriks
Copy link
Contributor Author

lafriks commented Oct 26, 2021

There will be problem handling this in adapters if only Table() is used as in sqlite and mysql schema should be used as prefix instead.

@Fs02
Copy link
Member

Fs02 commented Oct 26, 2021

I was thinking we don't have to support compatibility for all adapters, because even if we make it compatible for repository, I don't think there's a way to make it compatible for migration since table name is defined manually.

so I think it's about how far we want to support the compatibility, is it okay for migration to be not compatible? is it okay for this feature to be adapter specific? 🤔

@lafriks
Copy link
Contributor Author

lafriks commented Aug 11, 2023

Probably it would be ok also to use "." in Table for this and mysql, sqlite that does not support schemas could just use schema as table prefix

@Fs02
Copy link
Member

Fs02 commented Aug 14, 2023

sounds good to me

@lafriks
Copy link
Contributor Author

lafriks commented Aug 14, 2023

When looking at code probably best way to do this would be moving escaping of the table name to Quoter interface by adding method Table(string) string in addition to ID and Value methods. What do you think?

Looks like better as option in Buffer type

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

Successfully merging a pull request may close this issue.

2 participants