DBAL deprecation about CURRENT_TIMESTAMP

[greg0ire]

Since doctrine/dbal#7195 , using e.g. the string CURRENT_TIMESTAMP to represent a default expression for a column is deprecated in favor of using a value object.

To get details on the issue:

composer require doctrine/dbal '4.x@dev'
vendor/bin/phpunit -c ci/github/phpunit/sqlite3.xml  --stop-on-deprecation=CURRENT

1 test triggered 1 deprecation:

1) /home/greg/dev/doctrine-orm/patch/vendor/doctrine/deprecations/src/Deprecation.php:208
Using "CURRENT_TIMESTAMP" as a column default value is deprecated. Use a CurrentTimestamp instance instead. (AbstractPlatform.php:1562 called by AbstractPlatform.php:1490, https://github.com/doctrine/dbal/pull/7195, package doctrine/dbal)

/home/greg/dev/doctrine-orm/patch/vendor/doctrine/deprecations/src/Deprecation.php:208
/home/greg/dev/doctrine-orm/patch/vendor/doctrine/deprecations/src/Deprecation.php:108
/home/greg/dev/doctrine-orm/patch/vendor/doctrine/dbal/src/Platforms/AbstractPlatform.php:1562
/home/greg/dev/doctrine-orm/patch/vendor/doctrine/dbal/src/Platforms/AbstractPlatform.php:1490
/home/greg/dev/doctrine-orm/patch/vendor/doctrine/dbal/src/Platforms/AbstractPlatform.php:1468
/home/greg/dev/doctrine-orm/patch/vendor/doctrine/dbal/src/Platforms/SQLitePlatform.php:277
/home/greg/dev/doctrine-orm/patch/vendor/doctrine/dbal/src/Platforms/AbstractPlatform.php:933
/home/greg/dev/doctrine-orm/patch/vendor/doctrine/dbal/src/Platforms/AbstractPlatform.php:864
/home/greg/dev/doctrine-orm/patch/vendor/doctrine/dbal/src/Platforms/SQLitePlatform.php:554
/home/greg/dev/doctrine-orm/patch/vendor/doctrine/dbal/src/SQL/Builder/CreateSchemaObjectsSQLBuilder.php:55
/home/greg/dev/doctrine-orm/patch/vendor/doctrine/dbal/src/SQL/Builder/CreateSchemaObjectsSQLBuilder.php:26
/home/greg/dev/doctrine-orm/patch/vendor/doctrine/dbal/src/Schema/Schema.php:473
/home/greg/dev/doctrine-orm/patch/src/Tools/SchemaTool.php:112
/home/greg/dev/doctrine-orm/patch/src/Tools/SchemaTool.php:88
/home/greg/dev/doctrine-orm/patch/tests/Tests/OrmFunctionalTestCase.php:493
/home/greg/dev/doctrine-orm/patch/tests/Tests/ORM/Functional/Locking/OptimisticTest.php:40
/home/greg/dev/doctrine-orm/patch/tests/Tests/ORM/Functional/Locking/OptimisticTest.php:50
/home/greg/dev/doctrine-orm/patch/vendor/phpunit/phpunit/phpunit:104
/home/greg/dev/doctrine-orm/patch/vendor/bin/phpunit:122

Triggered by:

* Doctrine\Tests\ORM\Functional\Locking\OptimisticTest::testJoinedChildInsertSetsInitialVersionValue
  /home/greg/dev/doctrine-orm/patch/tests/Tests/ORM/Functional/Locking/OptimisticTest.php:48

The following patch is sufficient to make the issue go away, but introduces a breaking change:

diff --git a/src/Mapping/ClassMetadata.php b/src/Mapping/ClassMetadata.php
index c33f1155a..54363e344 100644
--- a/src/Mapping/ClassMetadata.php
+++ b/src/Mapping/ClassMetadata.php
@@ -7,6 +7,7 @@
 use BackedEnum;
 use BadMethodCallException;
 use Doctrine\DBAL\Platforms\AbstractPlatform;
+use Doctrine\DBAL\Schema\DefaultExpression\CurrentTimestamp;
 use Doctrine\DBAL\Types\Types;
 use Doctrine\Deprecations\Deprecation;
 use Doctrine\Instantiator\Instantiator;
@@ -2456,7 +2457,7 @@ public function setVersionMapping(array &$mapping): void
             if (in_array($mapping['type'], ['integer', 'bigint', 'smallint'], true)) {
                 $mapping['default'] = 1;
             } elseif ($mapping['type'] === 'datetime') {
-                $mapping['default'] = 'CURRENT_TIMESTAMP';
+                $mapping['default'] = new CurrentTimestamp();
             } else {
                 throw MappingException::unsupportedOptimisticLockingType($this->name, $mapping['fieldName'], $mapping['type']);
             }
diff --git a/src/Mapping/FieldMapping.php b/src/Mapping/FieldMapping.php
index f84de2f3b..a6e467a34 100644
--- a/src/Mapping/FieldMapping.php
+++ b/src/Mapping/FieldMapping.php
@@ -71,7 +71,7 @@ final class FieldMapping implements ArrayAccess
     public string|null $declaredField = null;
     public array|null $options        = null;
     public bool|null $version         = null;
-    public string|int|null $default   = null;
+    public string|int|object|null $default   = null;

     /**
      * @param string $type       The type name of the mapped field. Can be one of

We should probably aim for this for the next major version.

Another way to address the issue is this:

diff --git a/src/Tools/SchemaTool.php b/src/Tools/SchemaTool.php
index 43c1e7570..dd7b9b598 100644
--- a/src/Tools/SchemaTool.php
+++ b/src/Tools/SchemaTool.php
@@ -9,6 +9,9 @@
 use Doctrine\DBAL\Schema\AbstractAsset;
 use Doctrine\DBAL\Schema\AbstractSchemaManager;
 use Doctrine\DBAL\Schema\ComparatorConfig;
+use Doctrine\DBAL\Schema\DefaultExpression\CurrentDate;
+use Doctrine\DBAL\Schema\DefaultExpression\CurrentTime;
+use Doctrine\DBAL\Schema\DefaultExpression\CurrentTimestamp;
 use Doctrine\DBAL\Schema\ForeignKeyConstraintEditor;
 use Doctrine\DBAL\Schema\Index;
 use Doctrine\DBAL\Schema\Index\IndexedColumn;
@@ -480,6 +483,12 @@ private function gatherColumn(

         if (isset($mapping->default)) {
             $options['default'] = $mapping->default;
+            $options['default'] = match($options['default']) {
+                'CURRENT_DATE'      => new CurrentDate(),
+                'CURRENT_TIME'      => new CurrentTime(),
+                'CURRENT_TIMESTAMP' => new CurrentTimestamp(),
+                default => $options['default'],
+            };
         }

         if (isset($mapping->columnDefinition)) {

Arguably, it's also a breaking change. Maybe the column type should be checked as well.

[derrabus]

I think, the second solution is what we should do to get ORM applications deprecation-free in short therm.

[greg0ire]

Ah, I just saw that CURRENT_TIMESTAMP is not hardcoded at the AbstractPlatform level. It can vary per platform: https://github.com/doctrine/dbal/blob/7e29724ee7460d9196adaf6bb156202bed2d97e2/src/Platforms/AbstractPlatform.php#L1543-L1553

We should have the same kind of checks as this: $type instanceof Types\PhpDateTimeMappingType && $default === $this->getCurrentTimestampSQL()

Something like this:

            if (class_exists(CurrentTimestamp::class)) {
                $typeRegistry = Type::getTypeRegistry();
                if (
                        $typeRegistry->get($mapping->type) instanceof PhpDateTimeMappingType
                        && $options['default'] === $this->platform->getCurrentTimestampSQL()
                ) {
                    $options['default'] = new CurrentTimestamp();
                }

                if (
                        $typeRegistry->get($mapping->type) instanceof PhpTimeMappingType
                        && $options['default'] === $this->platform->getCurrentTimeSQL()
                ) {
                    $options['default'] = new CurrentTime();
                }

                if (
                        $typeRegistry->get($mapping->type) instanceof PhpDateMappingType
                        && $options['default'] === $this->platform->getCurrentDateSQL()
                ) {
                    $options['default'] = new CurrentDate();
                }
            }

[greg0ire]

Here is an attempt at implementing solution 2: #12262

[greg0ire]

Merged. Now I'm thinking about the next steps.

1. Altering the type of FieldMapping::$default is a breaking change for consumers as well as extending classes. I think it might be an acceptable breaking change though. We'll be using $options['default'] I deprecated $default in favor of that.
2. What should this look like when mapping entities with XML? Something like this?

            <options>
                <option name="default" class="Doctrine\DBAL\Schema\CurrentTimestamp" />
            </options>

Or mabye like this, so as to allow specifying a class only for default?

            <options>
                <default  class="Doctrine\DBAL\Schema\CurrentTimestamp"/>
            </options>