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

Allow users to specify the destination schema name #64

Merged
merged 13 commits into from
Apr 4, 2023
Merged

Conversation

Iain-S
Copy link
Collaborator

@Iain-S Iain-S commented Mar 29, 2023

We manually set the schema name in for the whole connection session using an explicit SET SEARCH_PATH TO statement.

This allows the user to have a different destination schema name than the source schema.

The disadvantage is that we cannot support multiple source schemas (not that we did previously, but this makes it harder), though postgres does allow you to set more than one schema on a search path with SET SEARCH_PATH TO schema1, schema2;.

Would appreciate some testing on a real database.

@Iain-S Iain-S added the WIP Work in Progress (do not merge) label Mar 29, 2023
@Iain-S Iain-S requested review from mhauru and myyong March 31, 2023 12:34
@Iain-S Iain-S removed the WIP Work in Progress (do not merge) label Mar 31, 2023
@Iain-S
Copy link
Collaborator Author

Iain-S commented Mar 31, 2023

I'm not sure what will happen if the src schema is myschema and the dst schema is left empty. I think it will try to make the tables in the public schema. Is that undesirable? Should the default behaviour be to set the dst schema the same as the src one?

@Iain-S Iain-S changed the title Add FAQ with dst schema question Allow users to specify the destination schema name Apr 3, 2023
Copy link
Collaborator

@mhauru mhauru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I only have a few style questions/points. Also, happy with how make_tables now avoids calling run (at the cost of some importlib stuff, but no free lunch).

@mhauru
Copy link
Collaborator

mhauru commented Apr 3, 2023

I'm not sure what will happen if the src schema is myschema and the dst schema is left empty. I think it will try to make the tables in the public schema. Is that undesirable? Should the default behaviour be to set the dst schema the same as the src one?

I wouldn't mind it if it used public. Given that it's the postgres default, I don't think it's unreasonable to have it be our default too.

@mhauru
Copy link
Collaborator

mhauru commented Apr 4, 2023

I tried running with and without dst_schema set, ran into a couple of tiny issues. I pushed a commit fixing them, with those my manual little test case works.

@Iain-S Iain-S mentioned this pull request Apr 4, 2023
@Iain-S Iain-S merged commit 746fc92 into main Apr 4, 2023
@Iain-S Iain-S deleted the is/dst-schema branch April 4, 2023 18:13
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 this pull request may close these issues.

2 participants