kysely-org / kysely

A type-safe typescript SQL query builder
https://kysely.dev
MIT License
10.44k stars 266 forks source link

Add an `.upsert()` that abstracts on top of `.onConflict()` and `.onDuplicateKeyUpdate()`? #451

Closed nc7s closed 1 year ago

nc7s commented 1 year ago

The two don't differ much in API:

await db
    .insertInto('pet')
    .values({
        name: 'Catto',
        species: 'cat',
    })
    .onConflict((oc) => oc
        .column('name')
        .doUpdateSet({ species: 'hamster' })
    )
    .execute()

await db
    .insertInto('pet')
    .values({
        name: 'Catto',
        species: 'cat',
    })
    .onDuplicateKeyUpdate({ species: 'hamster' })

Here's a pseudo-prototype:

class InsertQueryBuilder<Table> {
    upsert(keyColumn?: PrimaryKey<Table>, newValues: Updateable<Table>) {
        if(this.db.dialect instanceof PostgresqlDialect) {
            return this.onConflict(oc => oc
                .column(keyColumn | this.table.primaryKey)
                .doUpdateSet(newValues)
            )
        } else if(this.db.dialect instanceof MysqlDialect) {
            return this.onDuplicateKeyUpdate(newValues)
        }
    }
}

Of course it's oversimplified, but is this feasible?

igalklebanov commented 1 year ago

Hey 👋

Kysely has a WYSIWYG design philosophy - trying to be as close as possible to writing SQL. Your proposal doesn't align with that, and cannot be part of a builder's API. Kysely is also dialect agnostic, so we'll never expose and allow branching based on dialect.

We do have a place for QoL helper functions, and shipped some dialect-specific JSON helpers in recent releases. This proposal is a good candidate for that, but we need to figure out if it can be solved for a wide enough range of cases and in a type-safe way.

koskimas commented 1 year ago

Let's not start building an ORM in the helpers either.

nc7s commented 1 year ago

I'd argue it's mere QoL rather than leaning towards ORM, but anyway.

koskimas commented 1 year ago

My problem with adding more and more ORM features in the helpers folder is that, even though they are written as "helpers" and meant as second class citizens, people will start to think of them as official parts of the library. They won't understand the distinction between the core and the helpers.

We will get all the problems of maintaining an ORM. People will find more and more holes in those leaky abstractions and we need to keep fixing them. So soon we'd just have an oddly organized crappy ORM 😄

Having said that, I don't really know where to draw the line with the helpers. My strong reaction to this issue was because:

  1. It took knex something like five years to build a half-decent upsert abstraction.
  2. I wrote an upsert functionality in objection and it was the biggest mistake and took an insane amount of work and thousands of lines of code to make it usable. It was also the number one issue generator.
nc7s commented 1 year ago

Alright, I'm fine with this "query builder and no more" mindset ;) This issue is a rather random thought, won't blame you for not considering it.