drizzle-orm icon indicating copy to clipboard operation
drizzle-orm copied to clipboard

[BUG]: Support MySQL UNSIGNED

Open HaNdTriX opened this issue 1 year ago โ€ข 4 comments

What version of drizzle-orm are you using?

0.21.1

Describe the Bug

When using the following schema definition:

import { mysqlTable, serial, bigint } from "drizzle-orm/mysql-core";

export const users = mysqlTable("users", {
  id: serial("id"),
});

export const pages = mysqlTable("pages", {
  id: serial("id"),
  authorId: bigint("author_id", { mode: "bigint" })
    .notNull()
    .references(() => users.id, { onUpdate: "cascade", onDelete: "cascade" }),
});

Running drizzle-kit generate:mysql generates the following invalid schema:

CREATE TABLE `pages` (
  `id` serial AUTO_INCREMENT,
  `author_id` bigint NOT NULL
);

CREATE TABLE `users` (
  `id` serial AUTO_INCREMENT
);

ALTER TABLE pages 
  ADD CONSTRAINT pages_author_id_users_id_fk 
  FOREIGN KEY (`author_id`) 
  REFERENCES users(`id`) 
    ON DELETE cascade 
    ON UPDATE cascade;

This schema results in the following MySQL-Error:

Referencing column 'author_id' and referenced column 'id' in foreign key constraint 'pages_author_id_users_id_fk' are incompatible.

The issues with this schema are:

  • SERIAL is an alias for BIGINT UNSIGNED NOT NULL AUTO_INCREMENT UNIQUE in MySQL so AUTO_INCREMENT is not needed here.
  • SERIAL alias to an BIGINT UNSIGNED wich is not a valid FOREIGN KEY for author_id since this compiles to SIGNED.

Proposal

To fix this issue we need to be able to mark columns as UNSIGNED afaik. Something like:

interface MySqlBigIntConfig<T extends 'number' | 'bigint' = 'number' | 'bigint'> {
    mode: T;
++  unsigned: boolean
}

Additional Version Info

  • MySQL: 8.0.26
  • drizzle-kit: 0.17.0

Docs

  • https://dev.mysql.com/doc/refman/8.0/en/numeric-type-syntax.html

HaNdTriX avatar Mar 14 '23 09:03 HaNdTriX

@HaNdTriX thanks so much!

We had same problem with unsigned on db introspect command. Drizzle was generating simple int(for example) and used in a code simple int(without unsigned option). It was causing to generating new migration with changing column type. Also good suggestion on a syntax. We were thinking to place it same way

Good thing - it's on a list to be included in one of next releases, so will update under this ticket as well

AndriiSherman avatar Mar 14 '23 10:03 AndriiSherman

Thank you for you fast response!

One addition regarding my proposal:

UNSIGNED is applicable for every number type in MySQL so it makes sense to make apply this setting to all number types.

HaNdTriX avatar Mar 14 '23 10:03 HaNdTriX

@AndriiSherman any update on the progress of this bug. Thanks

notrealanurag avatar May 11 '23 15:05 notrealanurag

Bumping this issues as well. I've commented also on other threads but this seems to be the right one to track unsigned support.

It's also important for MySQL's clustered index means it's very common to use unsigned bigint as primary key and not some uuid. With a unsigned bigint you've an infinite primary key, with signed it's good too but yeah... why make compromises here and it should be an easy fix or no?

205g0 avatar May 31 '23 10:05 205g0

@HaNdTriX I don't know if it helps, but I'm currently creating a custom type as a workaround:

import { customType, mysqlTable, serial } from "drizzle-orm/mysql-core";

const unsignedBigint = customType<{ data: number }>({
    dataType() {
        return "bigint UNSIGNED";
    }
});

export const users = mysqlTable("users", {
    id: serial("id"),
});

export const pages = mysqlTable("pages", {
    id: serial("id"),
    authorId: unsignedBigint("author_id")
        .notNull()
        .references(() => users.id, { onUpdate: "cascade", onDelete: "cascade" }),
});

If you want, you can also create a custom id column type for any type of integer like this:

import { customType } from "drizzle-orm/mysql-core";

type IdType = "tinyint" | "smallint" | "mediumint" | "int" | "bigint";
interface UIntConfig {
    type?: IdType;
}

export const unsignedIntAutoIncrement = customType<{ data: number; config: UIntConfig; primaryKey: true; default: true }>({
    dataType: (config) => {
        return `${config?.type ?? "int"} UNSIGNED AUTO_INCREMENT`;
    }
});


export function id(dbName: string, config?: UIntConfig) {
    return unsignedIntAutoIncrement(dbName, config).primaryKey()
};

mymatsubara avatar Jun 12 '23 00:06 mymatsubara

I would also like to bump this issue (w/ #670). @AndriiSherman, while this gets sorted out maybe @mymatsubara's type could be added as an example in the custom types docs, with a note in the MySQL column types docs.

aretrace avatar Jun 27 '23 02:06 aretrace

There's even a bigger problem: if you use SERIAL in mysql, mysql creates an UNSIGNED column (next to BIGINT, UNIQUE and others). Now, when you want to define foregin keys you must match the exact type of SERIAL which is UNSIGNED but your don't want to have SERIAL auto increment feature or unique because not every foregin key fields need it. But you can't uses UNSIGNED because Drizzle has no API call for this. tldr you can't use FK properly

205g0 avatar Jun 28 '23 08:06 205g0

Put the next issue here because here is the most momentum, when pulling with drizzle-kit from mysql where I have:

CREATE TABLE some_table {
  ...
  some_column tinyint unsigned;
  ...

...drizzle-kit does convert it to .tinyint() in its TypeScript declaration but forgots the import tinyint in that auto-generated file. Once unsigned is removed, all works fine. But then we don't have unsigned on MySQL and waste storage.

@AndriiSherman apologies for spamming everywhere the UNSIGEND issues, seems there are just not supported nowhere. If you have a preferred way how and/or where deal with them, pls let us know ๐Ÿ™‚

Edit: just sawa that you can easily create a new issue from a comment, so I did this too

205g0 avatar Jun 29 '23 16:06 205g0

Any updates?

jeffminsungkim avatar Jul 27 '23 09:07 jeffminsungkim

@thisisandreww one issue we face with your workaround is that every time we run drizzle-kit push:mysql, it always alter the table:

ALTER TABLE `example` MODIFY COLUMN `another_id` bigint UNSIGNED NOT NULL;

It's an issue when the table has data, because drizzle truncates the table even if the column is not really changing.

SamyPesse avatar Aug 13 '23 18:08 SamyPesse

Would love to migrate to drizzle from prisma but definitely need unsigned ints

kelbyfaessler avatar Aug 17 '23 18:08 kelbyfaessler

Any news on this? This is a major issue making drizzle almost unusable and there's no real fix:

/ccing the team to get awareness, would be happy if you guys could focus on this before allocating resources to new features/tools, e.g. Studio, thanks for your understanding @dankochetov @AndriiSherman @AlexBlokh

  • using serial is not possible because FKs can't match the exact data type because of lacking unsigned (mysql), check https://github.com/drizzle-team/drizzle-orm/issues/258#issuecomment-1611022520
  • custom types are no help either like the solution in https://github.com/drizzle-team/drizzle-orm/issues/258#issuecomment-1586393450 because custom types are broken, see https://github.com/drizzle-team/drizzle-orm/issues/765
  • using compact and case-sensitive uuids instead of int ids is also not possible because of https://github.com/drizzle-team/drizzle-orm/issues/1141 and https://github.com/drizzle-team/drizzle-orm/issues/735

the only work-around is to not use drizzle's ts declaration but rather other tools like Atlas and declare in SQL and let drizzle only introspect which brings you quite far at the cost of a so-so DX

it's really sad since drizzle is the only lib with a sane architecture and approach

looking fwd to your feedback ๐Ÿ™‚

205g0 avatar Aug 29 '23 06:08 205g0

This appears to be coming soon! https://github.com/drizzle-team/drizzle-orm/pull/1271

Sparticuz avatar Sep 26 '23 15:09 Sparticuz

Can this be closed as complete in v0.29?

Angelelz avatar Nov 14 '23 04:11 Angelelz

Thanks for the awesome work! ๐Ÿ™

HaNdTriX avatar Nov 14 '23 11:11 HaNdTriX

I do not know if I should be posting this here or opening a new issue, but I have found a bug with unsigned tinyints.

The migration file is generated properly and the database is created correctly:

CREATE TABLE `levels` (
	`id` serial AUTO_INCREMENT NOT NULL,
	`collection_id` bigint unsigned,
	`level` tinyint unsigned NOT NULL,   # <-----
	`name` varchar(191) NOT NULL,
	`description` text,
	`points` int unsigned NOT NULL,
	`utilities` json NOT NULL DEFAULT ('[]'),
	CONSTRAINT `levels_id` PRIMARY KEY(`id`),
	CONSTRAINT `levels_collection_id_level_unique` UNIQUE(`collection_id`,`level`)
);

However, when trying to push, it does not recognize it as unsigned and the following warning appears, preventing the migration:

ยท You're about to change level column type from tinyint to tinyint unsigned with 294 items

I have tried to instrospect the database and this is what I get:

export const levels = mysqlTable("levels", {
	id: serial("id").notNull(),
	collectionId: bigint("collection_id", { mode: "number", unsigned: true }),
	level: tinyint("level").notNull(),                           // <--------------
	name: varchar("name", { length: 191 }).notNull(),
	description: text("description"),
	points: int("points", { unsigned: true }).notNull(),
	utilities: json("utilities").default('[]').notNull(),
},
(table) => {
	return {
		levelsIdPk: primaryKey({ columns: [table.id], name: "levels_id_pk"}),
		id: unique("id").on(table.id),
		levelsCollectionIdLevelUnique: unique("levels_collection_id_level_unique").on(table.collectionId, table.level),
	}
});

Describing the table, however:

+---------------+------------------+------+-----+----------------+-------------------+
| Field         | Type             | Null | Key | Default        | Extra             |
+---------------+------------------+------+-----+----------------+-------------------+
| id            | bigint unsigned  | NO   | PRI | NULL           | auto_increment    |
| collection_id | bigint unsigned  | YES  | MUL | NULL           |                   |
| level         | tinyint unsigned | NO   |     | NULL           |                   |    <---------
| name          | varchar(191)     | NO   |     | NULL           |                   |
| description   | text             | YES  |     | NULL           |                   |
| points        | int unsigned     | NO   |     | NULL           |                   |
| utilities     | json             | NO   |     | _utf8mb4\'[]\' | DEFAULT_GENERATED |
+---------------+------------------+------+-----+----------------+-------------------+

So it looks like introspection is not properly recognizing unsigned tinyints. It is really frustrating because it is preventing all of my migrations.

roman910dev avatar Nov 24 '23 12:11 roman910dev