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
24.25k stars 621 forks source link

[BUG]: drizzle-kit push:sqlite detects changes directly after initial push #1283

Closed lud-hu closed 2 weeks ago

lud-hu commented 1 year ago

What version of drizzle-orm are you using?

v0.28.6

What version of drizzle-kit are you using?

v0.19.13

Describe the Bug

I have a fairly simple app where I have trouble getting the db migrations done right. For example there is a simple schema:

export const costSplits = sqliteTable("costSplits", {
  id: integer("id", { mode: "number" }).primaryKey({ autoIncrement: true }),
  title: text("title").notNull(),
});

export const costSplitsRelations = relations(costSplits, ({ many, one }) => ({
  users: many(users),
  debts: many(debts),
}));

export const debts = sqliteTable("debts", {
  id: integer("id", { mode: "number" }).primaryKey({ autoIncrement: true }),
  title: text("title").notNull(),
  amount: real("amount").notNull(),
  debtorId: integer("debtorId", { mode: "number" }).notNull(),
  costSplitId: integer("costSplitId", { mode: "number" }).notNull(),
  date: integer("date", { mode: "timestamp" })
    .notNull()
    .default(sql`CURRENT_TIMESTAMP`),
});

export const debtsRelations = relations(debts, ({ many, one }) => ({
  creditorsToDebts: many(creditorsToDebts),
  debtor: one(users, {
    fields: [debts.debtorId],
    references: [users.id],
  }),
  costSplits: one(costSplits, {
    fields: [debts.costSplitId],
    references: [costSplits.id],
  }),
}));

export const users = sqliteTable("users", {
  id: integer("id", { mode: "number" }).primaryKey({ autoIncrement: true }),
  name: text("name").notNull(),
  costSplitId: integer("costSplitId", { mode: "number" }).notNull(),
});

export const usersRelations = relations(users, ({ many, one }) => ({
  creditorsToDebts: many(creditorsToDebts),
  costSplits: one(costSplits, {
    fields: [users.costSplitId],
    references: [costSplits.id],
  }),
}));

export const creditorsToDebts = sqliteTable(
  "creditors_to_debts",
  {
    userId: integer("user_id")
      .notNull()
      .references(() => users.id),
    debtId: integer("debt_id")
      .notNull()
      .references(() => debts.id),
  },
  (t) => ({
    pk: primaryKey(t.userId, t.debtId),
  })
);

export const creditorsToDebtsRelations = relations(
  creditorsToDebts,
  ({ one }) => ({
    debt: one(debts, {
      fields: [creditorsToDebts.debtId],
      references: [debts.id],
    }),
    user: one(users, {
      fields: [creditorsToDebts.userId],
      references: [users.id],
    }),
  })
);

I can push it perfectly to a fresh turso db: bunx drizzle-kit push:sqlite

CREATE TABLE `creditors_to_debts` (
        `user_id` integer NOT NULL,
        `debt_id` integer NOT NULL,
        PRIMARY KEY(`debt_id`, `user_id`),
        FOREIGN KEY (`user_id`) REFERENCES `users`(`id`) ON UPDATE no action ON DELETE no action,
        FOREIGN KEY (`debt_id`) REFERENCES `debts`(`id`) ON UPDATE no action ON DELETE no action
);

CREATE TABLE `debts` (
        `id` integer PRIMARY KEY AUTOINCREMENT NOT NULL,
        `title` text NOT NULL,
        `amount` real NOT NULL,
        `debtorId` integer NOT NULL,
        `costSplitId` integer NOT NULL,
        `date` integer DEFAULT CURRENT_TIMESTAMP NOT NULL
);

CREATE TABLE `costSplits` (
        `id` integer PRIMARY KEY AUTOINCREMENT NOT NULL,
        `title` text NOT NULL
);

CREATE TABLE `users` (
        `id` integer PRIMARY KEY AUTOINCREMENT NOT NULL,
        `name` text NOT NULL,
        `costSplitId` integer NOT NULL
);

DROP TABLE `libsql_wasm_func_table`;

When I execute the same command exactly after the initial push, it already detects some changes which it shouldn't, should it? bunx drizzle-kit push:sqlite

ALTER TABLE `debts` RENAME TO `__old_push_debts`;
CREATE TABLE `debts` (
        `id` integer PRIMARY KEY AUTOINCREMENT NOT NULL,
        `title` text NOT NULL,
        `amount` real NOT NULL,
        `debtorId` integer NOT NULL,
        `costSplitId` integer NOT NULL,
        `date` integer DEFAULT CURRENT_TIMESTAMP NOT NULL
);

INSERT INTO "debts" SELECT * FROM "__old_push_debts";
DROP TABLE `__old_push_debts`;
ALTER TABLE `creditors_to_debts` RENAME TO `__old_push_creditors_to_debts`;
CREATE TABLE `creditors_to_debts` (
        `user_id` integer NOT NULL,
        `debt_id` integer NOT NULL,
        PRIMARY KEY(`debt_id`, `user_id`),
        FOREIGN KEY (`user_id`) REFERENCES `users`(`id`) ON UPDATE no action ON DELETE no action,
        FOREIGN KEY (`debt_id`) REFERENCES `debts`(`id`) ON UPDATE no action ON DELETE no action
);

INSERT INTO "creditors_to_debts" SELECT * FROM "__old_push_creditors_to_debts";
DROP TABLE `__old_push_creditors_to_debts`;

The problem popped up after trying to just add one simple column to an existing db. The migration failed due to a foreign key constrain failure (which it shouldn't even have because I just added one column) and then in the next try the __old... tables as shown in the unnecessary migration cause problems

Expected behavior

No migration needed after executing the push:sqlite command again without changes.

Environment & setup

MacOS, bun, drizzle, turso and sqlite.

blenoski commented 7 months ago

Same issue with latest drizzle-kit and turso driver.

I narrowed down the issue to the .default(sql`CURRENT_TIMESTAMP`) function.

If a column is added with that default clause/function then the table gets altered, copied, and dropped every time db:push is run

blenoski commented 7 months ago
drizzle_push_sqlite_turso_bug
blenoski commented 7 months ago

Reproduction steps:

  1. define a sqlite table schema that uses .default(sql`CURRENT_TIMESTAMP')
  2. Run drizzle-kit: push:sqlite with turso driverĀ  (completes as expected)
  3. Run drizzle-kit: push:sqlite again with no changes

Expect: drizzle-kit reports no changes

Observed: drizzle-kit wants to rename the table, create a new table with the correct name, copy over all the data from the old table to the new table, and finally drop the renamed table

blenoski commented 7 months ago

Update: narrowed the issue down even further to use of sql function inside the default function

blenoski commented 7 months ago

Update: narrowed the issue down even further, seems to be directly triggered by using CURRENT_TIMESTAMP inside of the sql template string

timestamp: text("timestamp").default(sql`CURRENT_TIMESTAMP`),  ==> TRIGGERS ALTER TABLE BUG
timestamp: text("timestamp").default(sql`(abs(42))`),  ==> no alters table bug
nasyrov commented 7 months ago

You're trying to push date into an integer column, the correct way:

    createdAt: integer('created_at', { mode: 'timestamp' })
      .notNull()
      .default(sql`(unixepoch())`),

Ref: https://orm.drizzle.team/learn/guides/timestamp-default-value#sqlite

realmikesolo commented 7 months ago

Hello guys, this issue occurs due to incorrect usage of CURRENT_TIMESTAMP function with sql operator.

When you use functions with sql operators you should always wrap them into parentheses because SQL might not interpret them correctly. In the database, it should be (CURRENT_TIMESTAMP), but if you pass without parentheses Drizzle will think that it is a new default value and will detect the changes.

timestamp: text("timestamp").default(sql`(CURRENT_TIMESTAMP)`) // correct
realmikesolo commented 6 months ago

Hello @lud-hu @blenoski Could you please confirm that these approaches fix your issue? If not, let's discuss it here or in discord

// if you need to store date as string
timestamp: text("timestamp").default(sql`(CURRENT_TIMESTAMP)`)

or

// if you need to store data as number and treat it as Date object in application
timestamp: integer('timestamp', { mode: 'timestamp' }).notNull().default(sql`(unixepoch())`),