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

Composer.json should require doctrine/dbal ^2.5 #7

Closed
oniric85 opened this issue Dec 30, 2015 · 6 comments
Closed

Composer.json should require doctrine/dbal ^2.5 #7

oniric85 opened this issue Dec 30, 2015 · 6 comments

Comments

@oniric85
Copy link

I've just found an error when trying to use this library with Doctrine DBAL 2.4.4:

PHP Fatal error:  Call to undefined method Doctrine\DBAL\Platforms\MySqlPlatform::getBinaryTypeDeclarationSQL() in /vendor/ramsey/uuid-doctrine/src/UuidBinaryType.php on line 43

Seems like that method is available from version 2.5 of DBAL.

@ramsey ramsey closed this as completed in 4834bf3 Jan 3, 2016
@ramsey
Copy link
Owner

ramsey commented Jan 3, 2016

Actually, I'm re-opening this issue, since I realized that my recent commits to fix this have undone the changes made by @EdgarPE in PR #4.

@EdgarPE, any thoughts on this? doctrine/dbal 2.5 is when the method Doctrine\DBAL\Platforms\AbstractPlatform::getBinaryTypeDeclarationSQL() was added, and we are using this method in Ramsey\Uuid\Doctrine\UuidBinaryType. If someone is locked to doctrine/dbal 2.3.x or 2.4.x, and they attempt to use uuid_binary provided by this library, then they'll see this fatal error.

I'll wait to tag a release until after I hear back. I want to make sure that this isn't going to hurt others who use ramsey/uuid-doctrine with doctrine/dbal 2.3. If it could cause problems, I might tag it as 1.1.0 instead of 1.0.2.

Did doctrine/dbal 2.3 provide a means to get the binary field type for the platform's SQL declaration? If so, maybe we just go back to using that, instead of getBinaryTypeDeclarationSQL().

@ramsey ramsey reopened this Jan 3, 2016
@ramsey ramsey closed this as completed in abbcbe5 Jan 3, 2016
@ramsey
Copy link
Owner

ramsey commented Jan 3, 2016

@oniric85 and @EdgarPE, please take a look at commit abbcbe5. I think it solves this issue and allows the use of doctrine/dbal 2.3 and 2.5. Please let me know if you think there are any issues that could arise from this change.

Thanks!

@ramsey
Copy link
Owner

ramsey commented Jan 3, 2016

Keeping issue open until others feel the changes work well for both versions of doctrine/dbal.

@ramsey ramsey reopened this Jan 3, 2016
@EdgarPE
Copy link
Contributor

EdgarPE commented Jan 17, 2016

@ramsey Our software is updated to use symfony 3.0, so we use doctrine/dbal 2.5 now. At the moment there is no real world app I could test the patch with. I think it's a maintainer decision weather the library supports the older (dbal) library or the newer one with it's new api.

This adaptive solution in the patch I think it might cause problems, but I'm not sure. Maybe, when someone updates the dbal lib with existing database? After lib update defines new tables with uuid, then uuid columns would exist in the db with different db types. Does not sound really good to me.

@oniric85
Copy link
Author

I think @EdgarPE is right, the depicted scenario seems pretty scary. Maybe @ramsey you could release a new major version specifying the BC break so anyone which happens to be in this pre-2.5 situation has a chance to make proper migrations if needed? Not sure if this is too much overkill though.

@ramsey
Copy link
Owner

ramsey commented Feb 1, 2016

Thanks for weighing in on this, @EdgarPE and @oniric85. I've reverted commit 23c86eb, and we'll support doctrine/dbal ^2.5 starting in version 1.1.0 of this library.

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

3 participants