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

atk4 · May 7, 2022 · a647594 · a647594
1 parent 35a1c75
commit a647594
Show file tree

Hide file tree

Showing 13 changed files with 135 additions and 74 deletions.
diff --git a/.github/workflows/test-unit.yml b/.github/workflows/test-unit.yml
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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

diff --git a/src/Field.php b/src/Field.php
@@ -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');
                         }
@@ -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;
     }
 
     /**

diff --git a/src/Model/Scope/Condition.php b/src/Model/Scope/Condition.php
@@ -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 . '\'';
         }

diff --git a/src/Persistence/Sql/Expression.php b/src/Persistence/Sql/Expression.php
@@ -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;
                     }
                 }
 
@@ -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 {
@@ -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 {
@@ -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;
@@ -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;
@@ -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

diff --git a/src/Persistence/Sql/Mssql/Query.php b/src/Persistence/Sql/Mssql/Query.php
@@ -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
     {

diff --git a/src/Persistence/Sql/Oracle/ExpressionTrait.php b/src/Persistence/Sql/Oracle/ExpressionTrait.php
@@ -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 {
@@ -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(

diff --git a/src/Persistence/Sql/Query.php b/src/Persistence/Sql/Query.php
@@ -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;
         }

diff --git a/src/Persistence/Static_.php b/src/Persistence/Static_.php
@@ -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)) {

diff --git a/src/Schema/TestCase.php b/src/Schema/TestCase.php
@@ -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';
@@ -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();
+                    }
+                });
             }
         }
     }

diff --git a/src/ValidationException.php b/src/ValidationException.php
@@ -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)