From b6b527d4c498b668c183efe30802ecddc0de9df3 Mon Sep 17 00:00:00 2001 From: MatTheCat Date: Tue, 21 Mar 2023 16:38:51 +0100 Subject: [PATCH 1/3] Add test case for https://github.com/doctrine/orm/issues/7717 --- .../Tests/Models/GH7717/GH7717Child.php | 26 +++++++++++ .../Tests/Models/GH7717/GH7717Parent.php | 27 +++++++++++ .../ORM/Functional/Ticket/GH7717Test.php | 45 +++++++++++++++++++ 3 files changed, 98 insertions(+) create mode 100644 tests/Doctrine/Tests/Models/GH7717/GH7717Child.php create mode 100644 tests/Doctrine/Tests/Models/GH7717/GH7717Parent.php create mode 100755 tests/Doctrine/Tests/ORM/Functional/Ticket/GH7717Test.php diff --git a/tests/Doctrine/Tests/Models/GH7717/GH7717Child.php b/tests/Doctrine/Tests/Models/GH7717/GH7717Child.php new file mode 100644 index 00000000000..0ea30c8437f --- /dev/null +++ b/tests/Doctrine/Tests/Models/GH7717/GH7717Child.php @@ -0,0 +1,26 @@ +createSchemaForModels( + GH7717Parent::class, + GH7717Child::class + ); + } + + public function testManyToManyPersisterIsNullComparison(): void + { + $childWithNullProperty = new GH7717Child(); + $childWithoutNullProperty = new GH7717Child(); + $childWithoutNullProperty->nullableProperty = 'nope'; + + $parent = new GH7717Parent(); + $parent->children = new ArrayCollection([$childWithNullProperty, $childWithoutNullProperty]); + + $this->_em->persist($parent); + $this->_em->flush(); + $this->_em->clear(); + + $parent = $this->_em->find(GH7717Parent::class, 1); + + $this->assertCount(1, $parent->children->matching(new Criteria(Criteria::expr()->isNull('nullableProperty')))); + } +} From b8179e7f854ef00ba1a9a142f7f4a8980955c247 Mon Sep 17 00:00:00 2001 From: MatTheCat Date: Tue, 21 Mar 2023 19:25:07 +0100 Subject: [PATCH 2/3] Do not hide null equality checks in `SqlValueVisitor::walkComparison` --- .../Collection/ManyToManyPersister.php | 14 ++++++++++---- .../Persisters/Entity/BasicEntityPersister.php | 16 +++++++++------- lib/Doctrine/ORM/Persisters/SqlValueVisitor.php | 12 ++---------- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/Collection/ManyToManyPersister.php b/lib/Doctrine/ORM/Persisters/Collection/ManyToManyPersister.php index 0508aa2104a..892cdd3a7f4 100644 --- a/lib/Doctrine/ORM/Persisters/Collection/ManyToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/Collection/ManyToManyPersister.php @@ -6,6 +6,7 @@ use BadMethodCallException; use Doctrine\Common\Collections\Criteria; +use Doctrine\Common\Collections\Expr\Comparison; use Doctrine\DBAL\Exception as DBALException; use Doctrine\ORM\Mapping\ClassMetadata; use Doctrine\ORM\PersistentCollection; @@ -246,10 +247,15 @@ public function loadCriteria(PersistentCollection $collection, Criteria $criteri foreach ($parameters as $parameter) { [$name, $value, $operator] = $parameter; - $field = $this->quoteStrategy->getColumnName($name, $targetClass, $this->platform); - $whereClauses[] = sprintf('te.%s %s ?', $field, $operator); - $params[] = $value; - $paramTypes[] = PersisterHelper::getTypeOfField($name, $targetClass, $this->em)[0]; + $field = $this->quoteStrategy->getColumnName($name, $targetClass, $this->platform); + + if ($value === null && ($operator === Comparison::EQ || $operator === Comparison::NEQ)) { + $whereClauses[] = sprintf('te.%s %s NULL', $field, $operator === Comparison::EQ ? 'IS' : 'IS NOT'); + } else { + $whereClauses[] = sprintf('te.%s %s ?', $field, $operator); + $params[] = $value; + $paramTypes[] = PersisterHelper::getTypeOfField($name, $targetClass, $this->em)[0]; + } } $tableName = $this->quoteStrategy->getTableName($targetClass, $this->platform); diff --git a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php index 0b655254dfd..6629eb3734d 100644 --- a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php @@ -890,15 +890,17 @@ public function expandCriteriaParameters(Criteria $criteria) $valueVisitor->dispatch($expression); - [$params, $types] = $valueVisitor->getParamsAndTypes(); - - foreach ($params as $param) { - $sqlParams = array_merge($sqlParams, $this->getValues($param)); - } + [, $types] = $valueVisitor->getParamsAndTypes(); foreach ($types as $type) { - [$field, $value] = $type; - $sqlTypes = array_merge($sqlTypes, $this->getTypes($field, $value, $this->class)); + [$field, $value, $operator] = $type; + + if ($value === null && ($operator === Comparison::EQ || $operator === Comparison::NEQ)) { + continue; + } + + $sqlParams = array_merge($sqlParams, $this->getValues($value)); + $sqlTypes = array_merge($sqlTypes, $this->getTypes($field, $value, $this->class)); } return [$sqlParams, $sqlTypes]; diff --git a/lib/Doctrine/ORM/Persisters/SqlValueVisitor.php b/lib/Doctrine/ORM/Persisters/SqlValueVisitor.php index a61d0a25f86..999746faaa9 100644 --- a/lib/Doctrine/ORM/Persisters/SqlValueVisitor.php +++ b/lib/Doctrine/ORM/Persisters/SqlValueVisitor.php @@ -27,18 +27,10 @@ class SqlValueVisitor extends ExpressionVisitor */ public function walkComparison(Comparison $comparison) { - $value = $this->getValueFromComparison($comparison); - $field = $comparison->getField(); - $operator = $comparison->getOperator(); - - if (($operator === Comparison::EQ || $operator === Comparison::IS) && $value === null) { - return null; - } elseif ($operator === Comparison::NEQ && $value === null) { - return null; - } + $value = $this->getValueFromComparison($comparison); $this->values[] = $value; - $this->types[] = [$field, $value, $operator]; + $this->types[] = [$comparison->getField(), $value, $comparison->getOperator()]; return null; } From 314cebfec0975880974bd47cf1b7f6b0959ccd4e Mon Sep 17 00:00:00 2001 From: MatTheCat Date: Wed, 12 Apr 2023 16:37:25 +0200 Subject: [PATCH 3/3] Annotate `GH7717Parent::$children` type --- tests/Doctrine/Tests/Models/GH7717/GH7717Parent.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/Doctrine/Tests/Models/GH7717/GH7717Parent.php b/tests/Doctrine/Tests/Models/GH7717/GH7717Parent.php index d3273f994cc..74bbd954de6 100644 --- a/tests/Doctrine/Tests/Models/GH7717/GH7717Parent.php +++ b/tests/Doctrine/Tests/Models/GH7717/GH7717Parent.php @@ -22,6 +22,8 @@ class GH7717Parent /** * @ORM\ManyToMany(targetEntity="GH7717Child", cascade={"persist"}) + * + * @var Selectable */ public Selectable $children; }