kysely-org / kysely

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

Feature request - implement ON CONFLICT WHERE #45

Closed sampbarrow closed 2 years ago

sampbarrow commented 2 years ago

Sqlite supports this but the onConflictUpdate method does not allow me to add a where clause.

https://www.sqlite.org/lang_UPSERT.html

upsert-clause

koskimas commented 2 years ago

Yep, I'll add support for this. Postgres also supports the other where clause (before DO). I'll need to break/improve the onConflict methods to get this done. Currently there's two methods onConflictDoNothing and onConflictUpdate. I don't want to keep adding arguments to those since everything else in Kysely is builder-based.

I'm thinking this kind of interface would be handy and in line with Kysely's API:

db.insertInto('person')
  .values(person)
  .onConflict((oc) => oc
    .column('name')
    .where(...) // You can chain multiple of these.
    .doNothing()
  )
db.insertInto('person')
  .values(person)
  .onConflict((oc) => oc
    .constraint('name_unique_constraint')
    .where(...)
    .doNothing()
  )
db.insertInto('person')
  .values(person)
  .onConflict((oc) => oc
    .columns(['name', 'birth_date'])
    .where(...)
    .doUpdateSet({ ... })
    .where(...)
  )
db.insertInto('person')
  .values(person)
  .onConflict((oc) => oc
    .columns(['name', 'birth_date'])
    .doUpdateSet({ ... })
    .where(...) // You can chain multiple of these.
  )
koskimas commented 2 years ago

Actually it seems that postgres supports the exact same syntax as sqlite.

koskimas commented 2 years ago

This is now implemented in 0.14.0. All dialects so far that support on conflict (SQLite and PostgreSQL) support the same API. I didn't need to add any new dialect-specific methods.

sampbarrow commented 2 years ago

Just tested it, looks great! Thanks!

sampbarrow commented 2 years ago

Hey - since this adds that virtual "excluded" table to the query - is it possible to do this while keeping strong typing?

VALUES
    (2, 'robert'),
    (5, 'sheila'),
    (6, 'flora')
ON CONFLICT (id) DO UPDATE
SET name = EXCLUDED.name;

I checked the docs for the regular update query builder and didn't see anything about using refs in the values of the MutationObject there either.

koskimas commented 2 years ago

@midwesterntechnology Currently there's no way to do that in a type-safe way. We could add a ref method to the expression builder so that you could do this:

db.insertInto('person')
  .values(person)
  .onConflict((oc) => oc
    .column('id')
    .doUpdateSet({
      name: (eb) => eb.ref('excluded.name')
    })
  )

but I think that's too much ceremony for such a simple thing.

This wouldn't work:

db.insertInto('person')
  .values(person)
  .onConflict((oc) => oc
    .column('id')
    .doUpdateSet({
      name: db.ref('excluded.name')
    })
  )

because db isn't aware of the context and there is no excluded table as far as it's concerned.

This isn't possible either:

db.insertInto('person')
  .values(person)
  .onConflict((oc) => oc
    .column('id')
    .doUpdateSet({
      name: 'excluded.name'
    })
  )

because in theory, somebody could want to set the value to a string "excluded.something" and we can't know which one the user means.

So the only option really is to add a ref method to the expression builder passed to the callback.

koskimas commented 2 years ago

We could add a generic ref function like this

db.insertInto('person')
  .values(person)
  .onConflict((oc) => oc
    .column('id')
    .doUpdateSet({
      name: ref('excluded.name')
    })
  )

and we could make it type-safe BUT without autocompletion. You'd just get a massive obscure typescript error when the value doesn't match any of the visible columns.

sampbarrow commented 2 years ago

@midwesterntechnology Currently there's no way to do that in a type-safe way. We could add a ref method to the expression builder so that you could do this:

db.insertInto('person')
  .values(person)
  .onConflict((oc) => oc
    .column('id')
    .doUpdateSet({
      name: (eb) => eb.ref('excluded.name')
    })
  )

but I think that's too much ceremony for such a simple thing. This wouldn't work

db.insertInto('person')
  .values(person)
  .onConflict((oc) => oc
    .column('id')
    .doUpdateSet({
      name: db.ref('excluded.name')
    })
  )

because db isn't aware of the context and there is no excluded table as far as it's concerned.

This isn't possible either:

db.insertInto('person')
  .values(person)
  .onConflict((oc) => oc
    .column('id')
    .doUpdateSet({
      name: 'excluded.name'
    })
  )

because in theory, somebody could want to set the value to a string "excluded.something" and we can't know which the user means.

So the only option really is to add a ref method to the expression builder passed to the callback.

So this, correct:

>     .doUpdateSet({
>       name: (eb) => eb.ref('excluded.name')
>     })

That makes sense to me. Or maybe a ref method on the oc object? I believe in sqlite anything within the DO UPDATE SET clause is aware of the "excluded" row.

Seems like adding ref to the expression builder everywhere would also come in handy for regular update queries as well, I think? If you wanted to do an update based on a join for example.

koskimas commented 2 years ago

That's true. We could add it to the oc object, but would people find it?

I was thinking about a more general feature. ExpressionBuilder is always passed to callbacks throughout Kysely. Whenever you are creating a subquery or a raw expression in select, where etc. using a callback, it's always ExpressionBuilder that's passed there. If we added a ref method to ExpressionBuilder people would get used to it being there and it could be used anywhere.

update:

Seems like adding ref to the expression builder everywhere would also come in handy for regular update queries as well, I think? If you wanted to do an update based on a join for example.

Exactly.

sampbarrow commented 2 years ago

I'm all for that if you're willing to add it, seems like something that would definitely fill a gap.

koskimas commented 2 years ago

I've thought about this before. The only reason it's not added yet is that it only helps when you want to assign some other column directly. As soon as you want to do something more complex (for example: count = excluded.count + 1) you need to once again use raw. We would of course allow ref to be passed to raw as well, but the code quickly becomes so messy that it's better to just write less type-safe, but readable code.

.doUpdateSet({
  count: (eb) => eb.raw('? + 1', [eb.ref('excluded.count')])
})

vs.

.doUpdateSet({
  count: db.raw('excluded.count + 1')
})
sampbarrow commented 2 years ago

I think for basic updates you'll never want to assign columns directly like that, but for ON CONFLICT DO UPDATE at least, in most cases you'd be doing a direct assignment with no functions. An update that includes a join could have direct assignments as well. I'm using db.raw right now for mine, but in this case the type safe option would be no less readable.

.doUpdateSet({
  count: (eb) => eb.raw('? + 1', [eb.ref('excluded.count')])
})

I'd probably do it this way myself over the other way but I'm kind of crazy about type safety so YMMV.

koskimas commented 2 years ago

Could you open a new issue for this feature? You can just copy paste some examples from our discussion to it. I'll check what it would mean to add the ref method.

sampbarrow commented 2 years ago

Done.