tonyair/silverstripe-framework
1
0
Fork 0
mirror of https://github.com/silverstripe/silverstripe-framework synced 2024-06-28 23:49:26 +02:00
Code Issues Projects Releases Wiki Activity

Merge pull request #8590 from sminnee/zero-is-false

Browse Source 
Ensure that decimal DB values are fetched as floats
This commit is contained in:
Loz Calver 2019-04-05 09:41:41 +01:00 committed by GitHub
commit 0b5f5e3bcb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 341 additions and 89 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
src/ORM/Connect/MySQLQuery.php
View File

@ -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;

14
src/ORM/Connect/MySQLStatement.php
View File

@ -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;

65
src/ORM/Connect/PDOConnector.php
View File

@ -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);
}
/**

50
src/ORM/Connect/PDOQuery.php
View File

@ -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;

154
src/ORM/Connect/PDOStatementHandle.php Normal file
View File

@ -0,0 +1,154 @@
<?php
namespace SilverStripe\ORM\Connect;
use PDO;
use PDOStatement;
/**
* A handle to a PDOStatement, with cached column metadata, and type conversion
*
* Column metadata can't be fetched from a native PDOStatement after multiple calls in some DB backends,
* so we wrap in this handle object, which also takes care of tidying up content types to keep in line
* with the SilverStripe 4.4+ type expectations.
*/
class PDOStatementHandle
{
/**
* The statement to provide a handle to
*
* @var PDOStatement
*/
private $statement;
/**
* Cached column metadata
*
* @var array
*/
private $columnMeta = null;
/**
* Create a new handle.
*
* @param $statement The statement to provide a handle to
*/
public function __construct(PDOStatement $statement)
{
$this->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;
}
}

22
tests/php/ORM/DataListTest.php
View File

@ -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();

28
tests/php/ORM/DataObjectTest.php
View File

@ -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

6
tests/php/ORM/DataObjectTest/Fixture.php
View File

@ -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(

65
tests/php/ORM/DatabaseTest.php
View File

@ -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)');
}
}

1
tests/php/ORM/DatabaseTest/MyObject.php
View File

@ -16,6 +16,7 @@ class MyObject extends DataObject implements TestOnly
'MyField' => 'Varchar',
'MyInt' => 'Int',
'MyFloat' => 'Float',
'MyDecimal' => 'Decimal',
'MyBoolean' => 'Boolean',
);
}