Merge pull request #3353 from tractorcow/pulls/3.2/fix-pdo

BUG Fix PDOConnector issues

.travis.yml

@@ -14,20 +14,20 @@ env:
 
 matrix:
   include:
-    - php: 5.3
-      env: DB=PGSQL CORE_RELEASE=master
-    - php: 5.3
+    - php: 5.4
       env: DB=SQLITE CORE_RELEASE=master
+    - php: 5.4
+      env: DB=PGSQL CORE_RELEASE=master
     - php: 5.4
       env: DB=MYSQL CORE_RELEASE=master
+    - php: 5.4
+      env: DB=MYSQL CORE_RELEASE=master PDO=1
     - php: 5.5
       env: DB=MYSQL CORE_RELEASE=master
     - php: 5.6
       env: DB=MYSQL CORE_RELEASE=master
     - php: 5.4
       env: DB=MYSQL CORE_RELEASE=master BEHAT_TEST=1
-    - php: 5.6
-      env: DB=MYSQL CORE_RELEASE=master
 
 before_script:
  - composer self-update

model/connect/MySQLDatabase.php

@@ -9,10 +9,10 @@
  * @subpackage model
  */
 class MySQLDatabase extends SS_Database {
-	
+
 	/**
 	 * Default connection charset (may be overridden in $databaseConfig)
-	 * 
+	 *
 	 * @config
 	 * @var String
 	 */
@@ -23,9 +23,9 @@ class MySQLDatabase extends SS_Database {
 		if(empty($parameters['driver'])) {
 			$parameters['driver'] = $this->getDatabaseServer();
 		}
-		
+
 		// Set charset
-		if( empty($parameters['charset']) 
+		if( empty($parameters['charset'])
 			&& ($charset = Config::inst()->get('MySQLDatabase', 'connection_charset'))
 		) {
 			$parameters['charset'] = $charset;
@@ -42,13 +42,13 @@ class MySQLDatabase extends SS_Database {
 		}
 
 		// SS_Database subclass maintains responsibility for selecting database
-		// once connected in order to correctly handle schema queries about 
+		// once connected in order to correctly handle schema queries about
 		// existence of database, error handling at the correct level, etc
 		if (!empty($parameters['database'])) {
 			$this->selectDatabase($parameters['database'], false, false);
 		}
 	}
-	
+
 	/**
 	 * Sets the character set for the MySQL database connection.
 	 *
@@ -68,7 +68,7 @@ class MySQLDatabase extends SS_Database {
 
 	/**
 	 * Sets the SQL mode
-	 * 
+	 *
 	 * @param string $mode Connection mode
 	 */
 	public function setSQLMode($mode) {
@@ -78,7 +78,7 @@ class MySQLDatabase extends SS_Database {
 
 	/**
 	 * Sets the system timezone for the database connection
-	 * 
+	 *
 	 * @param string $timezone
 	 */
 	public function selectTimezone($timezone) {
@@ -112,7 +112,6 @@ class MySQLDatabase extends SS_Database {
 		if (!class_exists('File'))
 				throw new Exception('MySQLDatabase->searchEngine() requires "File" class');
 
-		$fileFilter = '';
 		$keywords = $this->escapeString($keywords);
 		$htmlEntityKeywords = htmlentities($keywords, ENT_NOQUOTES, 'UTF-8');
 
@@ -131,7 +130,7 @@ class MySQLDatabase extends SS_Database {
 		// Always ensure that only pages with ShowInSearch = 1 can be searched
 		$extraFilters['SiteTree'] .= " AND ShowInSearch <> 0";
 
-		// File.ShowInSearch was added later, keep the database driver backwards compatible 
+		// File.ShowInSearch was added later, keep the database driver backwards compatible
 		// by checking for its existence first
 		$fields = $this->fieldList('File');
 		if (array_key_exists('ShowInSearch', $fields))
@@ -201,7 +200,7 @@ class MySQLDatabase extends SS_Database {
 
 			$querySQLs[] = $query->sql($parameters);
 			$queryParameters = array_merge($queryParameters, $parameters);
-			
+
 			$totalCount += $query->unlimitedRowCount();
 		}
 		$fullQuery = implode(" UNION ", $querySQLs) . " ORDER BY $sortBy LIMIT $limit";
@@ -233,30 +232,30 @@ class MySQLDatabase extends SS_Database {
 	public function transactionStart($transactionMode = false, $sessionCharacteristics = false) {
 		// This sets the isolation level for the NEXT transaction, not the current one.
 		if ($transactionMode) {
-			$this->query('SET TRANSACTION ' . $transactionMode . ';');
+			$this->query('SET TRANSACTION ' . $transactionMode);
 		}
 
-		$this->query('START TRANSACTION;');
+		$this->query('START TRANSACTION');
 
 		if ($sessionCharacteristics) {
-			$this->query('SET SESSION TRANSACTION ' . $sessionCharacteristics . ';');
+			$this->query('SET SESSION TRANSACTION ' . $sessionCharacteristics);
 		}
 	}
 
 	public function transactionSavepoint($savepoint) {
-		$this->query("SAVEPOINT $savepoint;");
+		$this->query("SAVEPOINT $savepoint");
 	}
 
 	public function transactionRollback($savepoint = false) {
 		if ($savepoint) {
-			$this->query('ROLLBACK TO ' . $savepoint . ';');
+			$this->query('ROLLBACK TO ' . $savepoint);
 		} else {
 			$this->query('ROLLBACK');
 		}
 	}
 
 	public function transactionEnd($chain = false) {
-		$this->query('COMMIT AND ' . ($chain ? '' : 'NO ') . 'CHAIN;');
+		$this->query('COMMIT AND ' . ($chain ? '' : 'NO ') . 'CHAIN');
 	}
 
 	public function comparisonClause($field, $value, $exact = false, $negate = false, $caseSensitive = null,

model/connect/PDOConnector.php

@@ -6,54 +6,54 @@
  * @subpackage model
  */
 class PDOConnector extends DBConnector {
-	
+
 	/**
 	 * Should ATTR_EMULATE_PREPARES flag be used to emulate prepared statements?
-	 * 
+	 *
 	 * @config
 	 * @var boolean
 	 */
 	private static $emulate_prepare = false;
-	
+
 	/**
 	 * The PDO connection instance
-	 * 
-	 * @var PDO 
+	 *
+	 * @var PDO
 	 */
 	protected $pdoConnection = null;
 
 	/**
 	 * Name of the currently selected database
-	 * 
+	 *
 	 * @var string
 	 */
 	protected $databaseName = null;
 
 	/**
 	 * The most recent statement returned from PDODatabase->query
-	 * 
+	 *
 	 * @var PDOStatement
 	 */
 	protected $lastStatement = null;
-	
+
 	/**
 	 * List of prepared statements, cached by SQL string
 	 *
 	 * @var array
 	 */
 	protected $cachedStatements = array();
-	
+
 	/**
 	 * Flush all prepared statements
 	 */
 	public function flushStatements() {
 		$this->cachedStatements = array();
 	}
-	
+
 	/**
 	 * Retrieve a prepared statement for a given SQL string, or return an already prepared version if
 	 * one exists for the given query
-	 * 
+	 *
 	 * @param string $sql
 	 * @return PDOStatement
 	 */
@@ -66,10 +66,10 @@ class PDOConnector extends DBConnector {
 		}
 		return $this->cachedStatements[$sql];
 	}
-	
+
 	/**
 	 * Is PDO running in emulated mode
-	 * 
+	 *
 	 * @return boolean
 	 */
 	public static function is_emulate_prepare() {
@@ -84,7 +84,7 @@ class PDOConnector extends DBConnector {
 		// requested via selectDatabase
 		$driver = $parameters['driver'] . ":";
 		$dsn = array();
-		
+
 		// Typically this is false, but some drivers will request this
 		if($selectDB) {
 			// Specify complete file path immediately following driver (SQLLite3)
@@ -92,14 +92,14 @@ class PDOConnector extends DBConnector {
 				$dsn[] = $parameters['filepath'];
 			} elseif(!empty($parameters['database'])) {
 				// Some databases require a selected database at connection (SQLite3, Azure)
-				if($parameters['driver'] === 'sqlsrv') { 
+				if($parameters['driver'] === 'sqlsrv') {
 					$dsn[] = "Database={$parameters['database']}";
 				} else {
 					$dsn[] = "dbname={$parameters['database']}";
 				}
 			}
 		}
-		
+
 		// Syntax for sql server is slightly different
 		if($parameters['driver'] === 'sqlsrv') {
 			$server = $parameters['server'];
@@ -128,18 +128,20 @@ class PDOConnector extends DBConnector {
 
 		// Connection commands to be run on every re-connection
 		$options = array(
-			PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8',
-			PDO::ATTR_EMULATE_PREPARES => self::is_emulate_prepare()
+			PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8'
 		);
+		if(self::is_emulate_prepare()) {
+			$options[PDO::ATTR_EMULATE_PREPARES] = true;
+		}
 
 		// May throw a PDOException if fails
-		if(empty($parameters['username']) || empty($parameters['password'])) {
-			$this->pdoConnection = new PDO($driver.implode(';', $dsn));
-		} else {
-			$this->pdoConnection = new PDO($driver.implode(';', $dsn), $parameters['username'],
-											$parameters['password'], $options);
-		}
-		
+		$this->pdoConnection = new PDO(
+			$driver.implode(';', $dsn),
+			empty($parameters['username']) ? '' : $parameters['username'],
+			empty($parameters['password']) ? '' : $parameters['password'],
+			$options
+		);
+
 		// Show selected DB if requested
 		if($this->pdoConnection && $selectDB && !empty($parameters['database'])) {
 			$this->databaseName = $parameters['database'];
@@ -164,10 +166,10 @@ class PDOConnector extends DBConnector {
 	public function quoteString($value) {
 		return $this->pdoConnection->quote($value);
 	}
-	
+
 	/**
 	 * Executes a query that doesn't return a resultset
-	 * 
+	 *
 	 * @param string $sql
 	 * @param string $sql The SQL query to execute
 	 * @param integer $errorLevel For errors to this query, raise PHP errors
@@ -176,7 +178,7 @@ class PDOConnector extends DBConnector {
 	public function exec($sql, $errorLevel = E_USER_ERROR) {
 		// Check if we should only preview this query
 		if ($this->previewWrite($sql)) return;
-		
+
 		// Reset last statement to prevent interference in case of error
 		$this->lastStatement = null;
 
@@ -213,7 +215,7 @@ class PDOConnector extends DBConnector {
 
 		return new PDOQuery($this->lastStatement);
 	}
-	
+
 	/**
 	 * Determines the PDO::PARAM_* type for a given PHP type string
 	 * @param string $phpType Type of object in PHP
@@ -241,10 +243,10 @@ class PDOConnector extends DBConnector {
 				user_error("Cannot bind parameter as it is an unsupported type ($phpType)", E_USER_ERROR);
 		}
 	}
-	
+
 	/**
 	 * Bind all parameters to a PDOStatement
-	 * 
+	 *
 	 * @param PDOStatement $statement
 	 * @param array $parameters
 	 */
@@ -259,7 +261,7 @@ class PDOConnector extends DBConnector {
 				$phpType = $value['type'];
 				$value = $value['value'];
 			}
-			
+
 			// Check type of parameter
 			$type = $this->getPDOParamType($phpType);
 			if($type === PDO::PARAM_STR) $value = strval($value);
@@ -268,7 +270,7 @@ class PDOConnector extends DBConnector {
 			$statement->bindValue($index+1, $value, $type);
 		}
 	}
-	
+
 	public function preparedQuery($sql, $parameters, $errorLevel = E_USER_ERROR) {
 		// Check if we should only preview this query
 		if ($this->previewWrite($sql)) return;
@@ -276,14 +278,14 @@ class PDOConnector extends DBConnector {
 		// Benchmark query
 		$self = $this;
 		$this->lastStatement = $this->benchmarkQuery($sql, function($sql) use($parameters, $self) {
-			
+
 			// Prepare statement
 			$statement = $self->getOrPrepareStatement($sql);
 			if(!$statement) return null;
-			
+
 			// Inject parameters
 			$self->bindParameters($statement, $parameters);
-			
+
 			// Safely execute the statement
 			$statement->execute($parameters);
 			return $statement;
@@ -298,17 +300,17 @@ class PDOConnector extends DBConnector {
 
 		return new PDOQuery($this->lastStatement);
 	}
-	
+
 	/**
 	 * Determine if a resource has an attached error
-	 * 
+	 *
 	 * @param PDOStatement|PDO $resource the resource to check
 	 * @return boolean Flag indicating true if the resource has an error
 	 */
 	protected function hasError($resource) {
 		// No error if no resource
 		if(empty($resource)) return false;
-		
+
 		// If the error code is empty the statement / connection has not been run yet
 		$code = $resource->errorCode();
 		if(empty($code)) return false;

model/connect/PDOQuery.php

@@ -27,10 +27,6 @@ class PDOQuery extends SS_Query {
 		$statement->closeCursor();
 	}
 
-	public function __destruct() {
-		$this->statement->closeCursor();
-	}
-
 	public function seek($row) {
 		$this->rowNum = $row - 1;
 		return $this->nextRecord();