migrations icon indicating copy to clipboard operation
migrations copied to clipboard

Unnecessary migration commands on datetime fields

Open ppaulis opened this issue 4 years ago • 5 comments
trafficstars

Bug Report

PHP 7.4.14 Doctrine Migrations 3.0.2 MySQL 5.7 and 8.0 Laminas API Tools application with doctrine/doctrine-orm-module 3.1.2 Docker-compose environment

Q A
BC Break no
Version 3.0.2

Summary

When generating a migration file with vendor/bin/doctrine-module migrations:diff, I always get this for my datetime fields:

$this->addSql('ALTER TABLE table_name CHANGE field_name field_name DATETIME NOT NULL');

despite the table definition already being like this:

CREATE TABLE `table_name` (
  ...
  `field_name` datetime NOT NULL,
  ...
  PRIMARY KEY (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci

Executing this migration of course doesn't do anything. And generating a new migration file afterwards has again the same result. It's also no metadata caching issue. Cache is not used in development environment.

Also the validate-schema command shows this:

root@6acb76fd3968:/var/www/html/api# vendor/bin/doctrine-module orm:validate-schema

Mapping
-------                                                                                                                 
 [OK] The mapping files are correct.                                                                                    
                                                                                                                        
Database
--------                                                                                                                    
 [ERROR] The database schema is not in sync with the current mapping file.                                              
                                                                                                                        
root@6acb76fd3968:/var/www/html/api#

My first guess would be a bug, but perhaps I'm doing simply the configuration wrong? Any help is welcome :-)

Expected behavior

Having an empty migration file if the current schema is in sync with the mapping files.

ppaulis avatar Feb 01 '21 08:02 ppaulis

I'm seeing the exact same behaviour with Doctrine ORM 2.8.2 and Migrations 3.1.0

final class Version20210228040259 extends AbstractMigration
{
    public function getDescription() : string
    {
        return '';
    }

    public function up(Schema $schema) : void
    {
        // this up() migration is auto-generated, please modify it to your needs
        $this->addSql('ALTER TABLE asset_usage_readings ALTER taken_at TYPE TIMESTAMP(0) WITH TIME ZONE');
        $this->addSql('ALTER TABLE asset_usage_readings ALTER taken_at DROP DEFAULT');
        $this->addSql('ALTER TABLE thermal_readings ALTER "timestamp" TYPE TIMESTAMP(0) WITH TIME ZONE');
        $this->addSql('ALTER TABLE thermal_readings ALTER "timestamp" DROP DEFAULT');
    }

    public function down(Schema $schema) : void
    {
        // this down() migration is auto-generated, please modify it to your needs
        $this->addSql('CREATE SCHEMA public');
        $this->addSql('ALTER TABLE asset_usage_readings ALTER taken_at TYPE TIMESTAMP(0) WITH TIME ZONE');
        $this->addSql('ALTER TABLE asset_usage_readings ALTER taken_at DROP DEFAULT');
        $this->addSql('ALTER TABLE thermal_readings ALTER timestamp TYPE TIMESTAMP(0) WITH TIME ZONE');
        $this->addSql('ALTER TABLE thermal_readings ALTER timestamp DROP DEFAULT');
    }
}
create table asset_usage_readings
(
    taken_at   timestamp(0) with time zone not null,
...
    constraint asset_usage_readings_pkey
        primary key (taken_at, vehicle_id)
);
create table thermal_readings
(
    timestamp      timestamp(0) with time zone not null,
...
    constraint thermal_readings_pkey
        primary key (timestamp, device_address)
);

Both these tables use the timestamp along with another column as the primary key due to Time Series partitioning requirements

nspyke avatar Feb 28 '21 04:02 nspyke

Can you please provide a mapping example that results in that behaviour?

SenseException avatar Mar 01 '21 19:03 SenseException

@SenseException just found the source of the problem. It's because I used datetimetz. I'm handling timezones in the application-layer in fact, but having worked on a Postgres-based application clearly didn't do any good here :man_facepalming:

As for @nspyke 's comment, it seems to be Postgres and not MySQL. So I'll leave it up to you to close this issue or keep it open :-)

Thanks!

ppaulis avatar Mar 02 '21 11:03 ppaulis

Yes, I'm using Postgres, and I'm using datetimetz as a preference throughout my application.

Here's my mapping for the asset_usage_readings table

<?xml version="1.0" encoding="utf-8"?>
<doctrine-mapping xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                  xmlns="http://doctrine-project.org/schemas/orm/doctrine-mapping"
                  xmlns:gedmo="http://gediminasm.org/schemas/orm/doctrine-extensions-mapping"
                  xsi:schemaLocation="http://doctrine-project.org/schemas/orm/doctrine-mapping https://www.doctrine-project.org/schemas/orm/doctrine-mapping.xsd">
    <entity repository-class="Core\Repository\Asset\AssetUsageReadingRepository"
            name="Core\Entity\Asset\AssetUsageReading"
            table="asset_usage_readings">
        <id name="takenAt" type="datetime_id"/>
        <id name="vehicle" association-key="true"/>

         <!-- other irrelevant fields -->

    </entity>
</doctrine-mapping>

datetime_id is a custom type extension of Doctrine\DBAL\Types\DateTimeTzType, as I needed to override the format and __toString methods of \DateTime

use DateTime;

class DateTimeIdentifier extends DateTime
{
    public const FORMAT = DATE_ISO8601;

    public function __toString()
    {
        return $this->format();
    }

    public function format($format = null)
    {
        return parent::format($format ?? self::FORMAT);
    }
}

nspyke avatar Mar 02 '21 21:03 nspyke

Might be fixed by #1229 . Please try out 3.4.1 and doctrine/dbal 3.3.1

greg0ire avatar Jan 16 '22 22:01 greg0ire