Passing a string to \DateTime results in opaque format() error #5486

New Issue

admin · 2026-01-22T15:08:55+01:00

admin commented

2026-01-22 15:08:55 +01:00

Originally created by @smcjones on GitHub (Mar 29, 2017).

Originally assigned to: @Ocramius on GitHub.

(Sorry, hit enter accidentally on last issue, leaving it empty)

The issue

When you create a \DateTime entity that is not nullable, and assign a default, Doctrine fails. This appears to be because \Doctrine\DBAL\Types\convertToDatabaseValue fails to do the following:

Check if value is a DateTime object.
Attempt to convert a value to a DateTime object.

Example

/**
  * @ORM\Column(name="created", type="datetime", nullable=false)
  */
// fails - format() issue
private $created = 'now'; 

/**
  * @ORM\Column(name="created", type="datetime", nullable=false)
  */
// assumes it is being set in prePersist(). fails - cannot be null issue from Doctrine
private $created;

A fix

It appears the solution might be to change this line:

return ($value !== null)
    ? $value->format($platform->getDateTimeFormatString()) : null;

to this:

return ($value !== null)
    ? $this->convertToPHPValue($value, $platform)->format($platform->getDateTimeFromString()) : null;

or this:

$value = $this->convertToPHPValue($value, $platform);

return ($value !== null)
    ? $value->format($platform->getDateTimeFromString()) : null;

Edits Made

This was pretty poorly thought out at first. I would still like to see Exception handling.

This is my first attempt to contribute to Doctrine2 in any way, so I am reluctant to make a PR and do it incorrectly.

Originally created by @smcjones on GitHub (Mar 29, 2017). Originally assigned to: @Ocramius on GitHub. (Sorry, hit enter accidentally on last issue, leaving it empty) # The issue When you create a \DateTime entity that is not nullable, and assign a default, Doctrine fails. This appears to be because `\Doctrine\DBAL\Types\convertToDatabaseValue` fails to do the following: 1. Check if value is a DateTime object. 2. Attempt to convert a value to a DateTime object. # Example ``` /** * @ORM\Column(name="created", type="datetime", nullable=false) */ // fails - format() issue private $created = 'now'; /** * @ORM\Column(name="created", type="datetime", nullable=false) */ // assumes it is being set in prePersist(). fails - cannot be null issue from Doctrine private $created; ``` # A fix It appears the solution might be to change this line: return ($value !== null) ? $value->format($platform->getDateTimeFormatString()) : null; to this: return ($value !== null) ? $this->convertToPHPValue($value, $platform)->format($platform->getDateTimeFromString()) : null; or this: $value = $this->convertToPHPValue($value, $platform); return ($value !== null) ? $value->format($platform->getDateTimeFromString()) : null; #### Edits Made This was pretty poorly thought out at first. I would still like to see Exception handling. This is my first attempt to contribute to Doctrine2 in any way, so I am reluctant to make a PR and do it incorrectly.

admin added the Bug Invalid labels 2026-01-22 15:08:55 +01:00

admin closed this issue

2026-01-22 15:08:56 +01:00

admin commented

2026-01-22 15:08:56 +01:00

@Ocramius commented on GitHub (Mar 29, 2017):

You wrote:

/**
 * @ORM\Column(name="created", type="datetime", nullable=false)
 */
// fails - format() issue
private $created = 'now';

This is invalid, as a DateTime instance should be provided.

The correct approach would be:

/**
 * @var \DateTime
 * @ORM\Column(name="created", type="datetime", nullable=false)
 */
private $created; 

public function __construct()
{
    $this->created = new \DateTime();
}

An alternative approach is to also pass in a DateTime instance:

public function __construct(\DateTime $when)
{
    $this->created = clone $when;
}

The DBAL type does not work with strings, closing as invalid.

@Ocramius commented on GitHub (Mar 29, 2017): You wrote: ```php /** * @ORM\Column(name="created", type="datetime", nullable=false) */ // fails - format() issue private $created = 'now'; ``` This is invalid, as a `DateTime` instance should be provided. The correct approach would be: ```php /** * @var \DateTime * @ORM\Column(name="created", type="datetime", nullable=false) */ private $created; public function __construct() { $this->created = new \DateTime(); } ``` An alternative approach is to also pass in a `DateTime` instance: ```php public function __construct(\DateTime $when) { $this->created = clone $when; } ``` The DBAL type does not work with strings, closing as `invalid`.

admin commented

2026-01-22 15:08:56 +01:00

@smcjones commented on GitHub (Mar 29, 2017):

Interesting. Could be Symfony-related, but a reverse migration for a legacy app created a string default. Not entirely sure why the PHP mapping can only go one way. It feels a bit arbitrary and unhelpful, especially in annotation format. Thanks anyway.

@smcjones commented on GitHub (Mar 29, 2017): Interesting. Could be Symfony-related, but a reverse migration for a legacy app created a string default. Not entirely sure why the PHP mapping can only go one way. It feels a bit arbitrary and unhelpful, especially in annotation format. Thanks anyway.

admin commented

2026-01-22 15:08:56 +01:00

@Ocramius commented on GitHub (Mar 29, 2017):

Not entirely sure why the PHP mapping can only go one way.

This particular type only understands DateTime and null. Other types understand other type conversions only.

It feels a bit arbitrary and unhelpful, especially in annotation format.

It is arbitrary:

we can't validate every value you pass in, as that would cause a massive overhead
we can't silently ignore invalid values

So far, a crash due to an invalid format() call on a non-DateTime object is the best solution.

@Ocramius commented on GitHub (Mar 29, 2017): > Not entirely sure why the PHP mapping can only go one way. This particular type only understands `DateTime` and `null`. Other types understand other type conversions only. > It feels a bit arbitrary and unhelpful, especially in annotation format. It is arbitrary: 1. we can't validate every value you pass in, as that would cause a massive overhead 2. we can't silently ignore invalid values So far, a crash due to an invalid `format()` call on a non-`DateTime` object is the best solution.

admin commented

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

@smcjones commented on GitHub (Mar 29, 2017):

I get it. It sticks to typehinting and it makes things consistent. Passing via constructor is fine and makes a lot more sense than what I was proposing.

As for this issue, it would be nice if you could at least trigger an exception (would that be Doctrine\DBAL\Types\ConversionException?). This would help tremendously in debugging.

if ($value !== null && !($value instanceof \DateTime) {
    throw new \Doctrine\DBAL\Types\ConversionException(
        sprintf('Cannot pass %s as a \DateTime type', gettype($value)));
}
    
return ($value !== null)
    ? $value->format($platform->getDateTimeFromString()) : null;

@smcjones commented on GitHub (Mar 29, 2017): I get it. It sticks to typehinting and it makes things consistent. Passing via constructor is fine and makes a lot more sense than what I was proposing. As for this issue, it would be nice if you could at least trigger an exception (would that be `Doctrine\DBAL\Types\ConversionException`?). This would help tremendously in debugging. if ($value !== null && !($value instanceof \DateTime) { throw new \Doctrine\DBAL\Types\ConversionException( sprintf('Cannot pass %s as a \DateTime type', gettype($value))); } return ($value !== null) ? $value->format($platform->getDateTimeFromString()) : null;

admin commented

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

@Ocramius commented on GitHub (Mar 29, 2017):

Possibly, but the code you are looking at is extremely performance
sensitive. In php 7, you just get a throwable...

On 29 Mar 2017 2:44 p.m., "Sean" notifications@github.com wrote:

I get it. It sticks to typehinting and it makes things consistent. Passing
via constructor is fine and makes a lot more sense than what I was
proposing.

As for this issue, it would be nice if you could at least trigger an
exception (would that be Doctrine\DBAL\Types\ConversionException?) This
would help tremendously in debugging.

if ($value !== null && !($value instanceof \DateTime) {
throw new \Doctrine\DBAL\Types\ConversionException(
sprintf('Cannot pass %s as a \DateTime type', gettype($value)));
}

return ($value !== null)
? $value->format($platform->getDateTimeFromString()) : null;

—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
https://github.com/doctrine/doctrine2/issues/6370#issuecomment-290167527,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAJakELh-kM0z5ka7nC22gIDvDptX60tks5rqpiPgaJpZM4MtPVx
.

@Ocramius commented on GitHub (Mar 29, 2017): Possibly, but the code you are looking at is extremely performance sensitive. In php 7, you just get a throwable... On 29 Mar 2017 2:44 p.m., "Sean" <notifications@github.com> wrote: > I get it. It sticks to typehinting and it makes things consistent. Passing > via constructor is fine and makes a lot more sense than what I was > proposing. > > As for this issue, it would be nice if you could at least trigger an > exception (would that be Doctrine\DBAL\Types\ConversionException?) This > would help tremendously in debugging. > > if ($value !== null && !($value instanceof \DateTime) { > throw new \Doctrine\DBAL\Types\ConversionException( > sprintf('Cannot pass %s as a \DateTime type', gettype($value))); > } > > return ($value !== null) > ? $value->format($platform->getDateTimeFromString()) : null; > > — > You are receiving this because you modified the open/close state. > Reply to this email directly, view it on GitHub > <https://github.com/doctrine/doctrine2/issues/6370#issuecomment-290167527>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AAJakELh-kM0z5ka7nC22gIDvDptX60tks5rqpiPgaJpZM4MtPVx> > . >

admin commented

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

@smcjones commented on GitHub (Mar 29, 2017):

Fair point. I plan on upgrading as soon as I can - as I mentioned, I'm migrating a legacy app. Ultimately adding compatibility for a version of PHP that people are moving away from is probably not the right move...

This will be my last comment: You may want to note in Requirements under Architecture that while it works on PHP 5.4, it is highly recommended that you use PHP 7+.

There are good reasons not to include Exceptions to shave off some milliseconds in software like this, but it makes it significantly harder to track down bad code in PHP 5.6 and below. Unfortunately, that's where most legacy app conversions start out.

@smcjones commented on GitHub (Mar 29, 2017): Fair point. I plan on upgrading as soon as I can - as I mentioned, I'm migrating a legacy app. Ultimately adding compatibility for a version of PHP that people are moving away from is probably not the right move... This will be my last comment: You may want to note in *Requirements* under [Architecture](http://docs.doctrine-project.org/projects/doctrine-orm/en/latest/reference/architecture.html) that while it works on PHP 5.4, it is highly recommended that you use PHP 7+. There are good reasons not to include Exceptions to shave off some milliseconds in software like this, but it makes it significantly harder to track down bad code in PHP 5.6 and below. Unfortunately, that's where most legacy app conversions start out.

admin commented

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

@Ocramius commented on GitHub (Mar 29, 2017):

@smcjones yeah, but doctrine operates (at runtime) under the assumption that entities stay valid at all times. Indeed, having a nicer exception would be cool, but not if it impacts the actual throughput (it will in this case).

@Ocramius commented on GitHub (Mar 29, 2017): @smcjones yeah, but doctrine operates (at runtime) under the assumption that entities stay valid at all times. Indeed, having a nicer exception would be cool, but not if it impacts the actual throughput (it will in this case).

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#5486