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
23.51k stars 576 forks source link

[BUG]: using `returning` with `onConflictDoNothing` does not return anything #2474

Open jamiehaywood opened 3 months ago

jamiehaywood commented 3 months ago

What version of drizzle-orm are you using?

0.31.2

What version of drizzle-kit are you using?

0.22.5

Describe the Bug

import { drizzle } from "drizzle-orm/bun-sqlite";
import { Database } from "bun:sqlite";
import * as schema from "./schema";

const sqlite = new Database("sqlite.db");
const db = drizzle(sqlite);

const book = {
  id: "1",
  title: "The Great Gatsby",
  description: "A book about a rich guy",
};

const firstRes = await db.insert(schema.books).values(book).returning().onConflictDoNothing();
// [{id: "1",title: "The Great Gatsby",description: "A book about a rich guy"}]

const secondRes = await db.insert(schema.books).values(book).returning().onConflictDoNothing();
// []

Expected behavior

I would expect returning to always return from the insert statement even if there's a conflict

Environment & setup

No response

Markyiptw commented 3 months ago

Seems like a bug with sqlite: https://sqlite.org/forum/info/a490a6b058261365

Even if it's impossible to fix, I hope the type can be fixed to | undefined

Acorn221 commented 1 month ago

It looks like I'm having a similar issue with postgres. I've had to just do:

  .onConflictDoUpdate({
    target: entity.email,
    set: {
      updatedAt: new Date(),
    }
  }).returning({...});

to just get around the issue for now

dustinboss commented 2 days ago

@jamiehaywood are you actually getting an undefined response from that query?

I had a similar issue, but the query was actually throwing an error because there were no rows to insert (in PostgreSQL). In other words, If I was inserting 100 rows, and all of them were conflicts, the query would fail because there were 0 rows to insert. However, if even one of the queries was not a conflict, then it would not throw an error.

I think that's also how @Acorn221 solved the problem. He converted to "doUpdate" action and created a trivial update, so the database actually had some query to execute.

The error that is thrown doesn't have a specific error type, so it's not easy to check. I had to rely on the text of the error message, which felt very hacky.

Using your example, I did something like this:

try {
      await db.insert(schema.books).values(book).returning().onConflictDoNothing();
    } catch (err) {
      // Catch the known error
      const areAllRowsConflicts =
        err instanceof Error &&
        err.message.includes("values() must be called with at least one value"); // This was part of the full error message
      if (!areAllConflicts) {
        throw err;
      }
 }