pheasant icon indicating copy to clipboard operation
pheasant copied to clipboard

Published 20 hours ago • verified publisher icon lox

Replace Mysqli with DBAL?

Open lox opened this issue 10 years ago • 9 comments
trafficstars

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 avatar May 03 '15 05:05 lox

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?

bjornpost avatar May 04 '15 07:05 bjornpost

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.

lox avatar May 04 '15 07:05 lox

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?

bjornpost avatar May 04 '15 08:05 bjornpost

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 avatar May 04 '15 08:05 lox

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

lox avatar May 04 '15 08:05 lox

I'd say go for it :+1:

bjornpost avatar May 04 '15 10:05 bjornpost

I'm for this change :+1: .

After adding in HasManyThrough (https://github.com/judsonco/pheasant/commit/acc46036b2fd4875f9807659b96876d2b590c2ea) relationships and HasOneThrough (https://github.com/judsonco/pheasant/commit/3e0c82b8119b1d2776cfc7ae6094347f7dacede2) 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.

Jud avatar May 04 '15 19:05 Jud

@lox, any progress on this?

bjornpost avatar Aug 26 '15 19:08 bjornpost

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

bjornpost avatar Feb 10 '16 15:02 bjornpost