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

Fix problem with database names using hyphens #299

Merged
merged 1 commit into from
Jul 29, 2022
Merged

Fix problem with database names using hyphens #299

merged 1 commit into from
Jul 29, 2022

Conversation

icanhazstring
Copy link
Contributor

@icanhazstring icanhazstring commented Jul 29, 2022

Hi. First off all. Awesome lib. Thanks <3

While working to get it running, I noticed that there is a problem when creating a database where the name has hyphens in it. Using this, the name must be used with backticks.

@icanhazstring
Copy link
Contributor Author

Will squash/rebase if build is green to have a single commit :)

Copy link
Member

@DavidBadura DavidBadura left a comment

Choose a reason for hiding this comment

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

Hey, first of all thanks and good catch! We seem to have missed this case.

I looked again at how it was solved in the CreateDatabaseCommand of doctrine bundle and found these lines:

https://github.com/doctrine/DoctrineBundle/blob/2.7.x/Command/CreateDatabaseDoctrineCommand.php#L114

I would also do the quoting via dbal, since i don't know which databases need special treatment :D

In the CreateDatabaseCommand it is checked whether a path exists, if so, the quoting is skipped. I don't know if it is relevant to us. I think it only happens with sqlite?

src/Console/DoctrineHelper.php Outdated Show resolved Hide resolved
src/Console/DoctrineHelper.php Outdated Show resolved Hide resolved
@DavidBadura DavidBadura added the bug Something isn't working label Jul 29, 2022
@icanhazstring
Copy link
Contributor Author

Good point absolutely. Was thinking about it also.

In the CreateDatabaseCommand it is checked whether a path exists, if so, the quoting is skipped. I don't know if it is relevant to us. I think it only happens with sqlite?

Yes. I think this is only sqlite where the db is stored as a file.

@DavidBadura
Copy link
Member

Okay, i would ignore the "path" case for now. If the pipeline is green i would merge it. 👍
Oh, can you change the target branch to 2.0.x since it is a bug? Would then ship it with version 2.0.2

@icanhazstring icanhazstring changed the base branch from 2.1.x to 2.0.x July 29, 2022 12:52
This will fix an issue while trying to drop/create a database having hyphens:
E.g. `event-store-db`
@icanhazstring
Copy link
Contributor Author

Squashed and rebased onto 2.0.x 👍

@DavidBadura DavidBadura merged commit 26f77f3 into patchlevel:2.0.x Jul 29, 2022
@DavidBadura DavidBadura added this to the 2.0.2 milestone Jul 29, 2022
@DavidBadura
Copy link
Member

Thank you!

@icanhazstring icanhazstring deleted the patch-1 branch July 29, 2022 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants