Skip to content
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

Upsert #50

Open
calebkleveter opened this issue Jul 10, 2019 · 9 comments
Open

Upsert #50

calebkleveter opened this issue Jul 10, 2019 · 9 comments
Labels
enhancement New feature or request
Projects

Comments

@calebkleveter
Copy link
Member

An upsert is when you run a query that checks for a the existence of an entity matching a given predicate. If a match is found, then the rows found will be updated with the data passed in. If a match is not found, a new row with the given data is created. I think having builtin support for this would be nice:

User.query(on: database).filter(\.name == "Caleb Kleveter").upsert(update: updateRow, insert: insertRow)

From what I have been able to find, all SQL databases support this sort of operation. MongoDB appears to support it also.

@calebkleveter calebkleveter added the enhancement New feature or request label Jul 10, 2019
@calebkleveter calebkleveter added this to To Do in Vapor 4 via automation Jul 10, 2019
@rnantes
Copy link

rnantes commented Oct 1, 2019

+1 on this useful feature. The Rust Diesel ORM has a upsert functionality when inserting/updating multiple rows. If there is a conflict on row it allows custom functionality on for that specific row using the on_conflict() function. This is similar to how a raw postgres query works. Here is are postgreSQL and Diesel examples. Might be a good starting point.

@marc-medley
Copy link

Related fluent-sqlite-driver issue 23: 'save(on:)' with given UUID fails to save to database

The Vapor document seemed to indicate that save(on:) would behave something like an classic UPDATE OR INSERT SQL command. However, save(on:) is not an UPSERT and could fail (at that time) without any error notification.

Saves the model, calling either create(...) or update(...) …

Once the upsert is implemented in fluent-kit and the associated drivers (e.g. fluent-sqlite-driver), then perhaps then it would be time to deprecate save(on:) … as mentioned by @tanner0101 ?

@tanner0101
Copy link
Member

save simply calls either create or update depending on whether the model was fetched from the database. There's no upsert support yet. It would be a nice feature to have, though.

@marc-medley
Copy link

marc-medley commented Mar 6, 2020

Hmmm..., UPSERT (or MERGE) would be "simply a call to either create or update depending on whether" the id key exists in the SQL database:

IF EXISTS (key …) /* ** e.g. ID in the SQL database ** */
  UPDATE command  /* update */
ELSE
  INSERT command  /* create */

... seems that a variant of save which checks for key exists in database instead of model fetch source might provide an upsert capability.

And BTW, a MERGE (aka UPSERT) capabitility seems to be more than a "would be a nice feature " ... MERGE "was officially introduced in the SQL:2003 standard, and expanded in the SQL:2008 standard." ... it's a standard SQL capability which is missing in fluent-kit.

@tanner0101
Copy link
Member

The SQL you pasted checks the db and decides whether to update or insert in a single query. save() is simply checking whether your code fetched this model in from the database or initialized it manually. It assumes if you fetched from the db, then save should be an update. If this is a new model you just initialized, then save should be a create.

The fact that this is an old SQL capability is not very helpful support for inclusion in Fluent. If you have specific examples of what problems you're trying to solve, ideas about how this could relate to the Model API (as @calebkleveter has done), or references to other ORMs similar to Fluent that include support (as @rnantes has done), that is much more helpful. Building this feature in a meaningful, easy-to-use way that is consistent with how the rest of Fluent works takes more effort than just adding in the SQL keywords.

@tanner0101
Copy link
Member

tanner0101 commented Mar 6, 2020

Here's an initial idea for API based on what @calebkleveter and @rnantes shared:

let uuid = UUID()

// Insert user with name Foo
User(id: uuid, name: "Foo")
    .create(on: db)

// Updates name to Bar
// Automatically uses same input values for the update
User(id: uuid, name: "Bar")
    .create(orUpdate: .conflict(\.$id), on: db)

// Updates name to Qux
// User decides the input values in case of conflict
User(id: uuid, name: "Baz")
    .create(orUpdate: .conflict(\.$id), on: db) {
        $0.set(\.$name, to: "Qux")
    }

// Updates name to Quuz
// The model API will use this query builder API internally
User.query(on: db)
    .set(\.$id, to: uuid)
    .set(\.$name, to: "Quuz")
    .create(orUpdate: .conflict(\.$id), on: db)

I also tried with .on(conflict:) but then the method uses "on" twice which might be a little confusing.

user.create(orUpdate: .on(conflict: \.$id), on: db)

@tanner0101
Copy link
Member

Note from vapor/fluent-mysql-driver#153 that timestamps should be updated correctly during upsert.

@hernangonzalez
Copy link

Hi, is this still planned for inclusion?

@gwynne
Copy link
Member

gwynne commented Nov 21, 2021

This was implemented at the SQLKit layer in vapor/sql-kit#90, but support in the FluentKit layer has yet to be added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Vapor 4
  
Backlog
Development

No branches or pull requests

6 participants