[documentation] Missing caveat on CURRENT_TIMETSAMP default value

[yannoff]

Seems like only datetime type accepts (i.e is parsed correctly) DEFAULT_TIMESTAMP as a default value.

Observed on PostgreSQL, but it should be the same on almost all platforms, since the logic reside in the Doctrine\DBAL\Platforms\AbstractPlatform::getDefaultValueDeclarationSQL() base method.

Given the following entity (using date as column type):

/**
 * @ORM\Entity()
 */
class Acme
{
    /**
     * @ORM\Id
     * @ORM\GeneratedValue
     * @ORM\Column(type="integer")
     */
    private $id;

    /**
     * @ORM\Column(type="date", options={"default": "CURRENT_TIMESTAMP"})
     */
    private $createdAt;
}

Here is the resulting generated SQL:

phpuser@add87ae82c27:/var/www/html$ bin/console doctrine:schema:update --dump-sql

 The following SQL statements will be executed:

     CREATE SEQUENCE acme_id_seq INCREMENT BY 1 MINVALUE 1 START 1;
     CREATE TABLE acme (id INT NOT NULL, created_at DATE DEFAULT 'CURRENT_TIMESTAMP' NOT NULL, PRIMARY KEY(id));

On the other hand, when using datetime as column type :

/**
 * @ORM\Entity()
 */
class Acme
{
    /**
     * @ORM\Id
     * @ORM\GeneratedValue
     * @ORM\Column(type="integer")
     */
    private $id;

    /**
     * @ORM\Column(type="datetime", options={"default": "CURRENT_TIMESTAMP"})
     */
    private $createdAt;
}

The generated SQL is as expected :

phpuser@add87ae82c27:/var/www/html$ bin/console doctrine:schema:update --dump-sql

 The following SQL statements will be executed:

     CREATE SEQUENCE acme_id_seq INCREMENT BY 1 MINVALUE 1 START 1;
     CREATE TABLE acme (id INT NOT NULL, created_at DATE DEFAULT CURRENT_TIMESTAMP NOT NULL, PRIMARY KEY(id));

When using other date types, such as date for instance, that special default value is considered as a string (i.e is wrapped between single quotes).

While it may be totally acceptable to some extents, fact is, there is no any mention about it to be found anywhere in the documentation.

[SenseException]

Hi Yann, what do you propose? Your chosen headline marks this issue as a possible mention in the documentation, but your description reads more like that the expected behaviour should be CURRENT_TIMESTAMP instead of 'CURRENT_TIMESTAMP'.

[yannoff]

Hi Claudio, thanks for the reply :smiley_cat:

I actually marked this issue as a documentation flaw, because I assumed the actual behavior was the expected one.

Here a mapping of doctrine type and the default value constants supported (ie recognized as special values, and therefore not wrapped between quotes):

Doctrine type	Default value
date	CURRENT_DATE
time	CURRENT_TIME
datetime	CURRENT_TIMESTAMP

Which makes sense, if the purpose is to be compatible with the widest number of platforms...

Indeed, whereas the following example:

CREATE TABLE acme (id INT NOT NULL, created_at DATE DEFAULT CURRENT_TIMESTAMP NOT NULL, PRIMARY KEY(id));

works perfectly with PostgreSQL (ie CURRENT_TIMESTAMP gets interpreted as CURRENT_DATE), it raises an error (or silently fails) on MySQL.

So there are two options here :

• Update the documentation to explain the actual behavior, as reported in my table above (which is, IMHO, the simplest way)
• Override the getDefaultValueDeclarationSQL() in extending classes, to be more consistent with each platform specific behavior

If you opt for the 2nd choice, let me know if I can help in any way :)

[yannoff]

As an afterthought... :thinking:

Maybe the whole logic of the Doctrine\DBAL\Platforms\AbstractPlatform::getDefaultValueDeclarationSQL() should be changed :

• CURRENT_TIMESTAMP, CURRENT_DATE & CURRENT_TIME should never be wrapped between quotes for any of the date/time types (be it time, date or datetime)
• Inconsistency between data type and the choosen default value constant (eg: date vs CURRENT_TIMESTAMP on MySQL) should raise an invalid default value exception

[99hops]

I would like to join here and suggest one more approach. Here I override the datetime type to add support for precision

<?php

namespace App\Db;

use DateTime;
use DateTimeInterface;
use Doctrine\DBAL\Types\Type;
use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Types\ConversionException;

/**
 * Override datetime datatype to support microseconds
 */
class DateTimeMicrosecondsType extends Type
{
    const TYPENAME = 'datetime'; // modify to match your type name

    public function getSQLDeclaration(array $fieldDeclaration, AbstractPlatform $platform)
    {
        return 'TIMESTAMP(6) without time zone';
    }

    public function convertToPHPValue($value, AbstractPlatform $platform)
    {
        if ($value === null || $value instanceof DateTimeInterface) {
            return $value;
        }

        $val = DateTime::createFromFormat('Y-m-d H:i:s.u', $value, new \DateTimeZone('UTC'));

        if ( ! $val) {
            $val = date_create($value, new \DateTimeZone('UTC'));
        }

        if ( ! $val) {
            throw ConversionException::conversionFailedFormat(
                $value,
                $this->getName(),
                'Y-m-d H:i:s.u'
            );
        }

        return $val;
    }

    public function convertToDatabaseValue($value, AbstractPlatform $platform)
    {
        if (null === $value) {
            return $value;
        }

        if ($value instanceof DateTimeInterface) {
            return $value->format('Y-m-d H:i:s.u');
        }

        throw ConversionException::conversionFailedInvalidType(
            $value,
            $this->getName(),
            ['null', 'DateTime']
        );
    }

    public function getName()
    {
        return self::TYPENAME;
    }
}

What bugs me I cannot set the default value in any way because the getDefaultValueDeclarationSQL() is in AbstractPlatform and thus adding DEFAULT {WHATEVER} where I wanted to put default (now() at time zone 'utc')

So at the end it would be nice to be able to override getDefaultValueDeclarationSQ() or provide another way to set the default value from within the "custom" type itself.

[SenseException]

The initial idea was an adaptation of the docs but it seems that this becomes more a feature idea.

[acasademont]

Chiming in here. We're also bugged by not being able to use CURRENT_TIMESTAMP(6) as a default value without getting it quoted, seems like only CURRENT_TIMESTAMP is allowed when doing a diff migration (we remove the quotes manually) but then in the diff we always see pending changes.