Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 64-bit IEEE 754 floating-point precision support #991

Merged
merged 15 commits into from
May 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion .github/workflows/test-unit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ jobs:
if [ -n "$LOG_COVERAGE" ]; then mv coverage/phpunit.cov coverage/phpunit-sqlite.cov; fi

- name: "Run tests: MySQL - PDO"
if: success() || failure()
env:
DB_DSN: "pdo_mysql:host=mysql;dbname=atk4_test"
DB_USER: atk4_test_user
Expand All @@ -170,6 +171,7 @@ jobs:
if [ -n "$LOG_COVERAGE" ]; then mv coverage/phpunit.cov coverage/phpunit-mysql-pdo.cov; fi

- name: "Run tests: MySQL - mysqli"
if: success() || failure()
env:
DB_DSN: "mysqli:host=mysql;dbname=atk4_test"
DB_USER: atk4_test_user
Expand All @@ -179,6 +181,7 @@ jobs:
if [ -n "$LOG_COVERAGE" ]; then mv coverage/phpunit.cov coverage/phpunit-mysql-mysqli.cov; fi

- name: "Run tests: MySQL 5.6"
if: success() || failure()
env:
DB_DSN: "mysql:host=mysql56;dbname=atk4_test"
DB_USER: atk4_test_user
Expand All @@ -188,6 +191,7 @@ jobs:
if [ -n "$LOG_COVERAGE" ]; then mv coverage/phpunit.cov coverage/phpunit-mysql56.cov; fi

- name: "Run tests: MariaDB"
if: success() || failure()
env:
DB_DSN: "mysql:host=mariadb;dbname=atk4_test"
DB_USER: atk4_test_user
Expand All @@ -197,6 +201,7 @@ jobs:
if [ -n "$LOG_COVERAGE" ]; then mv coverage/phpunit.cov coverage/phpunit-mariadb.cov; fi

- name: "Run tests: PostgreSQL"
if: success() || failure()
env:
DB_DSN: "pgsql:host=postgres;dbname=atk4_test"
DB_USER: atk4_test_user
Expand All @@ -206,6 +211,7 @@ jobs:
if [ -n "$LOG_COVERAGE" ]; then mv coverage/phpunit.cov coverage/phpunit-postgres.cov; fi

- name: "Run tests: MSSQL"
if: success() || failure()
env:
DB_DSN: "sqlsrv:host=mssql;dbname=master;driverOptions[TrustServerCertificate]=1"
DB_USER: sa
Expand All @@ -215,7 +221,7 @@ jobs:
if [ -n "$LOG_COVERAGE" ]; then mv coverage/phpunit.cov coverage/phpunit-mssql.cov; fi

- name: "Run tests: Oracle - PDO (only for coverage or cron)"
if: env.LOG_COVERAGE || github.event_name == 'schedule'
if: (success() || failure()) && env.LOG_COVERAGE || github.event_name == 'schedule'
env:
DB_DSN: "pdo_oci:dbname=oracle/xe"
DB_USER: system
Expand All @@ -226,6 +232,7 @@ jobs:
if [ -n "$LOG_COVERAGE" ]; then mv coverage/phpunit.cov coverage/phpunit-oracle-pdo.cov; fi

- name: "Run tests: Oracle - OCI8"
if: success() || failure()
env:
DB_DSN: "oci8:dbname=oracle/xe"
DB_USER: system
Expand Down
11 changes: 9 additions & 2 deletions src/Field.php
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,10 @@ public function normalize($value)
} else {
throw new Exception('Must not be boolean type');
}
} elseif (is_scalar($value)) {
} elseif (is_int($value)) {
$value = (string) $value;
} elseif (is_float($value)) {
$value = Expression::castFloatToString($value);
} else {
throw new Exception('Must be scalar');
}
Expand Down Expand Up @@ -364,7 +366,12 @@ private function getValueForCompare($value): ?string
return null;
}

return (string) $this->typecastSaveField($value, true);
$res = $this->typecastSaveField($value, true);
if (is_float($res)) {
return Expression::castFloatToString($res);
}

return (string) $res;
}

/**
Expand Down
6 changes: 4 additions & 2 deletions src/Model/Scope/Condition.php
Original file line number Diff line number Diff line change
Expand Up @@ -395,8 +395,10 @@ protected function valueToWords(Model $model, $value): string

if (is_bool($value)) {
$valueStr = $value ? 'true' : 'false';
} elseif (is_int($value) || is_float($value)) {
$valueStr = $value;
} elseif (is_int($value)) {
$valueStr = (string) $value;
} elseif (is_float($value)) {
$valueStr = Expression::castFloatToString($value);
} else {
$valueStr = '\'' . (string) $value . '\'';
}
Expand Down
49 changes: 34 additions & 15 deletions src/Persistence/Sql/Expression.php
Original file line number Diff line number Diff line change
Expand Up @@ -385,15 +385,15 @@ function ($matches) use (&$nameless_count) {

$identifier = substr($matches[0], 1, -1);

$escaping = null;
$escapeMode = null;
if (substr($matches[0], 0, 1) === '[') {
$escaping = self::ESCAPE_PARAM;
$escapeMode = self::ESCAPE_PARAM;
} elseif (substr($matches[0], 0, 1) === '{') {
if (substr($matches[0], 1, 1) === '{') {
$escaping = self::ESCAPE_IDENTIFIER_SOFT;
$escapeMode = self::ESCAPE_IDENTIFIER_SOFT;
$identifier = substr($identifier, 1, -1);
} else {
$escaping = self::ESCAPE_IDENTIFIER;
$escapeMode = self::ESCAPE_IDENTIFIER;
}
}

Expand All @@ -406,7 +406,7 @@ function ($matches) use (&$nameless_count) {
$fx = '_render_' . $identifier;

if (array_key_exists($identifier, $this->args['custom'])) {
$value = $this->consume($this->args['custom'][$identifier], $escaping);
$value = $this->consume($this->args['custom'][$identifier], $escapeMode);
} elseif (method_exists($this, $fx)) {
$value = $this->{$fx}();
} else {
Expand Down Expand Up @@ -456,8 +456,10 @@ public function getDebugQuery(): string
$replacement = 'NULL';
} elseif (is_bool($val)) {
$replacement = $val ? '1' : '0';
} elseif (is_int($val) || is_float($val)) {
} elseif (is_int($val)) {
$replacement = (string) $val;
} elseif (is_float($val)) {
$replacement = self::castFloatToString($val);
} elseif (is_string($val)) {
$replacement = '\'' . addslashes($val) . '\'';
} else {
Expand Down Expand Up @@ -541,15 +543,15 @@ public function execute(object $connection = null): DbalResult
return $connection->execute($this);
}

[$query, $params] = $this->updateRenderBeforeExecute($this->render());
[$sql, $params] = $this->updateRenderBeforeExecute($this->render());

$platform = $this->connection->getDatabasePlatform();
try {
$statement = $connection->prepare($query);
$statement = $connection->prepare($sql);

foreach ($params as $key => $val) {
if (is_int($val)) {
$type = ParameterType::INTEGER;
if ($val === null) {
$type = ParameterType::NULL;
} elseif (is_bool($val)) {
if ($platform instanceof PostgreSQLPlatform) {
$type = ParameterType::STRING;
Expand All @@ -558,9 +560,10 @@ public function execute(object $connection = null): DbalResult
$type = ParameterType::INTEGER;
$val = $val ? 1 : 0;
}
} elseif ($val === null) {
$type = ParameterType::NULL;
} elseif (is_int($val)) {
$type = ParameterType::INTEGER;
} elseif (is_float($val)) {
$val = self::castFloatToString($val);
$type = ParameterType::STRING;
} elseif (is_string($val)) {
$type = ParameterType::STRING;
Expand Down Expand Up @@ -619,15 +622,31 @@ public function execute(object $connection = null): DbalResult

// {{{ Result Querying

/**
* Cast float to string with lossless precision.
*/
final public static function castFloatToString(float $value): string
{
$precisionBackup = ini_get('precision');
ini_set('precision', '-1');
try {
return (string) $value;
} finally {
ini_set('precision', $precisionBackup);
}
}

/**
* @param string|int|float|bool|null $v
*/
private function castGetValue($v): ?string
{
if (is_int($v) || is_float($v)) {
return (string) $v;
} elseif (is_bool($v)) {
if (is_bool($v)) {
return $v ? '1' : '0';
} elseif (is_int($v)) {
return (string) $v;
} elseif (is_float($v)) {
return self::castFloatToString($v);
}

// for PostgreSQL/Oracle CLOB/BLOB datatypes and PDO driver
Expand Down
25 changes: 18 additions & 7 deletions src/Persistence/Sql/Mssql/Query.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,24 @@ class Query extends BaseQuery

protected $expression_class = Expression::class;

protected $template_insert = 'begin try'
. "\n" . 'insert[option] into [table_noalias] ([set_fields]) values ([set_values])'
. "\n" . 'end try begin catch if ERROR_NUMBER() = 544 begin'
. "\n" . 'set IDENTITY_INSERT [table_noalias] on'
. "\n" . 'insert[option] into [table_noalias] ([set_fields]) values ([set_values])'
. "\n" . 'set IDENTITY_INSERT [table_noalias] off'
. "\n" . 'end end catch';
protected $template_insert = <<<'EOF'
begin try
insert[option] into [table_noalias] ([set_fields]) values ([set_values]);
end try begin catch
if ERROR_NUMBER() = 544 begin
set IDENTITY_INSERT [table_noalias] on;
begin try
insert[option] into [table_noalias] ([set_fields]) values ([set_values]);
set IDENTITY_INSERT [table_noalias] off;
end try begin catch
set IDENTITY_INSERT [table_noalias] off;
throw;
end catch
end else begin
throw;
end
end catch
EOF;

public function _render_limit(): ?string
{
Expand Down
4 changes: 2 additions & 2 deletions src/Persistence/Sql/Oracle/ExpressionTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

trait ExpressionTrait
{
protected function castLongStringToClobExpr(string $value): Expression
protected function convertLongStringToClobExpr(string $value): Expression
{
$exprArgs = [];
$buildConcatExprFx = function (array $parts) use (&$buildConcatExprFx, &$exprArgs): string {
Expand Down Expand Up @@ -50,7 +50,7 @@ protected function updateRenderBeforeExecute(array $render): array
function ($matches) use ($params, &$newParams, &$newParamBase) {
$value = $params[$matches[0]];
if (is_string($value) && strlen($value) > 4000) {
$expr = $this->castLongStringToClobExpr($value);
$expr = $this->convertLongStringToClobExpr($value);
unset($value);
[$exprSql, $exprParams] = $expr->render();
$sql = preg_replace_callback(
Expand Down
2 changes: 1 addition & 1 deletion src/Persistence/Sql/Query.php
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,7 @@ protected function _sub_render_condition(array $row): string
return '1 = 1'; // always true
}

$value = '(' . implode(', ', array_map(function ($v) { return $this->escapeParam($v); }, $value)) . ')';
$value = '(' . implode(', ', array_map(function ($v) { return $this->consume($v); }, $value)) . ')';

return $field . ' ' . $cond . ' ' . $value;
}
Expand Down
10 changes: 5 additions & 5 deletions src/Persistence/Static_.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,14 @@ public function __construct(array $data = null)
}

// try to detect type of field by its value
if (is_int($value)) {
$def_types[] = ['type' => 'integer'];
} elseif ($value instanceof \DateTime) {
$def_types[] = ['type' => 'datetime'];
} elseif (is_bool($value)) {
if (is_bool($value)) {
$def_types[] = ['type' => 'boolean'];
} elseif (is_int($value)) {
$def_types[] = ['type' => 'integer'];
} elseif (is_float($value)) {
$def_types[] = ['type' => 'float'];
} elseif ($value instanceof \DateTimeInterface) {
$def_types[] = ['type' => 'datetime'];
} elseif (is_array($value)) {
$def_types[] = ['type' => 'json'];
} elseif (is_object($value)) {
Expand Down
40 changes: 22 additions & 18 deletions src/Schema/TestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -249,18 +249,20 @@ public function setDb(array $dbData, bool $importData = true): void
}
}

$first_row = current($data);
$firstRow = current($data);
$idColumnName = null;
if ($first_row) {
$idColumnName = isset($first_row['_id']) ? '_id' : 'id';
if ($firstRow) {
$idColumnName = isset($firstRow['_id']) ? '_id' : 'id';
$migrator->id($idColumnName);

foreach ($first_row as $field => $row) {
foreach ($firstRow as $field => $row) {
if ($field === $idColumnName) {
continue;
}

if (is_int($row)) {
if (is_bool($row)) {
$fieldType = 'boolean';
} elseif (is_int($row)) {
$fieldType = 'integer';
} elseif (is_float($row)) {
$fieldType = 'float';
Expand All @@ -278,23 +280,25 @@ public function setDb(array $dbData, bool $importData = true): void

// import data
if ($importData) {
$hasId = (bool) key($data);
$this->db->atomic(function () use ($tableName, $data, $idColumnName) {
$hasId = array_key_first($data) !== 0;

foreach ($data as $id => $row) {
$query = $this->db->dsql();
if ($id === '_') {
continue;
}
foreach ($data as $id => $row) {
$query = $this->db->dsql();
if ($id === '_') {
continue;
}

$query->table($tableName);
$query->setMulti($row);
$query->table($tableName);
$query->setMulti($row);

if (!isset($row[$idColumnName]) && $hasId) {
$query->set($idColumnName, $id);
}
if (!isset($row[$idColumnName]) && $hasId) {
$query->set($idColumnName, $id);
}

$query->mode('insert')->execute();
}
$query->mode('insert')->execute();
}
});
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/ValidationException.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public function __construct(array $errors, Model $model = null, $intent = null)

if (count($errors) === 1) {
parent::__construct(reset($errors));
$this->addMoreInfo('field', key($errors));
$this->addMoreInfo('field', array_key_first($errors));
} else {
parent::__construct('Multiple unhandled validation errors');
$this->addMoreInfo('errors', $errors)
Expand Down
Loading