Skip to content

ULID fields being written to DB as UUID #39112

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

Closed
warslett opened this issue Nov 18, 2020 · 3 comments
Closed

ULID fields being written to DB as UUID #39112

warslett opened this issue Nov 18, 2020 · 3 comments
Labels

Comments

@warslett
Copy link
Contributor

Symfony version(s) affected: 5.2.0-RC1 (PHP 7.4.11, MySQL 8.0.22)

Description
There is something strange going on with Ulid since upgrading to RC1. I was playing with the new Ulid doctrine data type on the beta and all was working correctly with new entities being created with id fields that looked like Ulid.

Then I think around the time I updated to RC1 I tried persisting a new entity that had an association with an existing entity with a ulid primary key and I got a foreign key constraint error. I don't have the exact error message now (I will try to work back through the steps to reproduce if I can). However it seemed to be trying to set the foreign key value for my new association to a value that looked like a uuid (longer than a ulid and with hyphens) and the foreign key constraint rightly failed because there was no corresponding association with a matching primary key (because the existing entity had a uuid for primary key).

This was just in a local dev database so I just dropped the schema and rebuilt it with migrations and walked through the scenario again and this time I got no error but I checked the database and all my ulid columns are now populated with uuid.

Here is an example entity:

<?php

declare(strict_types=1);

namespace App\Domain\Entity;

use Doctrine\ORM\Mapping as ORM;
use Symfony\Bridge\Doctrine\IdGenerator\UlidGenerator;
use Symfony\Component\Uid\Ulid;

/**
 * @ORM\Entity
 */
class Organisation
{

    /**
     * @ORM\Id
     * @ORM\Column(type="ulid", unique=true)
     * @ORM\GeneratedValue(strategy="CUSTOM")
     * @ORM\CustomIdGenerator(class=UlidGenerator::class)
     * @var Ulid|null
     */
    public ?Ulid $ulid = null;

    /**
     * @ORM\Column(type="string", length=140)
     * @var string|null
     */
    public ?string $name = null;
}

The migration looks like this:

<?php

declare(strict_types=1);

namespace DoctrineMigrations;

use Doctrine\DBAL\Schema\Schema;
use Doctrine\Migrations\AbstractMigration;

/**
 * Auto-generated Migration: Please modify to your needs!
 */
final class Version20201026201204 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('CREATE TABLE organisation (ulid CHAR(36) NOT NULL COMMENT \'(DC2Type:ulid)\', name VARCHAR(140) NOT NULL, PRIMARY KEY(ulid)) DEFAULT CHARACTER SET utf8mb4 COLLATE `utf8mb4_unicode_ci` ENGINE = InnoDB');
    }

    public function down(Schema $schema) : void
    {
        // this down() migration is auto-generated, please modify it to your needs
        $this->addSql('DROP TABLE organisation');
    }
}

When a new entity is persisted with a name I get this in the db:

ulid,name
0175dd53-f32a-b9d5-9841-b4a0bec7aaf0,Acme Ltd.

However in code when I get the entity out the repository and run $organisation->getUlid()->__toString() on the same entity I get a ulid back 01EQEN7WSAQ7ASGGDMM2ZCFAQG

How to reproduce
Create an entity with a Ulid column
Persist the entity
Look in the database
The column contains a Uuid

Expected behaviour
The column should contain a Ulid in the db

@warslett warslett added the Bug label Nov 18, 2020
@nicolas-grekas
Copy link
Member

nicolas-grekas commented Nov 18, 2020

Hello, thanks for the report.
The new behavior is the expected one: database engines that have a native UUID column type should use it also for storing ULIDs. The reason is size of the storage: ULID, exactly like UUID, is a 128b number. That means it fits in 16 bytes when declared as UUID to the DB engine (that would be 26 bytes when using the standard base32 of ULID, or 22 with base58).

@nicolas-grekas
Copy link
Member

Actually, I submitted #39113 to "fix" this issue.

@fabpot fabpot closed this as completed Nov 19, 2020
fabpot added a commit that referenced this issue Nov 19, 2020
…s-grekas)

This PR was merged into the 5.2 branch.

Discussion
----------

[DoctrineBridge] drop binary variants of UID types

| Q             | A
| ------------- | ---
| Branch?       | 5.2
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | #39112
| License       | MIT
| Doc PR        | -

#39112 made me realize that when the DB engine doesn't have a native UUID/GUID type, there's no benefit to representing ULIDs as UUIDs.

This PR proposes to use the native GUID type for both UUIDs and ULIDs only when the DB engine provides such a type, and to use `BINARY(16)` when the DB engine doesn't have a native GUID type.

This leaves us in a situation where, whether the DB engine supports GUID natively or not, UUID and ULID are always stored in the most compact format.

This makes the "binary" variants useless.

MySQL 8 has [functions](https://mysqlserverteam.com/mysql-8-0-uuid-support/) to deal with binary GUID, and so does [SQLite](https://sqlite.org/src/file/ext/misc/uuid.c).

Commits
-------

bdfc205 [DoctrineBridge] drop binary variants of UID types
@warslett
Copy link
Contributor Author

thank you @nicolas-grekas for quick response

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

No branches or pull requests

3 participants