Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the support for custom parameter types in native queries #11543

Merged
merged 1 commit into from
Jul 4, 2024

Conversation

stof
Copy link
Member

@stof stof commented Jul 3, 2024

The Query class (used for DQL queries) takes care of using the value and type as is when a type was specified for a parameter instead of going through the default processing of values.
The NativeQuery class was missing the equivalent check, making the custom type work only if the default processing of values does not convert the value to a different one.

@stof stof added the Bug label Jul 3, 2024
@stof stof force-pushed the fix_native_query_parameter_type branch from e6efb86 to 3d63dfd Compare July 3, 2024 14:16
Copy link
Member

@greg0ire greg0ire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add a test for this? 🙏

@stof stof force-pushed the fix_native_query_parameter_type branch from 3d63dfd to 00f2925 Compare July 4, 2024 13:01
@stof
Copy link
Member Author

stof commented Jul 4, 2024

@greg0ire I added a test similar to the test added in #7528 (which fixed it for DQL queries), which uses a mock to ensure that one part of the value processing is never called in the test.
This is not 100% reliable (if we refactor the value processing to perform checks in a different way, we might end up regressing this fix without failing the test) but it is a lot easier than using a cross-platform functional test which would lead to a failure during the query execution when the custom type does not convert the value (my own use case which broke because of a bug relies on a custom DBAL type which is compatible only with the postgresql platform)

@stof stof force-pushed the fix_native_query_parameter_type branch from 00f2925 to 4b154e1 Compare July 4, 2024 13:06
greg0ire
greg0ire previously approved these changes Jul 4, 2024

class NativeQueryTest extends OrmTestCase
{
use VerifyDeprecations;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@derrabus no reason I think. Removed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created this file by copying another one, and I forgot to remove that trait

The Query class (used for DQL queries) takes care of using the value and
type as is when a type was specified for a parameter instead of going
through the default processing of values.
The NativeQuery class was missing the equivalent check, making the
custom type work only if the default processing of values does not
convert the value to a different one.
@greg0ire greg0ire merged commit 51ad860 into doctrine:2.19.x Jul 4, 2024
58 checks passed
@greg0ire greg0ire added this to the 2.19.7 milestone Jul 4, 2024
@greg0ire
Copy link
Member

greg0ire commented Jul 4, 2024

Thanks @stof !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants