kysely-org / kysely

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

[SQLite Dialect] Properly translate .ignore() #916

Open BitPhinix opened 5 months ago

BitPhinix commented 5 months ago

Currently .ignore() using the SQLite dialect gets translated into INSERT IGNORE INTO ....

Screenshot 2024-03-18 at 14 56 02

Which doesn't match the SQLite expected INSERT OR IGNORE INTO (see here)

Screenshot 2024-03-18 at 14 57 55
igalklebanov commented 5 months ago

Hey 👋

Nice catch!

@koskimas I wonder what the solution should be.

The low hangin' fruit is just adding an override in SqliteQueryCompiler that appends an or. The purist in me screams "But it's not WhatYouSeeIsWhatYouGet!!!".

WYSIWYG alternatives are:

db.insertInto('person').or('abort')
db.insertInto('person').or('fail')
db.insertInto('person').or('ignore')
db.insertInto('person').or('replace')
db.insertInto('person').or('rollback')
db.insertInto('person').orAbort()
db.insertInto('person').orFail()
db.insertInto('person').orIgnore()
db.insertInto('person').orReplace()
db.insertInto('person').orRollback()

But having or('ignore') or orIgnore() AND ignore() might be confusing.

koskimas commented 5 months ago

It's not wysiwyg but it's so close that I could live with it.

But supporting all those other or thingies is a good idea, and in that case the separate method(s) is better. I'd go with separate methods.

We might still override ignore to work as well to avoid mistakes?

igalklebanov commented 5 months ago

We might still override ignore to work as well to avoid mistakes?

I definitely think that's the nice thing to do.

joaobzrr commented 1 month ago

Any update on this?