diff --git a/src/ORM/Connect/MySQLQuery.php b/src/ORM/Connect/MySQLQuery.php index a8647363e..22f11a3eb 100644 --- a/src/ORM/Connect/MySQLQuery.php +++ b/src/ORM/Connect/MySQLQuery.php @@ -4,6 +4,7 @@ namespace SilverStripe\ORM\Connect; /** * A result-set from a MySQL database (using MySQLiConnector) + * Note that this class is only used for the results of non-prepared statements */ class MySQLQuery extends Query { @@ -17,6 +18,11 @@ class MySQLQuery extends Query */ protected $handle; + /** + * Metadata about the columns of this query + */ + protected $columns; + /** * Hook the result-set given into a Query class, suitable for use by SilverStripe. * @@ -27,6 +33,9 @@ class MySQLQuery extends Query public function __construct($database, $handle) { $this->handle = $handle; + if (is_object($this->handle)) { + $this->columns = $this->handle->fetch_fields(); + } } public function __destruct() @@ -40,7 +49,7 @@ class MySQLQuery extends Query { if (is_object($this->handle)) { $this->handle->data_seek($row); - return $this->handle->fetch_assoc(); + return $this->nextRecord(); } return null; } @@ -55,7 +64,19 @@ class MySQLQuery extends Query public function nextRecord() { - if (is_object($this->handle) && ($data = $this->handle->fetch_assoc())) { + $floatTypes = [MYSQLI_TYPE_FLOAT, MYSQLI_TYPE_DOUBLE, MYSQLI_TYPE_DECIMAL, MYSQLI_TYPE_NEWDECIMAL]; + + if (is_object($this->handle) && ($row = $this->handle->fetch_array(MYSQLI_NUM))) { + $data = []; + foreach ($row as $i => $value) { + if (!isset($this->columns[$i])) { + throw new DatabaseException("Can't get metadata for column $i"); + } + if (in_array($this->columns[$i]->type, $floatTypes)) { + $value = (float)$value; + } + $data[$this->columns[$i]->name] = $value; + } return $data; } else { return false; diff --git a/src/ORM/Connect/MySQLStatement.php b/src/ORM/Connect/MySQLStatement.php index 8ba6b8de5..9c7fdce26 100644 --- a/src/ORM/Connect/MySQLStatement.php +++ b/src/ORM/Connect/MySQLStatement.php @@ -6,7 +6,7 @@ use mysqli_result; use mysqli_stmt; /** - * Provides a record-view for mysqli statements + * Provides a record-view for mysqli prepared statements * * By default streams unbuffered data, but seek(), rewind(), or numRecords() will force the statement to * buffer itself and sacrifice any potential performance benefit. @@ -42,6 +42,13 @@ class MySQLStatement extends Query */ protected $columns = array(); + /** + * Map of column types, keyed by column name + * + * @var array + */ + protected $types = array(); + /** * List of bound variables in the current row * @@ -59,6 +66,7 @@ class MySQLStatement extends Query // Bind each field while ($field = $this->metadata->fetch_field()) { $this->columns[] = $field->name; + $this->types[$field->name] = $field->type; // Note that while boundValues isn't initialised at this point, // later calls to $this->statement->fetch() Will populate // $this->boundValues later with the next result. @@ -116,6 +124,10 @@ class MySQLStatement extends Query // Dereferenced row $row = array(); foreach ($this->boundValues as $key => $value) { + $floatTypes = [MYSQLI_TYPE_FLOAT, MYSQLI_TYPE_DOUBLE, MYSQLI_TYPE_DECIMAL, MYSQLI_TYPE_NEWDECIMAL]; + if (in_array($this->types[$key], $floatTypes)) { + $value = (float)$value; + } $row[$key] = $value; } return $row; diff --git a/src/ORM/Connect/PDOConnector.php b/src/ORM/Connect/PDOConnector.php index 19d6d3b0b..c26cc02bb 100644 --- a/src/ORM/Connect/PDOConnector.php +++ b/src/ORM/Connect/PDOConnector.php @@ -98,7 +98,7 @@ class PDOConnector extends DBConnector implements TransactionManager * one exists for the given query * * @param string $sql - * @return PDOStatement + * @return PDOStatementHandle|false */ public function getOrPrepareStatement($sql) { @@ -113,11 +113,14 @@ class PDOConnector extends DBConnector implements TransactionManager array(PDO::ATTR_CURSOR => PDO::CURSOR_FWDONLY) ); + // Wrap in a PDOStatementHandle, to cache column metadata + $statementHandle = ($statement === false) ? false : new PDOStatementHandle($statement); + // Only cache select statements if (preg_match('/^(\s*)select\b/i', $sql)) { - $this->cachedStatements[$sql] = $statement; + $this->cachedStatements[$sql] = $statementHandle; } - return $statement; + return $statementHandle; } /** @@ -214,7 +217,10 @@ class PDOConnector extends DBConnector implements TransactionManager $options[PDO::MYSQL_ATTR_SSL_CA] = $parameters['ssl_ca']; } // use default cipher if not provided - $options[PDO::MYSQL_ATTR_SSL_CIPHER] = array_key_exists('ssl_cipher', $parameters) ? $parameters['ssl_cipher'] : self::config()->get('ssl_cipher_default'); + $options[PDO::MYSQL_ATTR_SSL_CIPHER] = + array_key_exists('ssl_cipher', $parameters) ? + $parameters['ssl_cipher'] : + self::config()->get('ssl_cipher_default'); } if (static::config()->get('legacy_types')) { @@ -328,7 +334,11 @@ class PDOConnector extends DBConnector implements TransactionManager $statement = $this->pdoConnection->query($sql); // Generate results - return $this->prepareResults($statement, $errorLevel, $sql); + if ($statement === false) { + $this->databaseError($this->getLastError(), $errorLevel, $sql); + } else { + return $this->prepareResults(new PDOStatementHandle($statement), $errorLevel, $sql); + } } /** @@ -395,52 +405,49 @@ class PDOConnector extends DBConnector implements TransactionManager { $this->beforeQuery($sql); - // Prepare statement - $statement = $this->getOrPrepareStatement($sql); + // Fetch cached statement, or create it + $statementHandle = $this->getOrPrepareStatement($sql); - // Bind and invoke statement safely - if ($statement) { - $this->bindParameters($statement, $parameters); - $statement->execute($parameters); + // Error handling + if ($statementHandle === false) { + $this->databaseError($this->getLastError(), $errorLevel, $sql, $this->parameterValues($parameters)); + return null; } + // Bind parameters + $this->bindParameters($statementHandle->getPDOStatement(), $parameters); + $statementHandle->execute($parameters); + // Generate results - return $this->prepareResults($statement, $errorLevel, $sql); + return $this->prepareResults($statementHandle, $errorLevel, $sql); } /** * Given a PDOStatement that has just been executed, generate results * and report any errors * - * @param PDOStatement $statement + * @param PDOStatementHandle $statement * @param int $errorLevel * @param string $sql * @param array $parameters * @return PDOQuery */ - protected function prepareResults($statement, $errorLevel, $sql, $parameters = array()) + protected function prepareResults(PDOStatementHandle $statement, $errorLevel, $sql, $parameters = array()) { - // Record row-count and errors of last statement + // Catch error if ($this->hasError($statement)) { $this->lastStatementError = $statement->errorInfo(); - } elseif ($statement) { - // Count and return results - $this->rowCount = $statement->rowCount(); - return new PDOQuery($statement, $this); - } - - // Ensure statement is closed - if ($statement) { $statement->closeCursor(); + + $this->databaseError($this->getLastError(), $errorLevel, $sql, $this->parameterValues($parameters)); + + return null; } - // Report any errors - if ($parameters) { - $parameters = $this->parameterValues($parameters); - } - $this->databaseError($this->getLastError(), $errorLevel, $sql, $parameters); - return null; + // Count and return results + $this->rowCount = $statement->rowCount(); + return new PDOQuery($statement); } /** diff --git a/src/ORM/Connect/PDOQuery.php b/src/ORM/Connect/PDOQuery.php index 826432219..718fd6f8b 100644 --- a/src/ORM/Connect/PDOQuery.php +++ b/src/ORM/Connect/PDOQuery.php @@ -2,74 +2,30 @@ namespace SilverStripe\ORM\Connect; -use PDOStatement; -use PDO; - /** * A result-set from a PDO database. */ class PDOQuery extends Query { /** - * The internal MySQL handle that points to the result set. - * @var PDOStatement + * @var array */ - protected $statement = null; - protected $results = null; /** * Hook the result-set given into a Query class, suitable for use by SilverStripe. * @param PDOStatement $statement The internal PDOStatement containing the results */ - public function __construct(PDOStatement $statement, PDOConnector $conn) + public function __construct(PDOStatementHandle $statement) { - $this->statement = $statement; // Since no more than one PDOStatement for any one connection can be safely // traversed, each statement simply requests all rows at once for safety. // This could be re-engineered to call fetchAll on an as-needed basis - // Special case for Postgres - if ($conn->getDriver() == 'pgsql') { - $this->results = $this->fetchAllPgsql($statement); - } else { - $this->results = $statement->fetchAll(PDO::FETCH_ASSOC); - } + $this->results = $statement->typeCorrectedFetchAll(); $statement->closeCursor(); } - /** - * Fetch a record form the statement with its type data corrected - * Necessary to fix float data retrieved from PGSQL - * Returns data as an array of maps - * @return array - */ - protected function fetchAllPgsql($statement) - { - $columnCount = $statement->columnCount(); - $columnMeta = []; - for ($i = 0; $i<$columnCount; $i++) { - $columnMeta[$i] = $statement->getColumnMeta($i); - } - - // Re-map fetched data using columnMeta - return array_map( - function ($rowArray) use ($columnMeta) { - $row = []; - foreach ($columnMeta as $i => $meta) { - // Coerce floats from string to float - // PDO PostgreSQL fails to do this - if (isset($meta['native_type']) && strpos($meta['native_type'], 'float') === 0) { - $rowArray[$i] = (float)$rowArray[$i]; - } - $row[$meta['name']] = $rowArray[$i]; - } - return $row; - }, - $statement->fetchAll(PDO::FETCH_NUM) - ); - } - public function seek($row) { $this->rowNum = $row - 1; diff --git a/src/ORM/Connect/PDOStatementHandle.php b/src/ORM/Connect/PDOStatementHandle.php new file mode 100644 index 000000000..259e71091 --- /dev/null +++ b/src/ORM/Connect/PDOStatementHandle.php @@ -0,0 +1,154 @@ +statement = $statement; + } + + /** + * Mapping of PDO-reported "native types" to PHP types + */ + protected static $type_mapping = [ + // PGSQL + 'float8' => 'float', + 'float16' => 'float', + 'numeric' => 'float', + + // MySQL + 'NEWDECIMAL' => 'float', + + // SQlite + 'integer' => 'int', + 'double' => 'float', + ]; + + /** + * Fetch a record form the statement with its type data corrected + * Returns data as an array of maps + * @return array + */ + public function typeCorrectedFetchAll() + { + if ($this->columnMeta === null) { + $columnCount = $this->statement->columnCount(); + $this->columnMeta = []; + for ($i = 0; $i<$columnCount; $i++) { + $this->columnMeta[$i] = $this->statement->getColumnMeta($i); + } + } + + // Re-map fetched data using columnMeta + return array_map( + function ($rowArray) { + $row = []; + foreach ($this->columnMeta as $i => $meta) { + // Coerce any column types that aren't correctly retrieved from the database + if (isset($meta['native_type']) && isset(self::$type_mapping[$meta['native_type']])) { + settype($rowArray[$i], self::$type_mapping[$meta['native_type']]); + } + $row[$meta['name']] = $rowArray[$i]; + } + return $row; + }, + $this->statement->fetchAll(PDO::FETCH_NUM) + ); + } + + /** + * Closes the cursor, enabling the statement to be executed again (PDOStatement::closeCursor) + * + * @return bool Returns true on success + */ + public function closeCursor() + { + return $this->statement->closeCursor(); + } + + /** + * Fetch the SQLSTATE associated with the last operation on the statement handle + * (PDOStatement::errorCode) + * + * @return string + */ + public function errorCode() + { + return $this->statement->errorCode(); + } + + /** + * Fetch extended error information associated with the last operation on the statement handle + * (PDOStatement::errorInfo) + * + * @return array + */ + public function errorInfo() + { + return $this->statement->errorInfo(); + } + + /** + * Returns the number of rows affected by the last SQL statement (PDOStatement::rowCount) + * + * @return int + */ + public function rowCount() + { + return $this->statement->rowCount(); + } + + /** + * Executes a prepared statement (PDOStatement::execute) + * + * @param $parameters An array of values with as many elements as there are bound parameters in the SQL statement + * being executed + * @return bool Returns true on success + */ + public function execute(array $parameters) + { + return $this->statement->execute($parameters); + } + + /** + * Return the PDOStatement that this object provides a handle to + * + * @return PDOStatement + */ + public function getPDOStatement() + { + return $this->statement; + } +} diff --git a/tests/php/ORM/DataListTest.php b/tests/php/ORM/DataListTest.php index baecfeaf7..a00290c3f 100755 --- a/tests/php/ORM/DataListTest.php +++ b/tests/php/ORM/DataListTest.php @@ -11,6 +11,7 @@ use SilverStripe\ORM\DB; use SilverStripe\ORM\Filterable; use SilverStripe\ORM\Filters\ExactMatchFilter; use SilverStripe\Dev\SapphireTest; +use SilverStripe\ORM\Tests\DataObjectTest\Fixture; use SilverStripe\ORM\Tests\DataObjectTest\Bracket; use SilverStripe\ORM\Tests\DataObjectTest\EquipmentCompany; use SilverStripe\ORM\Tests\DataObjectTest\Fan; @@ -940,6 +941,27 @@ class DataListTest extends SapphireTest $this->assertCount(4, $list); } + public function testFilterAnyWithTwoGreaterThanFilters() + { + + for ($i=1; $i<=3; $i++) { + $f = new Fixture(); + $f->MyDecimal = $i; + $f->write(); + + $f = new Fixture(); + $f->MyInt = $i; + $f->write(); + } + + $list = Fixture::get()->filterAny([ + 'MyDecimal:GreaterThan' => 1, // 2 records + 'MyInt:GreaterThan' => 2, // 1 record + ]); + + $this->assertCount(3, $list); + } + public function testFilterAnyMultipleArray() { $list = TeamComment::get(); diff --git a/tests/php/ORM/DataObjectTest.php b/tests/php/ORM/DataObjectTest.php index 5ae6ac037..88b081c0d 100644 --- a/tests/php/ORM/DataObjectTest.php +++ b/tests/php/ORM/DataObjectTest.php @@ -1932,10 +1932,36 @@ class DataObjectTest extends SapphireTest { $obj = new DataObjectTest\Fixture(); $obj->write(); - + $this->assertInternalType("int", $obj->ID); } + /** + * Tests that zero values are returned with the correct types + */ + public function testZeroIsFalse() + { + $obj = new DataObjectTest\Fixture(); + $obj->MyInt = 0; + $obj->MyDecimal = 0.00; + $obj->MyCurrency = 0.00; + $obj->write(); + + $this->assertEquals(0, $obj->MyInt, 'DBInt fields should be integer on first assignment'); + $this->assertEquals(0.00, $obj->MyDecimal, 'DBDecimal fields should be float on first assignment'); + $this->assertEquals(0.00, $obj->MyCurrency, 'DBCurrency fields should be float on first assignment'); + + $obj2 = DataObjectTest\Fixture::get()->byId($obj->ID); + + $this->assertEquals(0, $obj2->MyInt, 'DBInt fields should be integer'); + $this->assertEquals(0.00, $obj2->MyDecimal, 'DBDecimal fields should be float'); + $this->assertEquals(0.00, $obj2->MyCurrency, 'DBCurrency fields should be float'); + + $this->assertFalse((bool)$obj2->MyInt, 'DBInt zero fields should be falsey on fetch from DB'); + $this->assertFalse((bool)$obj2->MyDecimal, 'DBDecimal zero fields should be falsey on fetch from DB'); + $this->assertFalse((bool)$obj2->MyCurrency, 'DBCurrency zero fields should be falsey on fetch from DB'); + } + public function testTwoSubclassesWithTheSameFieldNameWork() { // Create two objects of different subclasses, setting the values of fields that are diff --git a/tests/php/ORM/DataObjectTest/Fixture.php b/tests/php/ORM/DataObjectTest/Fixture.php index 11e407384..4c02ef20f 100644 --- a/tests/php/ORM/DataObjectTest/Fixture.php +++ b/tests/php/ORM/DataObjectTest/Fixture.php @@ -20,7 +20,11 @@ class Fixture extends DataObject implements TestOnly 'DatetimeField' => 'Datetime', 'MyFieldWithDefault' => 'Varchar', - 'MyFieldWithAltDefault' => 'Varchar' + 'MyFieldWithAltDefault' => 'Varchar', + + 'MyInt' => 'Int', + 'MyCurrency' => 'Currency', + 'MyDecimal'=> 'Decimal', ); private static $defaults = array( diff --git a/tests/php/ORM/DatabaseTest.php b/tests/php/ORM/DatabaseTest.php index 0fd108120..a13ff7d97 100644 --- a/tests/php/ORM/DatabaseTest.php +++ b/tests/php/ORM/DatabaseTest.php @@ -194,6 +194,9 @@ class DatabaseTest extends SapphireTest $obj->MyField = "value"; $obj->MyInt = 5; $obj->MyFloat = 6.0; + + // Note: in non-PDO SQLite, whole numbers of a decimal field will be returned as integers rather than floats + $obj->MyDecimal = 7.1; $obj->MyBoolean = true; $obj->write(); @@ -203,20 +206,66 @@ class DatabaseTest extends SapphireTest )->record(); // IDs and ints are returned as ints - $this->assertInternalType('int', $record['ID']); - $this->assertInternalType('int', $record['MyInt']); + $this->assertInternalType('int', $record['ID'], 'Primary key should be integer'); + $this->assertInternalType('int', $record['MyInt'], 'DBInt fields should be integer'); - $this->assertInternalType('float', $record['MyFloat']); + $this->assertInternalType('float', $record['MyFloat'], 'DBFloat fields should be float'); + $this->assertInternalType('float', $record['MyDecimal'], 'DBDecimal fields should be float'); // Booleans are returned as ints – we follow MySQL's lead - $this->assertInternalType('int', $record['MyBoolean']); + $this->assertInternalType('int', $record['MyBoolean'], 'DBBoolean fields should be int'); // Strings and enums are returned as strings - $this->assertInternalType('string', $record['MyField']); - $this->assertInternalType('string', $record['ClassName']); + $this->assertInternalType('string', $record['MyField'], 'DBVarchar fields should be string'); + $this->assertInternalType('string', $record['ClassName'], 'DBEnum fields should be string'); // Dates are returned as strings - $this->assertInternalType('string', $record['Created']); - $this->assertInternalType('string', $record['LastEdited']); + $this->assertInternalType('string', $record['Created'], 'DBDatetime fields should be string'); + + + // Ensure that the same is true when calling a query a second time (cached prepared statement) + + $record = DB::prepared_query( + 'SELECT * FROM "DatabaseTest_MyObject" WHERE "ID" = ?', + [ $obj->ID ] + )->record(); + + // IDs and ints are returned as ints + $this->assertInternalType('int', $record['ID'], 'Primary key should be integer (2nd call)'); + $this->assertInternalType('int', $record['MyInt'], 'DBInt fields should be integer (2nd call)'); + + $this->assertInternalType('float', $record['MyFloat'], 'DBFloat fields should be float (2nd call)'); + $this->assertInternalType('float', $record['MyDecimal'], 'DBDecimal fields should be float (2nd call)'); + + // Booleans are returned as ints – we follow MySQL's lead + $this->assertInternalType('int', $record['MyBoolean'], 'DBBoolean fields should be int (2nd call)'); + + // Strings and enums are returned as strings + $this->assertInternalType('string', $record['MyField'], 'DBVarchar fields should be string (2nd call)'); + $this->assertInternalType('string', $record['ClassName'], 'DBEnum fields should be string (2nd call)'); + + // Dates are returned as strings + $this->assertInternalType('string', $record['Created'], 'DBDatetime fields should be string (2nd call)'); + + + // Ensure that the same is true when using non-prepared statements + $record = DB::query('SELECT * FROM "DatabaseTest_MyObject" WHERE "ID" = ' . (int)$obj->ID)->record(); + + // IDs and ints are returned as ints + $this->assertInternalType('int', $record['ID'], 'Primary key should be integer (non-prepared)'); + $this->assertInternalType('int', $record['MyInt'], 'DBInt fields should be integer (non-prepared)'); + + $this->assertInternalType('float', $record['MyFloat'], 'DBFloat fields should be float (non-prepared)'); + $this->assertInternalType('float', $record['MyDecimal'], 'DBDecimal fields should be float (non-prepared)'); + + // Booleans are returned as ints – we follow MySQL's lead + $this->assertInternalType('int', $record['MyBoolean'], 'DBBoolean fields should be int (non-prepared)'); + + // Strings and enums are returned as strings + $this->assertInternalType('string', $record['MyField'], 'DBVarchar fields should be string (non-prepared)'); + $this->assertInternalType('string', $record['ClassName'], 'DBEnum fields should be string (non-prepared)'); + + // Dates are returned as strings + $this->assertInternalType('string', $record['Created'], 'DBDatetime fields should be string (non-prepared)'); } } diff --git a/tests/php/ORM/DatabaseTest/MyObject.php b/tests/php/ORM/DatabaseTest/MyObject.php index 3d09cb1cf..89f3dc0c8 100644 --- a/tests/php/ORM/DatabaseTest/MyObject.php +++ b/tests/php/ORM/DatabaseTest/MyObject.php @@ -16,6 +16,7 @@ class MyObject extends DataObject implements TestOnly 'MyField' => 'Varchar', 'MyInt' => 'Int', 'MyFloat' => 'Float', + 'MyDecimal' => 'Decimal', 'MyBoolean' => 'Boolean', ); }