PDO drivers refactoring with preload, new drivers for firebird and DB2

[dregad]

This PR's purpose is to facilitate review of @mnewnham's recent work on PDO drivers.

Includes updated mysql, sqlite, pgsql, sqlsrv and oci drivers, plus new drivers for firebird and ibm db2.

The only change required for legacy code is to update the 'ADONewConnection() call, as follows:

before

$db = ADONewConnection('PDO');
$db->connect('mysql:<parameters>');

after

$db = ADONewConnection('PDO\mysql');

All of the driver files are loaded as necessary by ADOLoadCode(). This allows the class to load properly i.e. 'ADORecordset_pdo_mysql -> ADORecordset_pdo -> ADORecordset', rather than the current method, where 'ADORecordset' loads 'ADORecordset_pdo_mysql'

[mnewnham]

See more about this new driver At the new documentation page

[dregad]

To be on the safe side, just in case there's another \ somewhere in the DSN, I would suggest

explode('\\', $db, 2);

[mnewnham]

I think that as it stands, provision of the DSN on the newADOcommand line is no longer supported. Not to say that the processing of the parameter should be super robust. I did note it in the Quick upgrade notes.

[dregad]

provision of the DSN on the newADOcommand line is no longer supported
I did note it in the Quick upgrade notes.

I missed that bit.
Is there a particular reason for dropping this functionality ? It could be fairly easily maintained by adapting the PDO-specific logic to use the $pdoExtension variable:

diff --git a/adodb.inc.php b/adodb.inc.php
index c5e30f09..6ff88c0b 100644
--- a/adodb.inc.php
+++ b/adodb.inc.php
@@ -5547,17 +5547,11 @@ class ADORecordSet implements IteratorAggregate {
                $dsna['host'] = '';
            }
 
-           if (strncmp($origdsn,'pdo',3) == 0) {
-               $sch = explode('_',$dsna['scheme']);
-               if (sizeof($sch)>1) {
-                   $dsna['host'] = isset($dsna['host']) ? rawurldecode($dsna['host']) : '';
-                   if ($sch[1] == 'sqlite') {
-                       $dsna['host'] = rawurlencode($sch[1].':'.rawurldecode($dsna['host']));
-                   } else {
-                       $dsna['host'] = rawurlencode($sch[1].':host='.rawurldecode($dsna['host']));
-                   }
-                   $dsna['scheme'] = 'pdo';
-               }
+           if ($pdoExtension) {
+               $host = $pdoExtension
+                   . ($pdoExtension == 'sqlite' ? ':' : ':host=')
+                   . rawurldecode($dsna['host'] ?? '');
+               $dsna['host'] = rawurlencode($host);
            }
 
            $db = @$dsna['scheme'];

[mnewnham]

Two things on future issues,

1. The ''_'' character can't be used if using off-the-shelf PSR style autoloaders, we'll probably need a project in the future to remove them from classnames. Might as well start somewhere.
2. To be honest, I don't particularly like fact that you can assign a DSN through here (and not any other type of parameter). It feels very unstructured, and I'd certainly not do it if I did the original design.

[dregad]

With this logic, we accept that whenever there is a \ in $db, we will connect to PDO regardless of what string is before it, i.e. xyz\mysql will successfully connect just like pdo\mysql.

Shouldn't we test the actual value ? Or maybe set $db = $pdoSplit[0] and let ADOLoadCode() deal with it ?

[mnewnham]

Maybe it would be better if I split the entirety of the statement

$split = preg_split('/^(pdo\\)/',$driver);

[dregad]

That would work better indeed.

I would suggest adjusting the regex: /^pdo[_\\]/

• removing the unnecessary capturing group,
• adding _ as accepted delimiter for backwards-compatibility, following my earlier suggestion (PDO drivers refactoring with preload, new drivers for firebird and DB2 #826 (comment))

[dregad]

Since $db == 'pdo' if $pdoExtension is not empty, this can be simplified

			$cls = 'ADODB_' . $db;
			if ($pdoExtension) {
				$cls .= '_' . $pdoExtension
			}

[dregad]

Also, what about performing early detection of legacy ADONewConnection('pdo') calls (i.e. when $db == 'pdo' && $pdoExtension == ''), either here or maybe even better when we split and process the pdo driver string above, and throw an error instead of delaying this until ADODB_pdo::_connect()

[mnewnham]

@dregad I have brought this repo up to date, could you take a look again please