Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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 this issue Dec 31, 2018 · 11 comments

Comments

@hostep
Copy link

hostep commented Dec 31, 2018

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
Copy link
Member

Ocramius commented Dec 31, 2018

@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
Copy link
Member

lcobucci commented Dec 31, 2018

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 🤣

@greg0ire
Copy link
Member

greg0ire commented Dec 31, 2018

Great Scott!

@morozov
Copy link
Member

morozov commented Dec 31, 2018

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.

@Majkl578 Majkl578 added MariaDB and removed MySQL labels Dec 31, 2018
@morozov
Copy link
Member

morozov commented Dec 31, 2018

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
Copy link
Author

hostep commented Jan 1, 2019

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.

  1. 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)
  2. 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
Copy link

lyrixx commented Jan 2, 2019

Hello,

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

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
Copy link

mcfedr commented Jan 16, 2019

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
Copy link

vincentbab commented Jun 28, 2019

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
Copy link

Nemo64 commented Mar 24, 2020

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
Copy link

binarious commented Oct 15, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests