drizzle-team / drizzle-orm

Headless TypeScript ORM with a head. Runs on Node, Bun and Deno. Lives on the Edge and yes, it's a JavaScript ORM too 😅
https://orm.drizzle.team
Apache License 2.0
22.13k stars 514 forks source link

[FEATURE]: bigint mode in SQLite integer column #1980

Open hyunbinseo opened 4 months ago

hyunbinseo commented 4 months ago

Describe what you want

9223372036854775807 // SQLite's largest possible integer
9007199254740991 // JavaScript Number.MAX_SAFE_INTEGER

Allow SQLite integer columns to be inferred as JavaScript BigInt.

integer('id', { mode: 'number' })
integer('id', { mode: 'bigint' }) // support this

PostgreSQL and MySQL support this feature in the bigint column.

// will be inferred as `number`
bigint: bigint('bigint', { mode: 'number' })

// will be inferred as `bigint`
bigint: bigint('bigint', { mode: 'bigint' })

This will be useful for Autoincrement columns that can become large.

Related issues:

ksmithut commented 1 month ago

One complication with this will be supporting all the different sqlite drivers. For example, better-sqlite3 has some documentation on how to set up BigInt, but it seems to be sort of an all-or-nothing thing, not column specific. So turning on BigInt for everything would mean that all the other int types would need to convert bigints to normal javascript numbers in order for the types to work.

MrRahulRamkumar commented 1 month ago

Will try and create a PR for this this weekend, but not sure what to do about the complication @ksmithut mentioned.

MrRahulRamkumar commented 1 month ago

Did some digging into this and it looks likes most drivers have a configuration option on how SQLite integers are converted to JavaScript values for the entire database. libsql-client-ts allows you to return integers as numbers, big ints or as strings. (See https://github.com/tursodatabase/libsql-client-ts/issues/32 and https://github.com/tursodatabase/libsql-client-ts/pull/51) Whereas bun:sqlite only supports conversion of all SQLite integers to big ints. (See https://bun.sh/docs/api/sqlite#safeintegers-true).

Since we do not have the ability to configure this per column and not all drivers support conversion to big ints, I suggest we add a new bigint column type which would be used instead of the number column. This column would have the same functionality and modes as number with the only difference being that it is set up to handle the underlying driver returning JavaScript big ints instead of numbers.

So for example, with the libsql driver you would do this:

import { drizzle } from 'drizzle-orm/libsql';
import { createClient } from '@libsql/client';
const client = createClient({
  url: "DATABASE_URL",
  authToken: "DATABASE_AUTH_TOKEN",
  intMode: "bigint"
});
const db = drizzle(client);

// all integer columns would use the bigint column
const table = sqliteTable('table', {
  id: bigint('id')
});

// you can customize integer mode to be number, boolean, timestamp, timestamp_ms
bigint('id', { mode: 'number' })
bigint('id', { mode: 'boolean' })
bigint('id', { mode: 'timestamp_ms' })
bigint('id', { mode: 'timestamp' }) // Date

@dankochetov I can create a PR if you are okay with this aproach

hyunbinseo commented 4 weeks ago

@MrRahulRamkumar If the int mode is set to BigInt, are bigint columns forced?

What would happen to the (existing) integer columns? Would they throw errors?

const client = createClient({ intMode: 'bigint' });

const db = drizzle(client);
const db = drizzle(client, { schema }); // Check for the schema here?

If BigInt → Number conversion has minimal performance impacts, this could be possible:

// Always returns a Number
// If the driver returns Number, keep
// In the driver returns BigInt, covnert
integer('id', { mode: 'number' });

// Always returns a BigInt
// If the driver returns Number, convert
// In the driver returns BigInt, keep
integer('id', { mode: 'bigint' });

Values larger than the Number.MAX_SAFE_INTEGER should be handled by the developer.

MrRahulRamkumar commented 3 weeks ago

@hyunbinseo if the int mode is set to bigint, all integer columns would be returned as Javascript big ints by the driver. Existing integer columns would possibly break as drizzle expects the driver to return Javascript number and not bigint.

Your suggestion would work but we would need to make sure that all integer modes (number, boolean, timestamp, timestamp_ms) are also updated to handle big ints being returned by the driver.

There might also be some cases where the developer would have to manually handle big ints being returned by drizzle.

I will create a PR that implements what you suggested. Let's see what the maintainers think.

xray-robot commented 2 weeks ago

+1