doctrine / dbal

Doctrine Database Abstraction Layer
https://www.doctrine-project.org/projects/dbal.html
MIT License
9.48k stars 1.34k forks source link

Version 2.9.2 keeps generating the same migration on an index where a length is defined on a varchar column #3419

Open hostep opened 5 years ago

hostep commented 5 years ago

Bug Report

Q A
BC Break ?
Version 2.9.2

Summary

I just updated to version 2.9.2 of doctrine/dbal and started noticing that when running bin/console doctrine:migrations:diff it generated a new migration, then running bin/console doctrine:migrations:migrate followed by another bin/console doctrine:migrations:diff generates the exact same migration again.

This didn't happen in version 2.9.1

This is my first time opening an issue here, so if some info is missing which can help you, please let me know.

How to reproduce

Using 10.0.33-MariaDB server_version is not defined in the configuration (doctrine.yaml).

Have a database table with the following format:

CREATE TABLE `enqueue` (
  `id` char(36) COLLATE utf8mb4_unicode_ci NOT NULL COMMENT '(DC2Type:guid)',
  `published_at` bigint(20) NOT NULL,
  `body` longtext COLLATE utf8mb4_unicode_ci,
  `headers` longtext COLLATE utf8mb4_unicode_ci,
  `properties` longtext COLLATE utf8mb4_unicode_ci,
  `redelivered` tinyint(1) DEFAULT NULL,
  `queue` varchar(255) COLLATE utf8mb4_unicode_ci NOT NULL,
  `priority` smallint(6) DEFAULT NULL,
  `delayed_until` bigint(20) DEFAULT NULL,
  `time_to_live` bigint(20) DEFAULT NULL,
  `delivery_id` char(36) COLLATE utf8mb4_unicode_ci DEFAULT NULL COMMENT '(DC2Type:guid)',
  `redeliver_after` bigint(20) DEFAULT NULL,
  PRIMARY KEY (`id`),
  KEY `IDX_CFC35A68AA0BDFF712136921` (`redeliver_after`,`delivery_id`),
  KEY `IDX_CFC35A68E0669C0612136921` (`time_to_live`,`delivery_id`),
  KEY `IDX_CFC35A6862A6DC27E0D4FDE17FFD7F63121369211A065DF8BF396750` (`priority`,`published_at`,`queue`(191),`delivery_id`,`delayed_until`,`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;

Running bin/console doctrine:migrations:diff generates the following migration:

$this->addSql('DROP INDEX IDX_CFC35A6862A6DC27E0D4FDE17FFD7F63121369211A065DF8BF396750 ON enqueue');
$this->addSql('CREATE INDEX IDX_CFC35A6862A6DC27E0D4FDE17FFD7F63121369211A065DF8BF396750 ON enqueue (priority, published_at, queue, delivery_id, delayed_until, id)');

Notice the length of the varchar column queue is not specified in the index here, but gets set when executing the migration. Everytime a diff is generated, it doesn't see the length of this field in the index.

The entity class is something like this:

<?php

declare(strict_types=1);

/*********************************************************************************************/
/* DO NOT USE THIS ENTITY, IS HACK FOR https://github.com/php-enqueue/enqueue-dev/issues/271 */
/*********************************************************************************************/

namespace App\Entity;

use Doctrine\ORM\Mapping as ORM;

/**
 * @ORM\Entity(repositoryClass="App\Repository\EnqueueRepository")
 * @ORM\Table(
    name="enqueue",
    indexes={
        @ORM\Index(name="IDX_CFC35A6862A6DC27E0D4FDE17FFD7F63121369211A065DF8BF396750", columns={"priority", "published_at", "queue", "delivery_id", "delayed_until", "id"}),
        @ORM\Index(name="IDX_CFC35A68AA0BDFF712136921", columns={"redeliver_after", "delivery_id"}),
        @ORM\Index(name="IDX_CFC35A68E0669C0612136921", columns={"time_to_live", "delivery_id"})
    })
 */
class Enqueue
{
    /**
     * @ORM\Id()
     * @ORM\Column(type="guid")
     */
    private $id;

    /**
     * @ORM\Column(type="bigint")
     */
    private $published_at;

    /**
     * @ORM\Column(type="text", nullable=true)
     */
    private $body;

    /**
     * @ORM\Column(type="text", nullable=true)
     */
    private $headers;

    /**
     * @ORM\Column(type="text", nullable=true)
     */
    private $properties;

    /**
     * @ORM\Column(type="boolean", nullable=true)
     */
    private $redelivered;

    /**
     * @ORM\Column(type="string", length=255)
     */
    private $queue;

    /**
     * @ORM\Column(type="smallint", nullable=true)
     */
    private $priority;

    /**
     * @ORM\Column(type="bigint", nullable=true)
     */
    private $delayed_until;

    /**
     * @ORM\Column(type="bigint", nullable=true)
     */
    private $time_to_live;

    /**
     * @ORM\Column(type="guid", nullable=true)
     */
    private $delivery_id;

    /**
     * @ORM\Column(type="bigint", nullable=true)
     */
    private $redeliver_after;
}

Thanks!

Ocramius commented 5 years ago

@hostep can you try reducing this to two Schema instances containing that table, and seeing if the problem is caused by a mistake in introspection, or in diffing?

lcobucci commented 5 years ago

~Also try to check if 2.9.3 doesn't fix it already~ sorry, I forgot that I had travelled in time and that it will take a few more days to get that version released :rofl:

greg0ire commented 5 years ago

Great Scott!

morozov commented 5 years ago

Notice the length of the varchar column queue is not specified in the index here, but gets set when executing the migration.

Is it some MariaDB-specific behavior? It's definitely not something we account for. From the details above, it's not clear where the (191) comes from.

morozov commented 5 years ago

Additionally, could you please check if recreating the index without specified length produces any warnings and if your DB setup has some strict mode components disabled. See the article:

MySQL would normally let this slide by silently truncating the index to 191 characters, for a possible total of 764 bytes used.

hostep commented 5 years ago

Hi guys

I'm currently on a machine with 10.2.19-MariaDB and the bug doesn't appear here. Also no (191) in the database schema on this version. Will try to find some time to try to figure out more info when back on my other machine, maybe later this week, but can't promise anything at the moment.

If I have to guess where the (191) comes from, it's probably Mariadb itself which added this to make it more obvious that the index is being truncated when using utf8mb4 columns in an index - 3 bytes utf8 (3*255 = 765) vs 4 bytes utf8mb4 (4*191 = 764) - and maybe MySQL indeed silently truncated the index and makes it less obviously that this happens.

More potential related info: 1) Changelog of MySQL 5.7.7 mentions:

The innodb_large_prefix default value was changed to ON. The previous default was OFF. When innodb_file_format is set to Barracuda, innodb_large_prefix=ON allows index key prefixes longer than 767 bytes (up to 3072 bytes) for tables that use a Compressed or Dynamic row format. 2) MariaDB 10.2.1 contains Innodb 5.6.31 and MariaDB 10.2.2 contains Innodb 5.7.14 (I don't know if the version from Innodb follow the mySQL versions, but this would then mean this was also fixed in a default configuration for MariaDB 10.2.2) 3) Laravel docs also seems to mention a fix in a default configuration for MySQL 5.7.7 and MariaDB 10.2.2: Laravel uses the utf8mb4 character set by default, which includes support for storing "emojis" in the database. If you are running a version of MySQL older than the 5.7.7 release or MariaDB older than the 10.2.2 release, you may need to manually configure the default string length generated by migrations in order for MySQL to create indexes for them.

Alternatively, you may enable the innodb_large_prefix option for your database. Refer to your database's documentation for instructions on how to properly enable this option.

I should probably update to a newer version of MySQL/MariaDB, or enable the innodb_large_prefix and related options to fix my issue on my other machine.

But not sure what doctrine/dbal should do about this. In 2.9.1 it worked and was probably unintentionally changed in 2.9.2. Not sure how important this is or if other users also run against this?

Disclaimer: I know very little about all these little database configuration thingies, so take everything I'm saying with a grain of salt, I'm only getting this from a bit of googling around :)

lyrixx commented 5 years ago

Hello,

I got the same issue. This is due (I guess) to this commit: d807849f95d454ca39c4b83b4aaddc3fac4f5593

I got the following diff:

     DROP INDEX rule_action_formatted_source_idx ON rule_action;
     CREATE INDEX rule_action_formatted_source_idx ON rule_action (formatted_source);
     DROP INDEX rule_project_source_input_idx ON rule;
     DROP INDEX rule_formatted_source_idx ON rule;
     CREATE INDEX rule_project_source_input_idx ON rule (project_id, source_input);
     CREATE INDEX rule_formatted_source_idx ON rule (formatted_source);

and when I play the migration, I got this exception:

  SQLSTATE[42000]: Syntax error or access violation: 1170 BLOB/TEXT column 'formatted_source' used in key specification without a key length  

I have updated my entity to add the lengths option

*     @ORM\Index(name="rule_action_formatted_source_idx", columns={"formatted_source"}, options={"lengths": {255}}),

Then I got this diff

     DROP INDEX rule_action_formatted_source_idx ON rule_action;
     CREATE INDEX rule_action_formatted_source_idx ON rule_action (formatted_source(255));
     DROP INDEX rule_project_source_input_idx ON rule;
     DROP INDEX rule_formatted_source_idx ON rule;
     CREATE INDEX rule_project_source_input_idx ON rule (project_id, source_input);
     CREATE INDEX rule_formatted_source_idx ON rule (formatted_source);

Now I can play it, but doctrine keeps telling the DB is not in sync and generate the same diff


server information:

erver:                 MariaDB
Server version:         10.1.26-MariaDB-1~jessie mariadb.org binary distribution
Protocol version:       10
Connection:             database via TCP/IP
Server characterset:    latin1
Db     characterset:    utf8mb4
Client characterset:    utf8
Conn.  characterset:    utf8
mcfedr commented 5 years ago

I dont think this is MariaDB specific, having same issues with MySQL 5.7 - where I have manually added the index length.

Given that its not possible to define the length in doctrine mappings not sure how that can be solved.

EDIT

I just found that it is possible to specify the length, although this isn't documented anywhere,

*         @ORM\UniqueConstraint(columns={"profile_id", "type"}, options={"lengths": {null, 25}})

But this still having this bug, e.g. i get this again and again

DROP INDEX UNIQ_D1ECABB5CCFA12B88CDE5729 ON ProfilePayload;
CREATE UNIQUE INDEX UNIQ_D1ECABB5CCFA12B88CDE5729 ON ProfilePayload (profile_id, type(25));
vincentbab commented 5 years ago

Same issue here with MariaDB 10.3 and dbal 2.9.2. My columns are utf8mb4_unicode_ci and ROW_FORMAT is DYNAMIC (so max index length is 3072 bytes)

I locked my composer.json to dbal 2.8 for now: "doctrine/dbal": "~2.8.0",

Nemo64 commented 4 years ago

I can confirm this issue with mysql 5.6.47 and aws aurora.

The index gets truncated and if you run the database with --sql-mode=STRICT_TRANS_TABLES then this results in an error that the key is too long. (see #3911, aurora always runs in strict mode).

I haven't looked too deep into it but Doctrine\DBAL\Platforms\AbstractPlatform::getIndexFieldDeclarationListSQL generates the field list of fields within the index. The easy fix would be to implement this in Doctrine\DBAL\Platforms\MySqlPlatform and add the length of the key based on the charset and the mysql version (key length in 5.7 was increased).

This will get more difficult when dealing with multi column indexes. I'd look into what mysql does in that situation. Maybe evenly distribute the available bytes across fields? There could be int fields... maybe it's not that easy

binarious commented 4 years ago

This problem occurred to me while switching to utf8mb4. Fixed it setting the index length to 190:

@Index(name="search_idx_name", columns={"name"}, options={"lengths": {190}})

Or by setting the indexed string column length to 190. Both seem to work.

The schema seems to be in sync after this change.