Skip to content

Pdomysql #1403

Closed
wants to merge 2 commits into from

5 participants

@ianmacl
ianmacl commented Jul 24, 2012

Now that the stuff in JDatabaseMysql is no longer needed it makes sense to implement a PDO-MySQL based driver since according to http://php.net/manual/en/function.mysql-connect.php use of the MySQL extensions is discouraged. This pull request replaces the old Mysql driver with a PDO based driver and makes it possible to use prepared statements with MySQL.

@realityking
Joomla! member

Interesting stuff. But wouldn't that be better as new driver? (Mysqlpdo for example)

@ianmacl
ianmacl commented Jul 25, 2012

Mysqlpdo is kind of awkward. Seems to make more sense just to use this as eventually we'll want to be PDO only IMO.

@realityking
Joomla! member

We do? Mysqli has a couple of nice things (asynchronous queries mostly) that PDO doesn't (yet) offer that could be useful in the future. From what we use right now there not really a problem with PDO's feature set thou.

Prepared statements can be done with mysqli thou, we need to abstract the statement class thou. I like the approach of Doctrine DBAL that emulates a (large) subset of the PDOStatement library for mysqli.

@pasamio
pasamio commented Jul 28, 2012

I'm skeptical of pulling everything over to PDO or that we will be PDO for everything at some point in the abstract future. PDO in my experience comes with a large selection of it's own baggage which you have to learn and understand the nuance as well as working out the particular nuances of each database layer as well. There was a time when PDO wasn't even maintained by anyone in the PHP core but I'm not sure if this is still a true statement. PDO will always lag behind what native drivers provide simply due to it's design. It is likely that we'll live in a world of PDO + native drivers for a while yet.

@ianmacl
ianmacl commented Aug 10, 2012

Either way, I think we can agree that we want to get rid of the MySQL driver and from a naming perspective calling this one MySQL makes sense, IMO. There is precedence in that other PDO based drivers aren't prefixed.

Anyway, if the decision is to rename it Mysqlpdo, that's fine. I just think it is awkward.

@dongilbert

Where does this stand? I'm with @ianmacl that we should be 100% PDO for our database drivers. Other major PHP frameworks (Laravel, FuelPHP, even CodeIgniter) are either already 100% PDO with their own specific implementations (such as Laravel's Eloquent ORM - smexy!) or have plans to get that way. I don't see why Joomla should be any different in those regards.

@realityking - your reasoning that PDO lags behind native drivers is entirely valid. However, there are MANY parts of Joomla that lag behind the native abilities of PHP. (Namespaces, anyone?) So I don't think that it's a good enough reason to hold off on getting the joomla platform entirely shifted over to PDO.

And my vote (as you can guess) for naming it Mysqlpdo, is no.

@pasamio
pasamio commented Nov 8, 2012

The reason that Joomla lags behind is because we purposely hold ourselves back to ensure that the majority of the hosting providers can run the application. The Platform only just switched to PHP 5.3 because of the CMS and because there is a lag between what PHP supports and what hosting providers offer. The lack of support for features I a sense is intentional. If anything this makes the reason PDO lags an even more significant reason: a significant number of our customers are on a lagging PHP version.

Given that I'm probably ready to contribute a replacement Oralce driver that uses the native OCI functions instead of PDO version and in the past I've talked to Louis about issues with SQLite PDO I'm not in favour of PDO in general. There are numerous issues and incompatibilities that PDO tries to hide badly. I think for me the most interesting thing that prompted the shift for OCI for me from the PDO Oracle was that Propel, which is a reasonable mature PRM for PHP, actually failback to the OCI functions to make up for the fact that PDO Oracle outright doesn't work. There are plenty of other places and while at the moment the PDO library is presently being supported, there was a time during the early 5.0 series where it had no maintainer at all. And in a sense not all of the drivers are well supported.

Given there is an existing MySQL driver, I think we should keep it clear for the time being.

@ianmacl
ianmacl commented Nov 8, 2012

I'm not really arguing that we shouldn't keep the Mysqli driver around. If we don't want to go the whole hog down the PDO path, that's fine. It obviously seems to be something that differs on a case by case basis. These days there are three MySQL APIs - MySQL, MySQLi and PDO-MySQL. The PHP website states fairly clearly that out of the three, MySQL is the one not to use. My impression is that as far as MySQL is concerned, MySQLi and PDO are about on par as far as support is concerned with at least a few of the core developers preferring PDO. Thus I think it is a reasonable choice to support MySQLi and PDO but not MySQL.

As far as PDO in general, I guess it really depends on the vendor and what sort of support they choose to provide. It seems Microsoft has gotten on board with PDO but it appears that Oracle has stuck with their own OCI driver. Which is fine. It seems evident that we have to choose APIs carefully based on the DB in question. I would argue that for MySQL PDO is a reasonable choice, but it really isn't a hill I'm willing to die on. I think we should add support for PDO-Mysql, and if we'd rather call it pdomysql or whatever, I'm fine with that. I will even do the work do rename it if we can come to a decision.

@dongilbert

Also relevant - https://wiki.php.net/rfc/mysql_deprecation

This is an RFC that proposes to generate E_DEPRECATED errors when connecting to a MySQL database with the ext/mysql API. Opened Nov 12th.

@ianmacl
ianmacl commented Dec 25, 2012

So the deprecation is really beside the point, although it should be a pretty clear indicator that ext/mysql is on its way out when as of today there is a 25-12 vote in favour of adding deprecation notices. I think we can all agree that we probably don't need the Mysql driver as it is today around anymore - the CMS defaults to MySQLi and I'm not really aware of anything in the way of issues that have arisen.

The only thing I think we have to figure out is what we want to call this set of classes.

@eddieajau

Ian, how about we do this in the FW?

@eddieajau eddieajau closed this Mar 16, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.