Check if Data Type implementes Stringable interface for Primary keys #6946

New Issue

admin · 2026-01-22T15:41:57+01:00

admin commented

2026-01-22 15:41:57 +01:00

Originally created by @michnovka on GitHub (Mar 8, 2022).

Feature Request

I came across this issue recently

Q	A
New Feature	yes
RFC	no

Summary

I think that if field has defined type in PHP Entity, then Doctrine should check whether this type implements Stringable interface (has __toString() method). If not, an error should be thrown, as UnitOfWork cannot keep track of changes and whole ORM is thus broken.

Originally created by @michnovka on GitHub (Mar 8, 2022). ### Feature Request I came across [this issue](https://stackoverflow.com/questions/15080573/doctrine-2-orm-datetime-field-in-identifier/71401564#71401564) recently | Q | A |------------ | ------ | New Feature | yes | RFC | no #### Summary I think that if field has defined type in PHP Entity, then Doctrine should check whether this type implements Stringable interface (has __toString() method). If not, an error should be thrown, as UnitOfWork cannot keep track of changes and whole ORM is thus broken.

admin commented

2026-01-22 15:41:58 +01:00

@derrabus commented on GitHub (Mar 9, 2022):

I wonder if we should rather leverage the type system here. After all, it's the database representation of the ID and the type system could get us that representation.

@derrabus commented on GitHub (Mar 9, 2022): I wonder if we should rather leverage the type system here. After all, it's the database representation of the ID and the type system could get us that representation.

admin commented

2026-01-22 15:41:59 +01:00

@michnovka commented on GitHub (Mar 9, 2022):

@derrabus yes, I agree. We could add Type::convertToPHPString(): string method or convertToPHPStringable(): \Stringable

DBAL would then check if #[ORM\Id] is set on column whether this method is implemented and if not, it would throw an error.

A default Type implementation could be:

    public function convertToPHPStringable($value, AbstractPlatform $platform): \Stringable
    {
        $value = self::convertToPHPValue($value, $platform);
        
        if(!($value instanceof \Stringable))
            throw new \LogicException("PHP value must implement Stringable interface");
        
        return $value;
    }

This would allow for already Stringable types to work without any further modification or method overloading.

@michnovka commented on GitHub (Mar 9, 2022): @derrabus yes, I agree. We could add `Type::convertToPHPString(): string` method or `convertToPHPStringable(): \Stringable` DBAL would then check if `#[ORM\Id]` is set on column whether this method is implemented and if not, it would throw an error. A default `Type` implementation could be: ```php public function convertToPHPStringable($value, AbstractPlatform $platform): \Stringable { $value = self::convertToPHPValue($value, $platform); if(!($value instanceof \Stringable)) throw new \LogicException("PHP value must implement Stringable interface"); return $value; } ``` This would allow for already `Stringable` types to work without any further modification or method overloading.

admin commented

2026-01-22 15:41:59 +01:00

@michnovka commented on GitHub (Mar 9, 2022):

Since the above would work only for PHP8, we can do instead


    public function convertToPHPString($value, AbstractPlatform $platform): string
    {
        $value = self::convertToPHPValue($value, $platform);

        if (is_object($value) && false === method_exists($value, '__toString')) {
            throw new \LogicException("PHP value must be convertible to string");
        }

        return (string) $value;
    }

@michnovka commented on GitHub (Mar 9, 2022): Since the above would work only for PHP8, we can do instead ```php public function convertToPHPString($value, AbstractPlatform $platform): string { $value = self::convertToPHPValue($value, $platform); if (is_object($value) && false === method_exists($value, '__toString')) { throw new \LogicException("PHP value must be convertible to string"); } return (string) $value; } ```

admin commented

2026-01-22 15:41:59 +01:00

@michnovka commented on GitHub (Mar 9, 2022):

@derrabus isnt this more for dbal than orm? as we need to edit Doctrine\DBAL\Types\Type

@michnovka commented on GitHub (Mar 9, 2022): @derrabus isnt this more for dbal than orm? as we need to edit `Doctrine\DBAL\Types\Type`

admin commented

2026-01-22 15:41:59 +01:00

@derrabus commented on GitHub (Mar 9, 2022):

Well, the main problem is that naive string cast inside UnitOfWork. That's an ORM problem. And no, I don't think we have to change the Type class. Its convertToDatabaseValue() method should be pretty much what we need.

@derrabus commented on GitHub (Mar 9, 2022): Well, the main problem is that naive string cast inside `UnitOfWork`. That's an ORM problem. And no, I don't think we have to change the `Type` class. Its `convertToDatabaseValue()` method should be pretty much what we need.

admin commented

2026-01-22 15:42:00 +01:00

@michnovka commented on GitHub (Apr 2, 2022):

@derrabus I ran into similar issue again with https://github.com/doctrine/orm/pull/9624

so given your suggestion, why not use the convertToDatabaseValue() function to flatten IDs?

@michnovka commented on GitHub (Apr 2, 2022): @derrabus I ran into similar issue again with https://github.com/doctrine/orm/pull/9624 so given your suggestion, why not use the convertToDatabaseValue() function to flatten IDs?

admin commented

2026-01-22 15:42:01 +01:00

@beberlei commented on GitHub (Apr 2, 2022):

There are some historical reasons why this was not done before, bugs of this kind with DateTime instances have been discussed on this tracker many times.

@beberlei commented on GitHub (Apr 2, 2022): There are some historical reasons why this was not done before, bugs of this kind with DateTime instances have been discussed on this tracker many times.

admin commented

2026-01-22 15:42:01 +01:00

@beberlei commented on GitHub (Apr 2, 2022):

Onw reasone we dont convert all fields via types to db representation before computing key representation is performance. As this operation is needed many times in each UoW <=> entity interaction .

@beberlei commented on GitHub (Apr 2, 2022): Onw reasone we dont convert all fields via types to db representation before computing key representation is performance. As this operation is needed many times in each UoW <=> entity interaction .

admin commented

2026-01-22 15:42:01 +01:00

@michnovka commented on GitHub (Apr 2, 2022):

@beberlei so is the issue with IdentifierFlattener.php ? The performance point you make is true, but at the same time we already cast all value types to string. I do not believe there would be a huge overhead.

At this point it is impossible to e.g. use BackedEnums as IDs. There is no __toString() allowed in BackedEnums. For other custom types, this method can be overloaded and it will work.

@michnovka commented on GitHub (Apr 2, 2022): @beberlei so is the issue with IdentifierFlattener.php ? The performance point you make is true, but at the same time we already cast all value types to string. I do not believe there would be a huge overhead. At this point it is impossible to e.g. use BackedEnums as IDs. There is no __toString() allowed in BackedEnums. For other custom types, this method can be overloaded and it will work.

admin commented

2026-01-22 15:42:01 +01:00

@michnovka commented on GitHub (Apr 13, 2022):

After working a bit more on similar issue, I still think that the proper way is to extend DBAL

    public function convertToPHPString($value, AbstractPlatform $platform): string
    {
        $value = self::convertToPHPValue($value, $platform);

        if (is_object($value) && false === method_exists($value, '__toString')) {
            throw new \LogicException("PHP value must be convertible to string");
        }

        return (string) $value;
    }

Then let IdentifierFlattener check whether the ID value is stringable. If (and only if) not, fetch the underlying type and call convertToPHPString() method.

Onw reasone we dont convert all fields via types to db representation before computing key representation is performance. As this operation is needed many times in each UoW <=> entity interaction .

This would eliminate this overhead, as it would be called only where necessary (and where currently it would not work and throw exception anyways). Thus the additional work would be done only for case which is currently unsupported. WDYT @beberlei ?

@michnovka commented on GitHub (Apr 13, 2022): After working a bit more on similar issue, I still think that the proper way is to extend DBAL ```php public function convertToPHPString($value, AbstractPlatform $platform): string { $value = self::convertToPHPValue($value, $platform); if (is_object($value) && false === method_exists($value, '__toString')) { throw new \LogicException("PHP value must be convertible to string"); } return (string) $value; } ``` Then let IdentifierFlattener check whether the ID value is stringable. If (and only if) not, fetch the underlying type and call `convertToPHPString()` method. > Onw reasone we dont convert all fields via types to db representation before computing key representation is performance. As this operation is needed many times in each UoW <=> entity interaction . This would eliminate this overhead, as it would be called only where necessary (and where currently it would not work and throw exception anyways). Thus the additional work would be done only for case which is currently unsupported. WDYT @beberlei ?

Sign in to join this conversation.

Branches Tags

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: doctrine/archived-orm#6946