Throwing __toString implementation makes Query(Builder) crash #5835

New Issue

admin · 2026-01-22T15:19:13+01:00

admin commented

2026-01-22 15:19:13 +01:00

Originally created by @ro0NL on GitHub (Jan 9, 2018).

Originally assigned to: @Ocramius on GitHub.

Hi,

I have a Id value object with equals() + isEmpty() + __toString() implementation. I'd like to vary empty ID's on being null yes/no; the constructor signature looks like __construct(string $id = null).

In theory string("") is a known ID value, thus not empty. But this is what the issue is about yes :)

For now im throwing a LogicException in __toString in case the ID value is empty (null), as i dont want to cast such a value to string (as mentioned; an empty string is a different value, technically).

Design-wise all good; but i noticed i ran into this somewhat unexpected errors here and there:

Fatal error: Method Id::__toString() must not throw an exception,
caught LogicException: Cannot cast empty ID in /Doctrine/ORM/UnitOfWork.php on line 1453

^ Happening with persist()

PHP Fatal error:  Method Id::__toString() must not throw an exception, 
caught LogicException: Cannot cast empty ID in /Doctrine/DBAL/Driver/PDOStatement.php on line 67

^ Happening with e.g. IDENTITY(deriving_entity.derived_entity) = :entity_object

(A) Does it need a fix on doctrine side? Or is this not a supported thing or so? In general i think we should just try/catch the casts 🤔 and default to null.
(B) Should i avoid the trow and threat null | string("") equally? Thus both empty.
(C) Should i avoid the string $id = null signature as a one-size-fits-all for scalars; i.e. go with Id::fromString(), Id::fromInt and decide "isEmpty" from there? (That sounds reasonable actually).
(D) If i do "B" could that lead to unexpected clauses? As we query with WHERE field = "" instead of field IS NULL
(E) Should i filter out the empty ids myself (i.e. cast to null beforehand). That's a real hassle for nested derived entities etc.

Originally created by @ro0NL on GitHub (Jan 9, 2018). Originally assigned to: @Ocramius on GitHub. Hi, I have a `Id` value object with `equals() + isEmpty() + __toString()` implementation. I'd like to vary empty ID's on being `null` yes/no; the constructor signature looks like `__construct(string $id = null)`. In theory `string("")` is a known ID value, thus not empty. But this is what the issue is about yes :) For now im throwing a `LogicException` in `__toString` in case the ID value is empty (`null`), as i dont want to cast such a value to string (as mentioned; an empty string is a different value, technically). Design-wise all good; but i noticed i ran into this somewhat unexpected errors here and there: ``` Fatal error: Method Id::__toString() must not throw an exception, caught LogicException: Cannot cast empty ID in /Doctrine/ORM/UnitOfWork.php on line 1453 ``` ^ Happening with `persist()` ``` PHP Fatal error: Method Id::__toString() must not throw an exception, caught LogicException: Cannot cast empty ID in /Doctrine/DBAL/Driver/PDOStatement.php on line 67 ``` ^ Happening with e.g. `IDENTITY(deriving_entity.derived_entity) = :entity_object` ____ - (A) Does it need a fix on doctrine side? Or is this not a supported thing or so? In general i think we should just try/catch the casts :thinking: and default to `null`. - (B) Should i avoid the trow and threat `null | string("")` **equally**? Thus both empty. - (C) Should i avoid the `string $id = null` signature as a one-size-fits-all for scalars; i.e. go with `Id::fromString(), Id::fromInt` and decide "isEmpty" from there? (That sounds reasonable actually). - (D) If i do "B" could that lead to unexpected clauses? As we query with `WHERE field = ""` instead of `field IS NULL` - (E) Should i filter out the empty ids myself (i.e. cast to null beforehand). That's a real hassle for nested derived entities etc.

admin added the Question label 2026-01-22 15:19:13 +01:00

admin closed this issue

2026-01-22 15:19:14 +01:00

admin commented

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

@ro0NL commented on GitHub (Jan 9, 2018):

In general the bigger picture is to have getId(): Id (not ?Id nor ?int).

@ro0NL commented on GitHub (Jan 9, 2018): In general the bigger picture is to have `getId(): Id` (not `?Id` nor `?int)`.

admin commented

2026-01-22 15:19:17 +01:00

@Ocramius commented on GitHub (Jan 9, 2018):

@ro0NL __toString() cannot throw - that is a PHP language limitation

@Ocramius commented on GitHub (Jan 9, 2018): @ro0NL `__toString()` cannot throw - that is a PHP language limitation

admin commented

2026-01-22 15:19:17 +01:00

@ro0NL commented on GitHub (Jan 9, 2018):

@Ocramius You're right! Totally overlooked the must not throw an exception part 😞

In general what do you suggest to safely have getId(): Id? I believe currently doctrine requires a nullable API for PK values.. no? Basically im seeking a VO design for the PK in case it's unknown beforehand (i was going for new Entity(new Id())).

@ro0NL commented on GitHub (Jan 9, 2018): @Ocramius You're right! Totally overlooked the `must not throw an exception` part :disappointed: In general what do you suggest to safely have `getId(): Id`? I believe currently doctrine requires a nullable API for PK values.. no? Basically im seeking a VO design for the PK in case it's unknown beforehand (i was going for `new Entity(new Id())`).

admin commented

2026-01-22 15:19:17 +01:00

@Ocramius commented on GitHub (Jan 9, 2018):

@ro0NL doctrine doesn't use your getter, so you are free to use whatever you like there, but the property must be set to something stringable and not null

@Ocramius commented on GitHub (Jan 9, 2018): @ro0NL doctrine doesn't use your getter, so you are free to use whatever you like there, but the property must be set to something stringable and not `null`

admin commented

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

@ro0NL commented on GitHub (Jan 9, 2018):

Yep, im figuring it out :)

    public function getId(): DomainIdInterface
    {
        return null === $this->id ? new DomainId() : ($this->id = new DomainId($this->id));
    }

In case an empty id is passed to the constructor i just set $this->id = null and ignore the VO. That should do!

Thanks a lot :) needed this.

@ro0NL commented on GitHub (Jan 9, 2018): Yep, im figuring it out :) ``` public function getId(): DomainIdInterface { return null === $this->id ? new DomainId() : ($this->id = new DomainId($this->id)); } ``` In case an empty id is passed to the constructor i just set `$this->id = null` and ignore the VO. That should do! Thanks a lot :) needed this.

admin commented

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

@Ocramius commented on GitHub (Jan 9, 2018):

👍

@Ocramius commented on GitHub (Jan 9, 2018): :+1:

admin referenced this issue

2026-01-22 16:05:18 +01:00

[PR #5835] Included additional time units to DATE_ADD and DATE_SUB functions #9750

Sign in to join this conversation.