Skip to content

Commit 518ec86

Browse files
abnegateabnegate
authored
Merge pull request #1036 from utopia-php/fix/float-precision
fix(adapters): bind and read every double exactly so find, count, cursors and deletes agree
2 parents 7154053 + 0043660 commit 518ec86

11 files changed

Lines changed: 593 additions & 98 deletions

File tree

‎src/Database/Adapter/SQL.php‎

Lines changed: 66 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -124,12 +124,9 @@ abstract class SQL extends Adapter implements Feature\Connection, Feature\RawQue
124124

125125
private const int UNPAIRED_JOINED_ROWS = 4;
126126

127-
protected object $pdo;
127+
private const string ROUND_TRIP_PRECISION = '-1';
128128

129-
/**
130-
* Controls how many fractional digits are used when binding float parameters.
131-
*/
132-
protected int $floatPrecision = 17;
129+
protected object $pdo;
133130

134131
/**
135132
* Lazily constructed AttributeMap shared by every builder's scope.
@@ -209,11 +206,57 @@ public function getDriver(): object
209206
}
210207

211208
/**
212-
* Helper to format a float value according to configured precision for binding/logging.
209+
* A decimal that reads back as exactly this double, in as few digits as PHP writes it when they suffice. Casts
210+
* and `%H` ignore the locale, and a double needs at most 17 significant digits to round-trip.
211+
*/
212+
protected function formatFloat(float $value): string
213+
{
214+
$formatted = (string) $value;
215+
if ((float) $formatted === $value) {
216+
return $formatted;
217+
}
218+
219+
$formatted = \sprintf('%.16H', $value);
220+
221+
return (float) $formatted === $value ? $formatted : \sprintf('%.17H', $value);
222+
}
223+
224+
/**
225+
* Binds a value as its PDO type, and a double as its exact decimal: PDO would write it at the `precision` ini.
226+
*
227+
* @param PDOStatement|DatabasePDOStatement|PDOStatementProxy $statement
228+
*/
229+
protected function bind(object $statement, int|string $parameter, mixed $value): void
230+
{
231+
if (\is_float($value)) {
232+
$statement->bindValue($parameter, $this->formatFloat($value), PDO::PARAM_STR);
233+
234+
return;
235+
}
236+
237+
$statement->bindValue($parameter, $value, $this->getPdoType($value));
238+
}
239+
240+
/**
241+
* Runs a fetch at the precision that round-trips every double. PDO stringifies a double it fetches at the
242+
* `precision` ini, 14 significant digits by default.
243+
*
244+
* @template T
245+
*
246+
* @param callable(): T $fetch
247+
* @return T
213248
*/
214-
protected function getFloatPrecision(float $value): string
249+
protected function fetchExactly(callable $fetch): mixed
215250
{
216-
return sprintf('%.'.$this->floatPrecision.'F', $value);
251+
$precision = \ini_set('precision', self::ROUND_TRIP_PRECISION);
252+
253+
try {
254+
return $fetch();
255+
} finally {
256+
if ($precision !== false) {
257+
\ini_set('precision', $precision);
258+
}
259+
}
217260
}
218261

219262
#[\Override]
@@ -621,12 +664,12 @@ public function getDocument(Document $collection, string $id, array $queries = [
621664
try {
622665
$statement = $this->prepareStatement($sql, Event::DocumentRead);
623666
foreach ($bindings as $parameter => $value) {
624-
$statement->bindValue($parameter, $value, $this->getPdoType($value));
667+
$this->bind($statement, $parameter, $value);
625668
}
626669
$this->describeStatement($statement, $bindings, $name);
627670
$this->execute($statement);
628671
/** @var array<string, mixed>|false $row */
629-
$row = $statement->fetch(PDO::FETCH_ASSOC);
672+
$row = $this->fetchExactly(static fn (): mixed => $statement->fetch(PDO::FETCH_ASSOC));
630673
} catch (PDOException $e) {
631674
$exception = $e;
632675
} finally {
@@ -984,7 +1027,7 @@ private function readRows(string $name, array $documents, array $columns): array
9841027
$statement = $this->executeResult($builder->build(), Event::DocumentRead);
9851028
$this->execute($statement);
9861029
/** @var list<list<mixed>> $rows */
987-
$rows = $statement->fetchAll(PDO::FETCH_NUM);
1030+
$rows = $this->fetchExactly(static fn (): array => $statement->fetchAll(PDO::FETCH_NUM));
9881031
$statement->closeCursor();
9891032

9901033
return $rows;
@@ -1536,7 +1579,7 @@ public function find(Document $collection, array $queries = [], ?int $limit = 25
15361579
$this->describeStatement($statement, [], $name);
15371580
$this->execute($statement);
15381581
/** @var array<int, array<string, mixed>> $rows */
1539-
$rows = $statement->fetchAll();
1582+
$rows = $this->fetchExactly(static fn (): array => $statement->fetchAll());
15401583
} catch (PDOException $e) {
15411584
$exception = $e;
15421585
} finally {
@@ -1837,14 +1880,14 @@ public function rawQuery(string $query, array $bindings = []): array
18371880
try {
18381881
$statement = $this->prepareStatement($query);
18391882
foreach ($bindings as $i => $value) {
1840-
$statement->bindValue($i + 1, $value, $this->getPdoType($value));
1883+
$this->bind($statement, $i + 1, $value);
18411884
}
18421885
$this->execute($statement);
18431886
} catch (PDOException $e) {
18441887
throw $this->processException($e);
18451888
}
18461889

1847-
$results = $statement->fetchAll();
1890+
$results = $this->fetchExactly(static fn (): array => $statement->fetchAll());
18481891
$statement->closeCursor();
18491892

18501893
$documents = [];
@@ -3552,7 +3595,7 @@ public function rawMutation(string $query, array $bindings = []): int
35523595
try {
35533596
$statement = $this->prepareStatement($query);
35543597
foreach ($bindings as $i => $value) {
3555-
$statement->bindValue($i + 1, $value, $this->getPdoType($value));
3598+
$this->bind($statement, $i + 1, $value);
35563599
}
35573600
$this->execute($statement);
35583601
} catch (PDOException $e) {
@@ -3661,10 +3704,6 @@ protected function writeContext(array $skipPermissions = []): WriteContext
36613704
* Prepares the SQL statement and binds positional parameters from the Statement.
36623705
* Does NOT call execute() - the caller is responsible for that.
36633706
*
3664-
* Floats are bound as 7.x bound them: a find() binds them in fixed-point notation, every other statement as
3665-
* PHP writes them, so a write keeps a magnitude fixed point would round to zero. A bulk delete binds them as a
3666-
* find() does, so its filters match the rows the find() of the documents it deletes matched.
3667-
*
36683707
* @param string $collection The collection the statement reads or writes, for the profiler
36693708
* @return PDOStatement|DatabasePDOStatement|PDOStatementProxy
36703709
*/
@@ -3676,13 +3715,7 @@ protected function executeResult(Statement $result, ?Event $event = null, string
36763715
if (\is_bool($value) && $this->supports(Capability::IntegerBooleans)) {
36773716
$value = (int) $value;
36783717
}
3679-
if (\is_float($value) && ($event === Event::DocumentFind || $event === Event::DocumentsDelete)) {
3680-
$prepared->bindValue($i + 1, $this->getFloatPrecision($value), PDO::PARAM_STR);
3681-
} elseif (\is_float($value)) {
3682-
$prepared->bindValue($i + 1, $value, PDO::PARAM_STR);
3683-
} else {
3684-
$prepared->bindValue($i + 1, $value, $this->getPdoType($value));
3685-
}
3718+
$this->bind($prepared, $i + 1, $value);
36863719
}
36873720

36883721
return $prepared;
@@ -6266,7 +6299,7 @@ private function runSelect(Statement $result, Event $event, string $collection):
62666299
$statement = $this->executeResult($result, $event, $collection);
62676300
$this->execute($statement);
62686301
/** @var array<int, array<string, mixed>> $results */
6269-
$results = $statement->fetchAll(PDO::FETCH_ASSOC);
6302+
$results = $this->fetchExactly(static fn (): array => $statement->fetchAll(PDO::FETCH_ASSOC));
62706303
} catch (PDOException $e) {
62716304
$exception = $e;
62726305
} finally {
@@ -6520,34 +6553,34 @@ protected function bindOperatorParameters(object $statement, Operator $operator,
65206553
case OperatorType::Divide:
65216554
$value = $values[0] ?? 1;
65226555
$bindKey = "op_{$bindIndex}";
6523-
$statement->bindValue(':'.$bindKey, $value, $this->getPdoType($value));
6556+
$this->bind($statement, ':'.$bindKey, $value);
65246557
$bindIndex++;
65256558

65266559
if (isset($values[1])) {
65276560
$limitKey = "op_{$bindIndex}";
65286561
$limit = self::exactLimit($values[1]);
6529-
$statement->bindValue(':'.$limitKey, $limit, $this->getPdoType($limit));
6562+
$this->bind($statement, ':'.$limitKey, $limit);
65306563
$bindIndex++;
65316564
}
65326565
break;
65336566

65346567
case OperatorType::Modulo:
65356568
$value = $values[0] ?? 1;
65366569
$bindKey = "op_{$bindIndex}";
6537-
$statement->bindValue(':'.$bindKey, $value, $this->getPdoType($value));
6570+
$this->bind($statement, ':'.$bindKey, $value);
65386571
$bindIndex++;
65396572
break;
65406573

65416574
case OperatorType::Power:
65426575
$value = $values[0] ?? 1;
65436576
$bindKey = "op_{$bindIndex}";
6544-
$statement->bindValue(':'.$bindKey, $value, $this->getPdoType($value));
6577+
$this->bind($statement, ':'.$bindKey, $value);
65456578
$bindIndex++;
65466579

65476580
if (isset($values[1])) {
65486581
$maxKey = "op_{$bindIndex}";
65496582
$limit = self::exactLimit($values[1]);
6550-
$statement->bindValue(':'.$maxKey, $limit, $this->getPdoType($limit));
6583+
$this->bind($statement, ':'.$maxKey, $limit);
65516584
$bindIndex++;
65526585
}
65536586
break;
@@ -6596,7 +6629,7 @@ protected function bindOperatorParameters(object $statement, Operator $operator,
65966629
if (is_array($value)) {
65976630
$value = json_encode($value);
65986631
}
6599-
$statement->bindValue(':'.$bindKey, $value, $this->getPdoType($value));
6632+
$this->bind($statement, ':'.$bindKey, $value);
66006633
$bindIndex++;
66016634
break;
66026635

‎src/Database/Adapter/SQLite.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1805,7 +1805,7 @@ protected function bindOperatorParameters(object $statement, Operator $operator,
18051805
if (in_array($filterType, $comparisonTypes)) {
18061806
$bindKey = "op_{$bindIndex}";
18071807
$value = (is_bool($filterValue)) ? (int) $filterValue : $filterValue;
1808-
$statement->bindValue(":{$bindKey}", $value, $this->getPdoType($value));
1808+
$this->bind($statement, ":{$bindKey}", $value);
18091809
$bindIndex++;
18101810
}
18111811
}
@@ -2408,7 +2408,7 @@ protected function executeUpsertBatch(
24082408
);
24092409

24102410
foreach ($bindValues as $key => $binding) {
2411-
$statement->bindValue($key, $binding, $this->getPdoType($binding));
2411+
$this->bind($statement, $key, $binding);
24122412
}
24132413

24142414
$opIndexForBinding = 0;

‎src/Database/Validator/Finite.php‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<?php
2+
3+
namespace Utopia\Database\Validator;
4+
5+
use Utopia\Validator\FloatValidator;
6+
7+
/**
8+
* A float or integer that is neither infinite nor NaN, which no engine stores or compares as a double.
9+
*/
10+
class Finite extends FloatValidator
11+
{
12+
#[\Override]
13+
public function getDescription(): string
14+
{
15+
return 'Value must be a finite float';
16+
}
17+
18+
#[\Override]
19+
public function isValid(mixed $value): bool
20+
{
21+
if (! parent::isValid($value)) {
22+
return false;
23+
}
24+
25+
return \is_numeric($value) && \is_finite((float) $value);
26+
}
27+
}

‎src/Database/Validator/Query/Filter.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use Utopia\Database\Query;
99
use Utopia\Database\Validator\BigInt;
1010
use Utopia\Database\Validator\Datetime as DatetimeValidator;
11+
use Utopia\Database\Validator\Finite;
1112
use Utopia\Database\Validator\Geometry;
1213
use Utopia\Database\Validator\ObjectPath;
1314
use Utopia\Database\Validator\Query\Joined\Attributes;
@@ -18,7 +19,6 @@
1819
use Utopia\Query\Query as BaseQuery;
1920
use Utopia\Query\Schema\ColumnType;
2021
use Utopia\Validator\Boolean;
21-
use Utopia\Validator\FloatValidator;
2222
use Utopia\Validator\Integer;
2323
use Utopia\Validator\Text;
2424

@@ -263,7 +263,7 @@ protected function isValidAttributeAndValues(string $attribute, array $values, M
263263

264264
case ColumnType::Float:
265265
case ColumnType::Double:
266-
$validator = new FloatValidator();
266+
$validator = new Finite();
267267
break;
268268

269269
case ColumnType::Boolean:

‎src/Database/Validator/Query/Having.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@
33
namespace Utopia\Database\Validator\Query;
44

55
use Utopia\Database\Query;
6+
use Utopia\Database\Validator\Finite;
67
use Utopia\Query\Method;
78
use Utopia\Query\Query as BaseQuery;
8-
use Utopia\Validator\FloatValidator;
99

1010
/**
1111
* Validates having query methods: each condition compares an aggregate alias declared in the same
@@ -196,7 +196,7 @@ private function isValidAliasCondition(Query $condition, BaseQuery $aggregation,
196196
return false;
197197
}
198198

199-
$number = new FloatValidator();
199+
$number = new Finite();
200200
foreach ($values as $value) {
201201
if (! $number->isValid($value)) {
202202
$this->message = 'Query value is invalid for aggregate alias "'.$alias.'"';

0 commit comments

Comments
 (0)