New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding upsert type capability #54
Comments
I agree, this would be a nice feature! |
👍 |
1 similar comment
👍 |
I'm importing some data from a CSV and there's a good chance that a few of the records overlap from the last import (i.e. last time was imported from Jan 1st to May 31st, this time importing from May 31st to Jun 18th). Fortunately the third party system assigns reliably unique ids. What's the best way to go about inserting the new records and updating the old? I haven't tried it yet, but I was thinking that it would be something like this: var ids = records.map(function (json) { return json.id })
;
Records.forge(ids).fetchAll().then(function () {
records.forEach(function (record) {
// now the existing records are loaded in the collection ?
Object.keys(record).forEach(function (key) {
Records.forge(record.id).set(key, record[key]);
});
});
Records.invokeThen('save').then(function () {
console.log('Records have been either inserted or updated');
});
}); |
Also, sometimes the thing I'm storing is stored by a determinate id value, such as a hash. In those cases I just want to add or replace the data. I don't always use SQL as traditional SQL. Often I use it as hybrid NoSQL with the benefit of clear relationship mapping and indexes. |
👍 |
Hi, Or can someone recommend examples, that show how to simulate this functionality for mysql? thx |
Right now I'm doing it with |
Postgres just implemented upsert support by the way 👍 https://news.ycombinator.com/item?id=9509870 Syntax is |
I was looking for a way to do a |
Any updates on this, especially with PostreSQL 9.5? |
I think one important question is whether or not to expose the same I realize that some of KnexJS library methods have distinctions regarding the return values in the context of different dialects (such as According to the documentation, the
While in PostgreSQL (http://www.postgresql.org/docs/9.5/static/sql-insert.html):
In this case, the return values can be changed with Thoughts? |
I monkey patched Client_PG to add the "onConflict" method for insert. Suppose we want to upsert github oauth credentials, we can write the query like this: const profile = {
access_token: "blah blah",
username: "foobar",
// ... etc
}
const oauth = {
uid: "13344398",
provider: "github",
created_at: new Date(),
updated_at: new Date(),
info: profile,
};
// todo: add a "timestamp" method
const insert = knex("oauths").insert(oauth).onConflict(["provider", "uid"],{
info: profile,
updated_at: new Date(),
});
console.log(insert.toString()) The array of column names specifies the uniquessness constraint.
See gist: https://gist.github.com/hayeah/1c8d642df5cfeabc2a5b for the monkey patch. This is a super hacky experiment... so don't exactly copy & paste the monkey patch into your production code : p Known problems:
Feedback? |
@hayeah I like your approach and it suits Postgres. I am going to try your monkey patch in a project to see if I can empirically detect any issues other than the ones you pointed out. |
Syntax Suggestion: Summary of the different implementations of upserts can be found at https://en.wikipedia.org/wiki/Merge_(SQL) |
I'm interested in having this capability too. Use case: building a system that is reliant on lots of outside data from an outside service; I periodically poll it for data that I save to a local MySQL db. Will probably be using knex.raw for now. |
Also interested, but in my use case would need to have it work in a way that isn't based on conflicts, as the columns don't always have 'unique' constraints - simply update entries matching the query if they exist, otherwise insert new rows. |
@haywirez I am curious as to why there are no unique constraints? Wouldn't you be exposed to race conditions? |
@hayeah I have a specific use case with time-windowed data, storing entries that have a value tied to a given day. Therefore I'm inserting and updating entries that have a "combined key" of a matching (day) timestamp, and two other IDs corresponding to PK's in other tables. Within a 24-hour window, I have to either insert them, or update them with the latest counts. |
This would be a great feature to have! |
Hi everyone who's ever commented here. I'm adding a PR Please label. Happy to take a PR adding this functionality, but I'd like to see a discussion of the desired API here first. PS.
I'm going to delete comments like this, if you want to add a +1 do so with the little emoji reaction thing. |
I have a bit of an issue with the array of column restraints as in @willfarrell and @hayeah's examples. Not sure if these examples can support
knex('table')
.where('id', '=', data.id)
.upsert(data)
knex('table')
.upsertQuery(knex => {
return knex('table')
.where('id', '=', data.id)
})
.upsertUpdate(knex => {
return knex('table')
.insert(data)
})
knex('table')
.where('id', '=', data.id)
.insert(data)
.upsert() // or .onConflictDoUpdate() I'm leaning most toward something like 3. Just to add in here's how mongodb does it. db.collection.update(
<query>,
<update>,
{
upsert: <boolean>,
multi: <boolean>,
writeConcern: <document>
}
) |
@reggi I believe my monkey patch is compatible with |
@reggi I don't see your point. |
@reggi The MongoDB example you provided reads "First try to UPDATE WHERE ... then do an INSERT if no document matches the query" whereas SQL UPSERT reads "INSERT INTO ... UPDATING in case a row with this primary key already exists". |
I would propose this API: knex.createTable('test')
.bigserial('id')
.varchar('unique').notNull().unique()
.varchar('whatever')
knex.table('test').insert(object, { upsert: ['unique'] })
I guess that would be the most simple and unambiguous solution homogeneous with the present API. |
As an aside, the fact that it doesn't run immediately is often useful. Sometimes you wanna pass the thing around, configuring it, before executing. There's also situations where you can do stuff like... const medicalBuildings = knex.select('building_id').from('buildings').where({type: 'medical'})
const medicalWorkers = knex.select().from('workers').whereIn('building', medicalBuildings) (super contrived example but let's run with it) I don't actually want to run that first statement - it's just part of my 2nd one. |
Not to mention that if all query builders would execute on creation the builder pattern queries would trigger before building is done. It would not work at all without having some terminator method (that executes the query). |
@elhigu I mean... I guess you could just always run it on the next tick, right? I'm not suggesting that would by any means be a good idea but how many queries are actually created and executed on different ticks? |
@timhuff I hadn't thought about that. Yeah I think that would be possible too. I find case quite common where one starts building query, then fetch some async data and keep on building more. I don't do that very often though. |
@lukewlms that ’execute()’ -like method is called ’.then()’ you can call always it when you like to execute query and get promise. It is just how ’thenable’ works and it is explained in promise spec. It is one important and widely used concept in javascript when dealing with promises and async/await (which are pretty much just glorified shortcuts for Promise.resolve and .then). Also if you are executing queries without handling results you are looking for problems like app crashing. |
Actually its better to just follow this PR about the upsert feature implementation #2197 it has already API designed how it should work. In this thread is not really any useful information that is not already mentioned in comments of that PR. If needed (PR is closed and never completed) lets open new issue for this one with additional API description. |
@elhigu Thanks for the heads up! I was unaware of that thread. Good to hear we're making progress on an upsert coming to the API. Looks like 6 months ago it failed 1 of the 802 tests and so it never passed travis-ci. Is that 1 failing test case the only thing keeping this from becoming a part of the knex API? |
@timhuff there was only initial implementation done, it must be completely rewritten. Most important part of that PR is the common API design, which can be supported by most of the dialects. So the feature comes when someone just decides to implement that API. If no-one else does that and some day I have some extra time or need it badly, I'll do it myself. That is one of the most important feature I would like knex to get (in addition to joins in updates). |
@elhigu Thanks for filling me in. I'll have to read up on the progress here when I get a little more time. |
I'm not sure if this helps anyone or if i'm just a noob, but for the solution from @timhuff I had to wrap my constraint in quotes because i was getting a query syntax error. const contraint = '("a", "b")'
|
Removed some unrelated discussions (about how promises/thenables work). |
Did this get added ? |
No. There is feature request and spec in #3186 |
Brought up by @adamscybot in bookshelf/bookshelf#55 - this could be a nice feature to add.
The text was updated successfully, but these errors were encountered: