Replace static `Type::*` calls with instance-based `TypeRegistry` lookups by GromNaN · Pull Request #12421 · doctrine/orm

GromNaN · 2026-03-31T07:16:21Z

All internal callers now resolve types through Configuration::getTypeRegistry() rather than the global static Type::getType() / Type::hasType() / Type::getTypeRegistry() methods. In SqlWalker, the registry is fetched once in the constructor and stored in a $typeRegistry property, avoiding repeated getConfiguration()->getTypeRegistry() chains.

Remaining static calls are justified fallbacks:

Configuration::getTypeRegistry() falls back to Type::getTypeRegistry() for DBAL v3 and 4.0 compatibility (no Configuration::getTypeRegistry() there)
DatabaseDriver: deprecated class, no EM access available

stof · 2026-04-03T13:04:36Z

LengthFunction and CountFunction: getReturnType() has no EM access, accessing built-in INTEGER type only

This indicates a flaw in the architecture IMO. If Function::getReturnType is meant to return a Type instance (rather than the identifier in the type registry), custom functions might need to return one of the custom types.

github-actions · 2026-07-07T04:20:13Z

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.

GromNaN · 2026-07-07T12:44:48Z

Thanks @stof, you're right. The two static fallbacks in CountFunction / LengthFunction are symptoms of the interface: TypedExpression::getReturnType() returns a Type instance without a registry, so implementers have no choice but a global lookup. And SqlWalker converts the instance back to a name via lookupName() right after, so the round-trip is pointless.

I'd like to expose the type name (string) instead. Two options:

New interface returning string, deprecate TypedExpression.
Keep TypedExpression, deprecate getReturnType(), add getReturnTypeName(): string via @method (duck typing). SqlWalker calls it when available, falls back otherwise.

All internal callers now resolve types through Configuration::getTypeRegistry() rather than the global static Type::getType() / Type::hasType() / Type::getTypeRegistry() methods. Remaining static calls are justified fallbacks: - Configuration::getTypeRegistry() falls back to Type::getTypeRegistry() for DBAL v3 compatibility (no Configuration::getTypeRegistry() there) - LengthFunction and CountFunction: getReturnType() has no EM access, accessing built-in INTEGER type only - DatabaseDriver: deprecated class, no EM access available

… methods Replace getType()/hasType() helpers with a $typeRegistry property initialized in the constructor, avoiding repeated getConfiguration()->getTypeRegistry() chain calls.

GromNaN · 2026-08-04T10:00:11Z

Rebased on top of #12543 (now merged into 3.7.x). The concern raised by @stof about LengthFunction / CountFunction relying on a static Type::getType() lookup is resolved there: SqlWalker now reads the return type name through ExpressionWithReturnType::getReturnTypeName(), so no Type instance round-trip remains.

stof · 2026-08-04T10:07:04Z

 use Doctrine\DBAL\ArrayParameterType;
 use Doctrine\DBAL\ParameterType;
-use Doctrine\DBAL\Types\Type;
-use Doctrine\DBAL\Types\Types;


this use statement must not be removed. It is still used to access constants (see the SA errors)

stof · 2026-08-04T10:10:26Z

+    {
+        // @phpstan-ignore function.alreadyNarrowedType (method_exists check is for DBAL v3 compatibility)
+        if (method_exists(parent::class, 'getTypeRegistry')) {
+            return parent::getTypeRegistry();


I'm wondering whether this is a good idea. If the TypeRegistry belongs to the DBAL configuration, we should be reading it from the configuration object of the associated DBAL Connection, not from the ORM Configuration object.
There is no guarantee that the ORM Configuration is used to create the DBAL connection (even though it extends the DBAL Configuration class). Actually, DoctrineBundle configures DBAL and ORM separately, so it is actually guaranteed that they will not match in Symfony projects.

I think the same TypeRegistry will be injected into both by the DoctrineBundle (see doctrine/DoctrineBundle#2221), but having two configuration sources does indeed open the door to inconsistencies. So I'm going to remove Configuration::getTypeRegistry from the ORM and read it from the DBAL configuration instead. I'll use a helper function to fall back to the static method for DBAL versions earlier than 4.5.

Well, the bundle wires the same type registry in the ORM config only because of this PR using a double source of truth. Using the DBAL config as single source of truth for the type registry allows to simplify the bundle PR.

GromNaN

Waiting for doctrine/dbal#7342

stof · 2026-08-04T16:14:33Z

Btw, I think it would make sense to first work on fixing CI failures in the 3.7.x branch instead of merging PRs without a working CI.

greg0ire · 2026-08-04T19:40:02Z

Btw, I think it would make sense to first work on fixing CI failures in the 3.7.x branch instead of merging PRs without a working CI.

Kind of weird we seem not to have a rule enforcing that. I'll look into it, this looks highly irregular.

EDIT: in fact, this is probably normal, it's just that I'm not used to seeing PRs with failing checks that are also approved, I think.

…ration The type registry belongs to the DBAL Configuration used to create the connection, not to the ORM Configuration. Both are configured independently in DoctrineBundle, so they are not guaranteed to share the same registry. Introduce Internal\TypeRegistryLocator::fromConnection() to resolve the registry from the connection, with a fallback to the global registry on DBAL < 4.5. Restore the Types import in PersisterHelper (still used for constants).

TypeRegistryLocator::fromConnection() was called inside loops (temporary table column definitions, schema validation, select clause generation) and once per parameter in array_map. Hoist the lookup to a local variable before the loop, or to a readonly property populated in the constructor for the per-field helpers in AbstractHydrator and BasicEntityPersister. getArrayBindingType() now receives the TypeRegistry instead of the EntityManager, so it is resolved a single time per inferParameterTypes() call.

DBAL 4.5 exposes Configuration::getTypeProvider() returning the new Doctrine\DBAL\Types\TypeProvider interface rather than the final TypeRegistry, so the type source is no longer required to be that concrete class. Renamed TypeRegistryLocator to TypeProviderLocator accordingly, along with the $typeRegistry properties and locals it feeds. The declared types are unions of TypeProvider and TypeRegistry, because the interface only exists as of DBAL 4.5 while these code paths still support older versions. PHP matches the first applicable member of a union and never loads the other, so the union resolves fine when TypeProvider is absent, whereas a sole TypeProvider type would raise a TypeError. Every such union sits on a private property, a private parameter, or the @internal locator, so none of it reaches the public API. DatabaseDriver now reads Column::getTypeName() instead of looking a name back up from an instance, which drops two deprecated calls at once. SqlWalker's deprecated TypedExpression branch keeps using Type::lookupName(), since a Type instance is all it has; both are deprecated and will be removed together.

GromNaN changed the title ~~Replace static Type::* calls with instance-based TypeRegistry lookups~~ Replace static Type::* calls with instance-based TypeRegistry lookups Mar 31, 2026

GromNaN mentioned this pull request Apr 3, 2026

Add #[AsDbalType] attribute doctrine/DoctrineBundle#2197

Merged

GromNaN marked this pull request as ready for review April 3, 2026 08:36

This was referenced Apr 3, 2026

Update #[AsDbalType] attribute to register DBAL types as Symfony services doctrine/DoctrineBundle#2221

Open

Demonstrate #[AsDbalType] GromNaN/symfony-demo#5

Open

github-actions Bot added the Stale label Jul 7, 2026

github-actions Bot removed the Stale label Jul 8, 2026

GromNaN mentioned this pull request Jul 31, 2026

Deprecate TypedExpression in favor of ExpressionWithReturnType #12543

Merged

GromNaN force-pushed the type-registry-instance branch from 460fdc9 to 0bc9c35 Compare July 31, 2026 20:42

GromNaN added 2 commits August 4, 2026 11:42

Store TypeRegistry in property in SqlWalker instead of private helper…

24a4f03

… methods Replace getType()/hasType() helpers with a $typeRegistry property initialized in the constructor, avoiding repeated getConfiguration()->getTypeRegistry() chain calls.

GromNaN force-pushed the type-registry-instance branch from 0bc9c35 to 24a4f03 Compare August 4, 2026 09:45

stof requested changes Aug 4, 2026

View reviewed changes

GromNaN force-pushed the type-registry-instance branch from 351b8de to 2f40411 Compare August 4, 2026 16:01

stof previously approved these changes Aug 4, 2026

View reviewed changes

GromNaN commented Aug 4, 2026

View reviewed changes

Comment thread tests/Tests/ORM/PersistentCollectionTest.php Outdated

GromNaN added 2 commits August 4, 2026 23:48

GromNaN dismissed stof’s stale review via 38c65bf August 4, 2026 21:48

GromNaN force-pushed the type-registry-instance branch from 2f40411 to 38c65bf Compare August 4, 2026 21:48

GromNaN mentioned this pull request Aug 5, 2026

Add TypeRegistry to Configuration and use it in all internal type resolution doctrine/dbal#7342

Open

GromNaN force-pushed the type-registry-instance branch from c1273c9 to f4dd7a1 Compare August 5, 2026 11:04

Navigation Menu

Uh oh!

Replace static `Type::*` calls with instance-based `TypeRegistry` lookups - #12421

Replace static `Type::*` calls with instance-based `TypeRegistry` lookups#12421
GromNaN wants to merge 5 commits into
doctrine:3.7.xfrom
GromNaN:type-registry-instance

GromNaN commented Mar 31, 2026 •

edited

Loading

Uh oh!

stof commented Apr 3, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 7, 2026

Uh oh!

GromNaN commented Jul 7, 2026 •

edited

Loading

Uh oh!

GromNaN commented Aug 4, 2026

Uh oh!

stof Aug 4, 2026

Uh oh!

stof Aug 4, 2026

Uh oh!

GromNaN Aug 4, 2026

Uh oh!

stof Aug 4, 2026

Uh oh!

GromNaN left a comment

Uh oh!

Uh oh!

stof commented Aug 4, 2026

Uh oh!

greg0ire commented Aug 4, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

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

Uh oh!

Uh oh!

stof commented Apr 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 7, 2026

Uh oh!

GromNaN commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GromNaN commented Aug 4, 2026

Uh oh!

stof Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

stof Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

GromNaN Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

stof Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

GromNaN left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

stof commented Aug 4, 2026

Uh oh!

greg0ire commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

GromNaN commented Mar 31, 2026 •

edited

Loading

stof commented Apr 3, 2026 •

edited

Loading

GromNaN commented Jul 7, 2026 •

edited

Loading

greg0ire commented Aug 4, 2026 •

edited

Loading