Failing test: Criteria matching on fields with custom column types by mpdude · Pull Request #11897 · doctrine/orm

mpdude · 2025-03-30T20:50:50Z

This adds failing tests that show that Criteria matching with at least in and eq expressions does not work for the case of SQL-based lookups (uninitialized PersistentCollections) when the targeted field uses custom types.

In that case, we need to convert the value used within the expression to what it looks like in the database.

The challenging part is that I do not know how to combine the handling of custom types with the fact that the in and notIn expressions need to deal with query parameters that are arrays.

https://www.doctrine-project.org/projects/doctrine-dbal/en/4.2/reference/data-retrieval-and-manipulation.html#list-of-parameters-conversion seems to be part of the puzzle, but I guess we somehow have to apply the type conversion (derive the value that would be found at the DBMS level) before constructing such a list?

Maybe @morozov or @derrabus could advise, you guys are much more knowledgeable with the DBAL type system.

mpdude · 2025-03-31T07:57:32Z

Not sure wheter #7102 is about the same issue

morozov · 2025-03-31T12:33:01Z

tests/Tests/ORM/Functional/ValueConversionType/ManyToManyCriteriaMatchingTest.php

+    public function provideMatchingExpressions(): Generator
+    {
+        yield [Criteria::expr()->eq('field', 'match this')];
+        yield [Criteria::expr()->in('field', ['match this'])];


Not sure if it's related to custom types at all, but this test fails because the following code doesn't properly handle the IN operator:

orm/src/Persisters/Collection/ManyToManyPersister.php

Lines 257 to 263 in fd041fb

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];

}

It should:

Add parentheses to the SQL.

Repeat the placeholder, $params[] and $paramTypes[] for each element of the value.

#11031 seems to be related to this bug.

morozov · 2025-03-31T13:00:10Z

tests/Tests/ORM/Functional/ValueConversionType/OneToManyCriteriaMatchingTest.php

+    public function provideMatchingExpressions(): Generator
+    {
+        yield [Criteria::expr()->eq('field', 'match this')];
+        yield [Criteria::expr()->in('field', ['match this'])];


And this one fails because by the time when the following line is executed

orm/src/Persisters/Entity/BasicEntityPersister.php

Line 924 in a67f677

$stmt = $this->conn->executeQuery($query, $params, $types);

$types contains [ArrayParameterType::STRING, 'rot13'], so at this point, the information about the custom type is lost.

Right now, the DBAL only supports a fixed set of array types (see ArrayParameterType).

For this to work, we can try introducing the ArrayType class and have it eventually replace the ArrayParameterType enum. This class should accepts the element type in its constructor. For example, new ArrayType(ParameterType::INTEGER) would represent what's currently represented as ArrayParameterType::INTEGER, and new ArrayType('rot13') would represent an array of custom type.

Not sure what effort it would take and how doable this is within the current type system.

Yeah, something like this was my assumption... that we currently cannot combine the array type hint with custom types. If I get you right, your suggestion is to have the ArrayType as a wrapper (not necessarily decorator) around other types.

What's the difference between a wrapper and a decorator? I believe I want to see "typed array" as one of the standard types. It will (I guess) wrap (or be parameterized with) its value type. This way, we can build queries with the array values of any standard or custom type.

The distinction between wrapper and decorator was not sharp enough. What I meant was that ArrayType could be wrapped around any other base type, but (to my understanding) need not implement the same interface as the base types themselves.

A textbook decorator – to my understanding – implements at least the same interface as the objects it wraps, it's transparent from the interface point of view.

Yeah, in this case I mean the decorator. It should wrap a type and be a type.

Let’s discuss this over at doctrine/dbal#6883

github-actions · 2025-06-30T03:28:50Z

There hasn't been any activity on this pull request in the past 90 days, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days.
If you want to continue working on it, please leave a comment.

github-actions · 2025-07-08T03:25:46Z

This pull request was closed due to inactivity.

mpdude · 2025-10-07T13:01:08Z

Retrying to fix this in #12190

@morozov

…ype conversions This PR fixes the `Criteria` matching API for `IN` and `NIN` conditions with values that are arrays, by making sure that type information for the matched field is passed to the DBAL level correctly. Passing the right parameter type to DBAL is important to make sure parameter conversions are applied before matching at the database level. Memory-based collections (`ArrayCollection`s or initialized collection fields) would perform matching on the objects in memory where no type conversion to the database representation is required, giving correct results. But uninitialized collections that have their conditions evaluated at the database level need to convert parameter values to the database representation before performing the comparison. One extra challenge is that the DBAL type system does currently not support array-valued parameters for custom types. Only a [limited list of types](https://www.doctrine-project.org/projects/doctrine-dbal/en/4.2/reference/data-retrieval-and-manipulation.html#list-of-parameters-conversion) is supported. I discussed this with @morozov at the Doctrine Hackathon and came to the conclusion that it would be best to work around this limitation at the ORM level. Thus, this fix recognizes array-valued parameters and creates multiple placeholders (like `?, ?, ?`) for them, flattening out the arrays in the parameter list and repeating the type information for each one of them. Previous stalled attempt to fix this was in doctrine#11897.

@morozov

…ype conversions This PR fixes the `Criteria` matching API for `IN` and `NIN` conditions with values that are arrays, by making sure that type information for the matched field is passed to the DBAL level correctly. Passing the right parameter type to DBAL is important to make sure parameter conversions are applied before matching at the database level. Memory-based collections (`ArrayCollection`s or initialized collection fields) would perform matching on the objects in memory where no type conversion to the database representation is required, giving correct results. But uninitialized collections that have their conditions evaluated at the database level need to convert parameter values to the database representation before performing the comparison. One extra challenge is that the DBAL type system does currently not support array-valued parameters for custom types. Only a [limited list of types](https://www.doctrine-project.org/projects/doctrine-dbal/en/4.2/reference/data-retrieval-and-manipulation.html#list-of-parameters-conversion) is supported. I discussed this with @morozov at the Doctrine Hackathon and came to the conclusion that it would be best to work around this limitation at the ORM level. Thus, this fix recognizes array-valued parameters and creates multiple placeholders (like `?, ?, ?`) for them, flattening out the arrays in the parameter list and repeating the type information for each one of them. Previous stalled attempt to fix this was in doctrine#11897.

mpdude changed the title ~~Add failing tests: Criteria matching on fields with custom column types~~ Failing test: Criteria matching on fields with custom column types Mar 30, 2025

Add failing tests: Criteria matching on fields with custom column types

8cfa174

mpdude force-pushed the criteria-matching-custom-type branch from 7b74b56 to 8cfa174 Compare March 30, 2025 20:59

morozov reviewed Mar 31, 2025

View reviewed changes

mpdude mentioned this pull request Mar 31, 2025

Add ArrayType as a generic wrapper for a "list of custom types" doctrine/dbal#6883

Closed

github-actions bot added the Stale label Jun 30, 2025

github-actions bot closed this Jul 8, 2025

mpdude mentioned this pull request Oct 7, 2025

Fix collection filtering API for IN/NOT IN comparisons that require type conversions #12190

Merged

mpdude deleted the criteria-matching-custom-type branch October 9, 2025 08:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Failing test: Criteria matching on fields with custom column types#11897

Failing test: Criteria matching on fields with custom column types#11897
mpdude wants to merge 1 commit intodoctrine:2.20.xfrom
mpdude:criteria-matching-custom-type

mpdude commented Mar 30, 2025 •

edited

Loading

Uh oh!

mpdude commented Mar 31, 2025

Uh oh!

morozov Mar 31, 2025 •

edited

Loading

Uh oh!

W0rma Mar 31, 2025

Uh oh!

morozov Mar 31, 2025 •

edited

Loading

Uh oh!

mpdude Mar 31, 2025

Uh oh!

morozov Mar 31, 2025 •

edited

Loading

Uh oh!

mpdude Mar 31, 2025

Uh oh!

morozov Mar 31, 2025

Uh oh!

mpdude Mar 31, 2025

Uh oh!

github-actions bot commented Jun 30, 2025

Uh oh!

github-actions bot commented Jul 8, 2025

Uh oh!

mpdude commented Oct 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	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];
	}

Conversation

mpdude commented Mar 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mpdude commented Mar 31, 2025

Uh oh!

morozov Mar 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

W0rma Mar 31, 2025

Choose a reason for hiding this comment

Uh oh!

morozov Mar 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mpdude Mar 31, 2025

Choose a reason for hiding this comment

Uh oh!

morozov Mar 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mpdude Mar 31, 2025

Choose a reason for hiding this comment

Uh oh!

morozov Mar 31, 2025

Choose a reason for hiding this comment

Uh oh!

mpdude Mar 31, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jun 30, 2025

Uh oh!

github-actions bot commented Jul 8, 2025

Uh oh!

mpdude commented Oct 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mpdude commented Mar 30, 2025 •

edited

Loading

morozov Mar 31, 2025 •

edited

Loading

morozov Mar 31, 2025 •

edited

Loading

morozov Mar 31, 2025 •

edited

Loading