Skip to content

Commit 7f2a331

Browse files
janedbaljanedbal
committed
Fix COALESCE behaviour
1 parent 415c8d4 commit 7f2a331

File tree

4 files changed

+197
-40
lines changed

4 files changed

+197
-40
lines changed

‎README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ Most DQL features are supported, including `GROUP BY`, `DISTINCT`, all flavors o
131131
### Query type inference of expressions
132132

133133
Whether `MAX(e.id)` is fetched as `numeric-string` or `int` highly [depends on drivers, their setup and PHP version](https://github.com/janedbal/php-database-drivers-fetch-test).
134-
This extension autodetects your setup and provides accurate results for `pdo_mysql`, `mysqli`, `pdo_sqlite`, `sqlite3`, `pdo_pgsql` and `pgsql`.
134+
This extension autodetects your setup and provides quite accurate results for `pdo_mysql`, `mysqli`, `pdo_sqlite`, `sqlite3`, `pdo_pgsql` and `pgsql`.
135135
Any other driver will result in mixed.
136136

137137
### Supported methods

‎src/Type/Doctrine/Query/QueryResultTypeWalker.php

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,7 @@ public function walkFunction($function): string
404404
// MIN(col_bigint) => int int int int
405405

406406
$exprType = $this->unmarshalType($function->getSql($this));
407-
$exprType = $this->generalizeLiteralType($exprType, $this->hasAggregateWithoutGroupBy());
407+
$exprType = $this->generalizeConstantType($exprType, $this->hasAggregateWithoutGroupBy());
408408
return $this->marshalType($exprType); // retains underlying type
409409

410410
case $function instanceof AST\Functions\SumFunction:
@@ -428,7 +428,7 @@ public function walkFunction($function): string
428428

429429
$exprType = $this->unmarshalType($this->walkSimpleArithmeticExpression($function->simpleArithmeticExpression));
430430
$exprType = $this->castStringLiteralForFloatExpression($exprType);
431-
$exprType = $this->generalizeLiteralType($exprType, false);
431+
$exprType = $this->generalizeConstantType($exprType, false);
432432

433433
$exprTypeNoNull = TypeCombinator::removeNull($exprType);
434434
$nullable = $this->canBeNull($exprType);
@@ -575,7 +575,7 @@ public function walkFunction($function): string
575575
$type = TypeCombinator::addNull($type);
576576
}
577577

578-
return $this->marshalType($this->generalizeLiteralType($type, false));
578+
return $this->marshalType($this->generalizeConstantType($type, false));
579579

580580
case $function instanceof AST\Functions\SqrtFunction:
581581
// mysql sqlite pdo_pgsql pgsql
@@ -745,15 +745,15 @@ private function inferAvgFunction(AST\Functions\AvgFunction $function): Type
745745
return $this->createFloat($nullable);
746746
}
747747

748-
return $this->generalizeLiteralType($exprType, $nullable);
748+
return $this->generalizeConstantType($exprType, $nullable);
749749
}
750750

751751
if ($driverType === DriverType::PGSQL || $driverType === DriverType::PDO_PGSQL) {
752752
if ($exprTypeNoNull->isInteger()->yes()) {
753753
return $this->createNumericString($nullable);
754754
}
755755

756-
return $this->generalizeLiteralType($exprType, $nullable);
756+
return $this->generalizeConstantType($exprType, $nullable);
757757
}
758758

759759
return new MixedType();
@@ -783,7 +783,7 @@ private function inferSumFunction(AST\Functions\SumFunction $function): Type
783783
return $this->createFloat($nullable);
784784
}
785785

786-
return $this->generalizeLiteralType($exprType, $nullable);
786+
return $this->generalizeConstantType($exprType, $nullable);
787787
}
788788

789789
if ($driverType === DriverType::PDO_MYSQL || $driverType === DriverType::MYSQLI) {
@@ -795,7 +795,7 @@ private function inferSumFunction(AST\Functions\SumFunction $function): Type
795795
return $this->createFloat($nullable);
796796
}
797797

798-
return $this->generalizeLiteralType($exprType, $nullable);
798+
return $this->generalizeConstantType($exprType, $nullable);
799799
}
800800

801801
if ($driverType === DriverType::PGSQL || $driverType === DriverType::PDO_PGSQL) {
@@ -806,7 +806,7 @@ private function inferSumFunction(AST\Functions\SumFunction $function): Type
806806
);
807807
}
808808

809-
return $this->generalizeLiteralType($exprType, $nullable);
809+
return $this->generalizeConstantType($exprType, $nullable);
810810
}
811811

812812
return new MixedType();
@@ -849,6 +849,12 @@ private function createNumericString(bool $nullable): Type
849849
return $nullable ? TypeCombinator::addNull($numericString) : $numericString;
850850
}
851851

852+
private function createString(bool $nullable): Type
853+
{
854+
$string = new StringType();
855+
return $nullable ? TypeCombinator::addNull($string) : $string;
856+
}
857+
852858
private function containsOnlyNumericTypes(
853859
Type ...$checkedTypes
854860
): bool
@@ -876,7 +882,7 @@ private function containsOnlyTypes(
876882
/**
877883
* E.g. to ensure SUM(1) is inferred as int, not 1
878884
*/
879-
private function generalizeLiteralType(Type $type, bool $makeNullable): Type
885+
private function generalizeConstantType(Type $type, bool $makeNullable): Type
880886
{
881887
$containsNull = $this->canBeNull($type);
882888
$typeNoNull = TypeCombinator::removeNull($type);
@@ -893,6 +899,9 @@ private function generalizeLiteralType(Type $type, bool $makeNullable): Type
893899
} elseif ($typeNoNull->isNumericString()->yes()) {
894900
$result = $this->createNumericString($containsNull);
895901

902+
} elseif ($typeNoNull->isString()->yes()) {
903+
$result = $this->createString($containsNull);
904+
896905
} else {
897906
$result = $type;
898907
}
@@ -967,7 +976,10 @@ public function walkCoalesceExpression($coalesceExpression): string
967976
$type = $this->unmarshalType($expression->dispatch($this));
968977
$allTypesContainNull = $allTypesContainNull && $this->canBeNull($type);
969978

970-
$expressionTypes[] = $type;
979+
// Some drivers manipulate the types, lets avoid false positives by generalizing constant types
980+
// e.g. sqlsrv: "COALESCE returns the data type of value with the highest precedence"
981+
// e.g. mysql: COALESCE(1, 'foo') === '1'
982+
$expressionTypes[] = $this->generalizeConstantType($type, false);
971983
}
972984

973985
$type = TypeCombinator::union(...$expressionTypes);
@@ -1684,7 +1696,7 @@ private function inferPlusMinusTimesType(array $termTypes): Type
16841696
$typesNoNull = [];
16851697

16861698
foreach ($termTypes as $termType) {
1687-
$generalizedType = $this->generalizeLiteralType($termType, false);
1699+
$generalizedType = $this->generalizeConstantType($termType, false);
16881700
$types[] = $generalizedType;
16891701
$typesNoNull[] = TypeCombinator::removeNull($generalizedType);
16901702
}
@@ -1765,7 +1777,7 @@ private function inferDivisionType(array $termTypes): Type
17651777
$typesNoNull = [];
17661778

17671779
foreach ($termTypes as $termType) {
1768-
$generalizedType = $this->generalizeLiteralType($termType, false);
1780+
$generalizedType = $this->generalizeConstantType($termType, false);
17691781
$types[] = $generalizedType;
17701782
$typesNoNull[] = TypeCombinator::removeNull($generalizedType);
17711783
}

‎tests/Platform/QueryResultTypeWalkerFetchTypeMatrixTest.php

Lines changed: 168 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1416,8 +1416,8 @@ public static function provideCases(): iterable
14161416
yield 'COALESCE(t.col_bool, t.col_bool)' => [
14171417
'data' => self::dataDefault(),
14181418
'select' => 'SELECT COALESCE(t.col_bool, t.col_bool) FROM %s t',
1419-
'mysql' => self::boolAsInt(),
1420-
'sqlite' => self::boolAsInt(),
1419+
'mysql' => self::int(),
1420+
'sqlite' => self::int(),
14211421
'pdo_pgsql' => self::bool(),
14221422
'pgsql' => self::bool(),
14231423
'mssql' => self::mixed(),
@@ -2232,10 +2232,10 @@ public static function provideCases(): iterable
22322232
yield "MAX('foobar')" => [
22332233
'data' => self::dataDefault(),
22342234
'select' => "SELECT MAX('foobar') FROM %s t",
2235-
'mysql' => TypeCombinator::addNull(new ConstantStringType('foobar')),
2236-
'sqlite' => TypeCombinator::addNull(new ConstantStringType('foobar')),
2237-
'pdo_pgsql' => TypeCombinator::addNull(new ConstantStringType('foobar')),
2238-
'pgsql' => TypeCombinator::addNull(new ConstantStringType('foobar')),
2235+
'mysql' => TypeCombinator::addNull(self::string()),
2236+
'sqlite' => TypeCombinator::addNull(self::string()),
2237+
'pdo_pgsql' => TypeCombinator::addNull(self::string()),
2238+
'pgsql' => TypeCombinator::addNull(self::string()),
22392239
'mssql' => self::mixed(),
22402240
'mysqlResult' => 'foobar',
22412241
'sqliteResult' => 'foobar',
@@ -3348,6 +3348,168 @@ public static function provideCases(): iterable
33483348
'mssqlResult' => '2024-01-31 12:59:59.000000', // doctrine/dbal changes default ReturnDatesAsStrings to true
33493349
'stringify' => self::STRINGIFY_NONE,
33503350
];
3351+
3352+
yield 'COALESCE(SUM(t.col_int_nullable), 0)' => [
3353+
'data' => self::dataDefault(),
3354+
'select' => 'SELECT COALESCE(SUM(t.col_int_nullable), 0) FROM %s t',
3355+
'mysql' => TypeCombinator::union(self::numericString(), self::int()),
3356+
'sqlite' => self::int(),
3357+
'pdo_pgsql' => TypeCombinator::union(self::numericString(), self::int()),
3358+
'pgsql' => TypeCombinator::union(self::numericString(), self::int()),
3359+
'mssql' => self::mixed(),
3360+
'mysqlResult' => '0',
3361+
'sqliteResult' => 0,
3362+
'pdoPgsqlResult' => 0,
3363+
'pgsqlResult' => 0,
3364+
'mssqlResult' => 0,
3365+
'stringify' => self::STRINGIFY_DEFAULT,
3366+
];
3367+
3368+
yield "COALESCE(t.col_int_nullable, 'foo')" => [
3369+
'data' => self::dataDefault(),
3370+
'select' => "SELECT COALESCE(t.col_int_nullable, 'foo') FROM %s t",
3371+
'mysql' => TypeCombinator::union(self::int(), self::string()),
3372+
'sqlite' => TypeCombinator::union(self::int(), self::string()),
3373+
'pdo_pgsql' => null, // COALESCE types cannot be matched
3374+
'pgsql' => null, // COALESCE types cannot be matched
3375+
'mssql' => null, // Conversion failed
3376+
'mysqlResult' => 'foo',
3377+
'sqliteResult' => 'foo',
3378+
'pdoPgsqlResult' => null,
3379+
'pgsqlResult' => null,
3380+
'mssqlResult' => null,
3381+
'stringify' => self::STRINGIFY_DEFAULT,
3382+
];
3383+
3384+
yield "COALESCE(t.col_int, 'foo')" => [
3385+
'data' => self::dataDefault(),
3386+
'select' => "SELECT COALESCE(t.col_int, 'foo') FROM %s t",
3387+
'mysql' => TypeCombinator::union(self::int(), self::string()),
3388+
'sqlite' => TypeCombinator::union(self::int(), self::string()),
3389+
'pdo_pgsql' => null, // COALESCE types cannot be matched
3390+
'pgsql' => null, // COALESCE types cannot be matched
3391+
'mssql' => self::mixed(),
3392+
'mysqlResult' => '9',
3393+
'sqliteResult' => 9,
3394+
'pdoPgsqlResult' => null,
3395+
'pgsqlResult' => null,
3396+
'mssqlResult' => 9,
3397+
'stringify' => self::STRINGIFY_DEFAULT,
3398+
];
3399+
3400+
yield "COALESCE(t.col_bool, 'foo')" => [
3401+
'data' => self::dataDefault(),
3402+
'select' => "SELECT COALESCE(t.col_bool, 'foo') FROM %s t",
3403+
'mysql' => TypeCombinator::union(self::int(), self::string()),
3404+
'sqlite' => TypeCombinator::union(self::int(), self::string()),
3405+
'pdo_pgsql' => null, // COALESCE types cannot be matched
3406+
'pgsql' => null, // COALESCE types cannot be matched
3407+
'mssql' => self::mixed(),
3408+
'mysqlResult' => '1',
3409+
'sqliteResult' => 1,
3410+
'pdoPgsqlResult' => null,
3411+
'pgsqlResult' => null,
3412+
'mssqlResult' => 1,
3413+
'stringify' => self::STRINGIFY_DEFAULT,
3414+
];
3415+
3416+
yield "COALESCE(1, 'foo')" => [
3417+
'data' => self::dataDefault(),
3418+
'select' => "SELECT COALESCE(1, 'foo') FROM %s t",
3419+
'mysql' => TypeCombinator::union(self::int(), self::string()),
3420+
'sqlite' => TypeCombinator::union(self::int(), self::string()),
3421+
'pdo_pgsql' => null, // COALESCE types cannot be matched
3422+
'pgsql' => null, // COALESCE types cannot be matched
3423+
'mssql' => self::mixed(),
3424+
'mysqlResult' => '1',
3425+
'sqliteResult' => 1,
3426+
'pdoPgsqlResult' => null,
3427+
'pgsqlResult' => null,
3428+
'mssqlResult' => 1,
3429+
'stringify' => self::STRINGIFY_DEFAULT,
3430+
];
3431+
3432+
yield 'COALESCE(t.col_int_nullable, 0)' => [
3433+
'data' => self::dataDefault(),
3434+
'select' => 'SELECT COALESCE(t.col_int_nullable, 0) FROM %s t',
3435+
'mysql' => self::int(),
3436+
'sqlite' => self::int(),
3437+
'pdo_pgsql' => self::int(),
3438+
'pgsql' => self::int(),
3439+
'mssql' => self::mixed(),
3440+
'mysqlResult' => 0,
3441+
'sqliteResult' => 0,
3442+
'pdoPgsqlResult' => 0,
3443+
'pgsqlResult' => 0,
3444+
'mssqlResult' => 0,
3445+
'stringify' => self::STRINGIFY_DEFAULT,
3446+
];
3447+
3448+
yield 'COALESCE(t.col_float_nullable, 0)' => [
3449+
'data' => self::dataDefault(),
3450+
'select' => 'SELECT COALESCE(t.col_float_nullable, 0) FROM %s t',
3451+
'mysql' => TypeCombinator::union(self::float(), self::int()),
3452+
'sqlite' => TypeCombinator::union(self::float(), self::int()),
3453+
'pdo_pgsql' => TypeCombinator::union(self::numericString(), self::int()),
3454+
'pgsql' => TypeCombinator::union(self::float(), self::int()),
3455+
'mssql' => self::mixed(),
3456+
'mysqlResult' => 0.0,
3457+
'sqliteResult' => 0,
3458+
'pdoPgsqlResult' => '0',
3459+
'pgsqlResult' => 0.0,
3460+
'mssqlResult' => 0.0,
3461+
'stringify' => self::STRINGIFY_DEFAULT,
3462+
];
3463+
3464+
yield 'COALESCE(t.col_int_nullable, t.col_decimal_nullable, 0)' => [
3465+
'data' => self::dataDefault(),
3466+
'select' => 'SELECT COALESCE(t.col_int_nullable, t.col_decimal_nullable, 0) FROM %s t',
3467+
'mysql' => TypeCombinator::union(self::numericString(), self::int()),
3468+
'sqlite' => TypeCombinator::union(self::float(), self::int()),
3469+
'pdo_pgsql' => TypeCombinator::union(self::numericString(), self::int()),
3470+
'pgsql' => TypeCombinator::union(self::numericString(), self::int()),
3471+
'mssql' => self::mixed(),
3472+
'mysqlResult' => '0.0',
3473+
'sqliteResult' => 0,
3474+
'pdoPgsqlResult' => '0',
3475+
'pgsqlResult' => '0',
3476+
'mssqlResult' => '.0',
3477+
'stringify' => self::STRINGIFY_DEFAULT,
3478+
];
3479+
3480+
yield 'COALESCE(t.col_int_nullable, t.col_decimal_nullable, t.col_float_nullable, 0)' => [
3481+
'data' => self::dataDefault(),
3482+
'select' => 'SELECT COALESCE(t.col_int_nullable, t.col_decimal_nullable, t.col_float_nullable, 0) FROM %s t',
3483+
'mysql' => TypeCombinator::union(self::numericString(), self::int(), self::float()),
3484+
'sqlite' => TypeCombinator::union(self::float(), self::int()),
3485+
'pdo_pgsql' => TypeCombinator::union(self::numericString(), self::int()),
3486+
'pgsql' => TypeCombinator::union(self::numericString(), self::int(), self::float()),
3487+
'mssql' => self::mixed(),
3488+
'mysqlResult' => 0.0,
3489+
'sqliteResult' => 0,
3490+
'pdoPgsqlResult' => '0',
3491+
'pgsqlResult' => 0.0,
3492+
'mssqlResult' => 0.0,
3493+
'stringify' => self::STRINGIFY_DEFAULT,
3494+
];
3495+
3496+
yield 'COALESCE(t.col_int_nullable, t.col_decimal_nullable, t.col_float_nullable, t.col_string)' => [
3497+
'data' => self::dataDefault(),
3498+
'select' => 'SELECT COALESCE(t.col_int_nullable, t.col_decimal_nullable, t.col_float_nullable, t.col_string) FROM %s t',
3499+
'mysql' => TypeCombinator::union(self::string(), self::int(), self::float()),
3500+
'sqlite' => TypeCombinator::union(self::float(), self::int(), self::string()),
3501+
'pdo_pgsql' => null, // COALESCE types cannot be matched
3502+
'pgsql' => null, // COALESCE types cannot be matched
3503+
'mssql' => null, // Error converting data
3504+
'mysqlResult' => 'foobar',
3505+
'sqliteResult' => 'foobar',
3506+
'pdoPgsqlResult' => null,
3507+
'pgsqlResult' => null,
3508+
'mssqlResult' => null,
3509+
'stringify' => self::STRINGIFY_DEFAULT,
3510+
];
3511+
3512+
// TODO test IDENTITY
33513513
}
33523514

33533515
/**
@@ -3700,14 +3862,6 @@ private static function boolOrNull(): Type
37003862
return TypeCombinator::addNull(new BooleanType());
37013863
}
37023864

3703-
private static function boolAsInt(): Type
3704-
{
3705-
return TypeCombinator::union(
3706-
new ConstantIntegerType(0),
3707-
new ConstantIntegerType(1)
3708-
);
3709-
}
3710-
37113865
private static function numericString(): Type
37123866
{
37133867
return new IntersectionType([

‎tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -545,24 +545,15 @@ public function getTestData(): iterable
545545
$this->constantArray([
546546
[
547547
new ConstantIntegerType(1),
548-
TypeCombinator::union(
549-
new ConstantStringType('a'),
550-
new ConstantStringType('b')
551-
),
548+
new StringType(),
552549
],
553550
[
554551
new ConstantIntegerType(2),
555-
TypeCombinator::union(
556-
new ConstantIntegerType(1),
557-
new ConstantIntegerType(2)
558-
),
552+
new IntegerType(),
559553
],
560554
[
561555
new ConstantIntegerType(3),
562-
TypeCombinator::union(
563-
new ConstantStringType('1'),
564-
new ConstantStringType('2')
565-
),
556+
$this->numericString(),
566557
],
567558
]),
568559
'
@@ -1078,7 +1069,7 @@ public function getTestData(): iterable
10781069
],
10791070
[
10801071
new ConstantIntegerType(2),
1081-
new ConstantIntegerType(1),
1072+
new IntegerType(),
10821073
],
10831074
[
10841075
new ConstantStringType('intColumn'),

0 commit comments

Comments
 (0)