romeerez / orchid-orm

Orchid ORM
https://orchid-orm.netlify.app/
MIT License
495 stars 14 forks source link

Disregard SQL default for fields with function default #160

Closed IlyaSemenov closed 1 year ago

IlyaSemenov commented 1 year ago

Following #159, I have a problem and a suggestion.

My use case is, I want to use uuidv7 as primary keys for all tables. So I came up with a base table:

import { createBaseTable } from "orchid-orm"
import { uuidv7 } from "uuidv7"

export const BaseTable = createBaseTable({
  columnTypes: (t) => ({
    ...t,
    id: () => t.uuid().primaryKey().default(uuidv7),
  }),
})

a table:

import { BaseTable } from "../base"

export class UserTable extends BaseTable {
  readonly table = "user"

  columns = this.setColumns((t) => ({
    id: t.id(),
    username: t.varchar().unique(),
    password: t.varchar().nullable(),
  }))
}

and a migration:

import { change } from "../rake"

change(async (db) => {
  await db.createTable("user", (t) => ({
    id: t.id(),
    username: t.varchar().unique(),
    password: t.varchar().nullable(),
  }))
})

There are no typing errors but the migration crashes:

> vite-node --script src/rake.ts "reset"

Database test was successfully dropped
Database test successfully created
Created versions table
file:///Users/semenov/work/test/node_modules/.pnpm/pqb@0.18.0/node_modules/pqb/dist/index.mjs:3028
    return `'${JSON.stringify(value).replace(singleQuoteRegex, "''")}'`;
                                    ^

TypeError: Cannot read properties of undefined (reading 'replace')
    at quote (file:///Users/semenov/work/test/node_modules/.pnpm/pqb@0.18.0/node_modules/pqb/src/quote.ts:34:41)
    at columnToSql (file:///Users/semenov/work/test/node_modules/.pnpm/rake-db@2.10.28/node_modules/rake-db/src/migration/migrationUtils.ts:69:28)

My current workaround is adding explicit .default(null) in migration (as @romeerez suggested in #159):

await db.createTable("user", (t) => ({
  id: t.id().default(null), // workaround for rake - not really semantic
  username: t.varchar().unique(),
  password: t.varchar().nullable(),
}))

So what I suggest is to have this logic applied automatically: if a field provides a Node.js function as a default, createTable (and friends) should create a field that has no SQL default.

romeerez commented 1 year ago

Confirming the bug, it's easy to solve and I'll publish an update for it soon.

romeerez commented 1 year ago

Published a fix, it should be working now