Skip to content
This repository has been archived by the owner on May 26, 2023. It is now read-only.

Replace Mysqli with DBAL? #149

Open
lox opened this issue May 3, 2015 · 9 comments
Open

Replace Mysqli with DBAL? #149

lox opened this issue May 3, 2015 · 9 comments
Labels

Comments

@lox
Copy link
Owner

lox commented May 3, 2015

We've done some work to replace the Mysqli parts of Pheasant with Doctrine's DBAL project. Thoughts on whether this is something we'd want to do for 2.0?

@lox lox added the Question label May 3, 2015
@bjornpost
Copy link
Collaborator

There were some thoughts on replacing Mysqli with PDO in the past. I'm all in favor of making Pheasant less dependent on MySQL (read: db independent). I'm not familiair with Doctrine's DBAL project, what does it do? And what are the pros/cons versus vanilla PDO?

@lox
Copy link
Owner Author

lox commented May 4, 2015

DBAL is a database abstraction layer, where as PDO is just a connection abstraction layer. DBAL will actually rewrite SQL to suit different database engines.

Beyond that DBAL offers a nice connection api that would replace a lot of code in Pheasant, including things like better transaction handling and nice extras like sharding and caching.

@bjornpost
Copy link
Collaborator

If we want Pheasant to support more database backends, writing all that code ourselves wouldn't add much value. It just adds a bunch of complexity on our side which we have to test and maintain. I'm all in favor of using a 3rd party library to keep our own code base small. Again, not familiar with the specifics of the Doctrine's DBAL project, but Doctrine seems well tested and has a lot of eyes looking at it.

What are the implications of such a change in terms of performance and Pheasants public API?

@lox
Copy link
Owner Author

lox commented May 4, 2015

Hmmm, it depends. I'd say the biggest impact would be that we expose the classname of \Pheasant\Database\Mysqli:Connection in a lot of places.

@lox
Copy link
Owner Author

lox commented May 4, 2015

Performance implications would be minimal, and it would have huge security upside in that parameter binding could be moved into DBAL/pdo.

@bjornpost
Copy link
Collaborator

I'd say go for it 👍

@Jud
Copy link
Collaborator

Jud commented May 4, 2015

I'm for this change 👍 .

After adding in HasManyThrough (judsonco@acc4603) relationships and HasOneThrough (judsonco@3e0c82b) there's lots of hackiness around getting the current table alias and it seems like DBAL handles that quite a bit better, and the code could be simplified a good bit.

@bjornpost
Copy link
Collaborator

@lox, any progress on this?

@bjornpost
Copy link
Collaborator

@lox, can you push this to a separate branch? I have some time to work on this.

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

No branches or pull requests

3 participants