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

Add upserts #158

Closed
countermeasure opened this issue Jun 2, 2015 · 26 comments
Closed

Add upserts #158

countermeasure opened this issue Jun 2, 2015 · 26 comments
Labels

Comments

@countermeasure
Copy link

Upserts would be nice, and it sounds from #79 like they're on the way.

I'm not good enough with JS to attempt a patch, but I can share a snippet I'm using to do upserts if anyone wants to use it until LokiJS has them baked in.

I can't guarantee how fast or robust it is, but it's working for me.

Note that it requires lodash.

/**
 * Performs an upsert.
 * This means performing an update if the record exists, or performing an
 * insert if it doesn't.
 * LokiJS (as at version 1.2.5) lacks this function.
 * TODO: Remove this function when LokiJS has built-in support for upserts.
 * @param {object} collection - The target DB collection.
 * @param {string} idField - The field which contains the record's unique ID.
 * @param {object} record - The record to be upserted.
 * @depends lodash
 */
function upsert(collection, idField, record) {
  var query = {};
  query[idField] = record[idField];
  var existingRecord = collection.findOne(query);
  if (existingRecord) {
    // The record exists. Do an update.
    var updatedRecord = existingRecord;
    // Only update the fields contained in `record`. All fields not contained
    // in `record` remain unchanged.
    _.forEach(record, function(value, key){
      updatedRecord[key] = value;
    });
    collection.update(updatedRecord);
  } else {
    // The record doesn't exist. Do an insert.
    collection.insert(record);
  }
}
@techfort
Copy link
Owner

techfort commented Jun 2, 2015

thanks for this! Upserts are in fact underway for 1.4 - mechanism is very similar to the one you outlined so it should be to your satisfaction.

@countermeasure
Copy link
Author

Thanks! 👍

@techfort
Copy link
Owner

techfort commented Jun 3, 2015

Leaving this open till its implemented. Cheers

@retorquere
Copy link

@techfort this is actually what I wrote the byExample/findObject code for (pseudocode, as I spend my time in coffeescript):

entry = {itemID: 1, exportCharset: 'UTF-8'}
cached = collection.findObject(entry)
if (cached) {
    cached.citekey = citekey
    collection.update(cached)
} else {
    entry.citekey = citekey
    collection.insert(entry)
}
return (cached || entry)

@techfort
Copy link
Owner

techfort commented Jun 7, 2015

@retorquere that looks like perfectly sound logic, and the final code will probably be just a port to js of it. Thanks!

@retorquere
Copy link

If update will return its subject at some point, it would become even shorter:

entry = {itemID: 1, exportCharset: 'UTF-8'}
cached = collection.findObject(entry)
if (cached) {
    cached.citekey = citekey
    return collection.update(cached)
} else {
    entry.citekey = citekey
    return collection.insert(entry)
}

for upsert, you'd probably want to do something like (actual coffeescript, just easier for me to write)

upsert = (entry) ->
  # assuming the existence of findIndexedObject, which does
  # what findObject does, but only for properties declared
  # unique together in some way
  cached = @findIndexedObject(entry)

  return @insert(entry) unless cached

  for own key, value of entry
    continue if key in ['meta', '$loki']
    # this will be a no-op for the search values, but hey
    cached[key] = value
  return @update(cached)

without unique constraints, and doesn't cause updates unless actual changes are made

merge = (objects...) ->
  res = objects.shift()
  for o in objects
    for own k, v of o
      continue if k in ['meta', '$loki']
      res[k] = v
  return res

upsert = (key, data) ->
  # key is an object with the fields you want to use for search-by-example
  # data is all the other settable properties
  # clunky, but avoids having to add unique constraints to LokiJS
  cached = @findObject(key)

  return @insert(@merge(key, data)) unless cached

  changes = false
  for own k, v of data
    continue if k in ['meta', '$loki']
    changes ||= cached[k] != v
    cached[k] = v
  return cached unless changes
  return @update(cached)

@techfort
Copy link
Owner

techfort commented Jun 8, 2015

thanks a lot for this @retorquere !

@jvkassi
Copy link

jvkassi commented Nov 29, 2015

+1, this will be very handy to have :)
Awesome work by the way 😍

@arcanis
Copy link

arcanis commented May 28, 2016

Worth nothing that the upsert(findBy, document) is what Sequelize uses for its upserts (first argument is a query object, second one is the properties that have to be set).

@ghost
Copy link

ghost commented Aug 10, 2016

v1.4.1 has been released on 5 July 2016, but it seems to lack upserts. Any updates on this?

@kubal5003
Copy link

By any chance is there some progress with this?

@nuborian
Copy link

Any Updates on this?

@Ruuudi
Copy link

Ruuudi commented Jun 20, 2017

Any Updates? We really need this functionality.

@Viatorus
Copy link
Collaborator

I will track it for LokiJS2.

@jlsjonas
Copy link

Hi, is there any ETA for this? @Viatorus
Seems like the code has been laying in this issue for over 2 years and was supposed to be implemented ~2 years ago? 😅

@Viatorus
Copy link
Collaborator

Yes, I tracked it for LokiJS2 but there are still some other work to do. But this could be one of the first features after the first release of v2.

@msolters
Copy link

any news?

@dimascrocco
Copy link

Is this feature available for 2017 Christmas? Or the Nordic gods don't like it?

@obeliskos
Copy link
Collaborator

obeliskos commented Nov 29, 2017

Does anyone want to create a pr for this? We are in a maintenance mode for this branch but this feature should be isolated and somewhat limited in scope. (Typically LokiJS2 will be where most new feature requests go)

If anyone wants to have a go at it on this repo, please use ecma 5 standards and provide a unit test.

I would expect this could be done with :

  • collection level upsert method only
  • 'idField' (string) and 'doc' (object) params should suffice
  • within method, see if idField name exists in 'this.uniqueNames' array (unique index list).
  • if it does, look it up with collection.by(), otherwise perform a collection.findOne()
  • if the document was found by either method, copy properties from doc param to found document, similar to this. You will want to -not- copy the property if that key (property) name is '$loki' or 'meta'. You would then call update() with that document.
  • if the document was not found, just pass doc param to insert().

I think that would do it, if anyone sees I missed anything feel free to point that out.

@retorquere
Copy link

Is there a LokiJS2 roadmap btw? Will the serialization formats remain the same (thinking migration issues here)?

@Viatorus
Copy link
Collaborator

Viatorus commented Nov 30, 2017

@obeliskos This should probably do it. But the idField is obsolete or not enough. What happen if someone has multiple unique constraints?

The function upsert have to check if either the 'doc' fulfills all unique constraints with an existing document in the collection, if not try to insert (maybe violates the constraint and throws).

@retorquere No, LokiJS2 is not compatible right away. But an extra database-migration-tool script could be possible.

You find the beta of LokiJS2 here: https://github.com/LokiJS-Forge/LokiJS2

@obeliskos
Copy link
Collaborator

Sorry I didn't read or respond a month ago... all valid points. All inserts/updates should go through collection.insert() and collection.update() to ensure any changes affecting indices are processed in one place. Detection and utilization of unique indices (via by()) would be for ensuring optimal lookup of individual doc(s) and if passed properties modify indexed fields, the call to update() should reintegrate within indices. I think that covers danger areas but it would need unit tests to fully define and be sure.

@stale
Copy link

stale bot commented Jul 13, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jul 13, 2018
@stale stale bot closed this as completed Jul 20, 2018
@tonyxiao
Copy link

What's the recommended way to do upsert now with the latest loki version? 1.5.7?

@xkem
Copy link

xkem commented Jun 22, 2021

What's the recommended way to do upsert now with the latest loki version?

@SteveChurch
Copy link

Any update on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests