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

Postgres: Add support for schemas containing special characters #446

Merged
merged 2 commits into from
May 25, 2023

Conversation

alfredringstad
Copy link
Contributor

As stated in #445, the MigrationsTableExists function does not properly query when the schema contains special characters, since it is then querying for the quoted value.

This PR makes it use unquoted values instead, and adds tests for this behaviour (the tests will fail on the current main branch).

@gregwebs
Copy link

awesome, thank you for the contribution!
@dossy let's get this merged.

Copy link
Collaborator

@dossy dossy left a comment

Choose a reason for hiding this comment

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

Nice, this looks good.

While I think we can merge this PR as-is, I would love to see three more test cases, where drv.migrationsTableName is set to a schema.tablename where schema contains a character that would require quoting in one case, tablename contains a character that would require quoting in another case, and where both schema and tablename contain characters that require quoting in the third case. I fully expect the code to pass these additional tests as-is, so it's not essential to have the tests, but it would be nice.

This is purely for completeness, and like I said, I think this PR can definitely be merged as-is, but if @alfredringstad agrees and adds these three additional tests, I would be happy to wait on merging until they're added.

@dossy dossy merged commit 9181d85 into amacneil:main May 25, 2023
@alfredringstad
Copy link
Contributor Author

Thanks! ☺️

@mijamo
Copy link

mijamo commented May 29, 2023

Hello! Would it be possible to get a patch release with those changes included @dossy ? I am unsure about the release process so forgive me if that was already planned

@amacneil
Copy link
Owner

Done!

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.

Postgres Schemas with special characters have DBMate create migration tables then cannot find it again
5 participants