FindAndModify + upsert

[Globegitter]

As mentioned by @dmarcelino in #939 a PR against the 0.11 branch.

I know there is the current discussion going on about findOrCreateEach (
https://github.com/balderdashy/waterline/issues/931) so it might not have been the wisest choice to add a findAndModifyEach but I needed a findAndModify functionality (see http://docs.mongodb.org/manual/reference/method/db.collection.findAndModify/) and it seemed like a good solution to put it such a way that other people can use it as well.

Usage is Model.findAndModify(search critieras, values, [options, cb]);

where-as default options look like that:

{
  upsert: false, // on true automatically inserts the values if nothing is found
  new: true, // if false returns the model before it was updated/created
  mergeArrays: false // quite a special case that I needed and I am still testing in my app. When any of the values given contain an array it is merged to the found object rather than just replaced.
}

I added passing unit tests as well as adapter-tests, see balderdashy/waterline-adapter-tests#47
There is something strange going on, even though the findAndModify passes the unit tests and works in my app (via npm i git+https://github.com/Globegitter/waterline.git#findAndModify) the adapter-tests do not pass and throw the same error throughout:

9) Semantic Interface .findAndModfiy() should return a model instance:
     TypeError: undefined is not a function
      at Context.<anonymous> (Git/waterline/node_modules/waterline-adapter-tests/interfaces/semantic/findAndModify.test.js:73:20)
      at Test.Runnable.run (Git/waterline/node_modules/waterline-adapter-tests/node_modules/mocha/lib/runnable.js:194:15)
      at Runner.runTest (Git/waterline/node_modules/waterline-adapter-tests/node_modules/mocha/lib/runner.js:355:10)
      at Git/waterline/node_modules/waterline-adapter-tests/node_modules/mocha/lib/runner.js:401:12
      at next (Git/waterline/node_modules/waterline-adapter-tests/node_modules/mocha/lib/runner.js:281:14)
      at Git/waterline/node_modules/waterline-adapter-tests/node_modules/mocha/lib/runner.js:290:7
      at next (Git/waterline/node_modules/waterline-adapter-tests/node_modules/mocha/lib/runner.js:234:23)
      at Immediate._onImmediate (Git/waterline/node_modules/waterline-adapter-tests/node_modules/mocha/lib/runner.js:258:5)
      at processImmediate [as _immediateCallback] (timers.js:361:17)

I have no idea why this happens or how to fix this. So apart from if this actually gets merged, anyone got any idea of why that could be?

Also it does not work completely in my app via sails-postgresql, I seem to be getting this error. I am not sure if that is related to my code or to sails-postgresql.

db_1     | ERROR:  syntax error at or near "LIMIT" at character 140
db_1     | STATEMENT:  UPDATE "subject" AS "subject" SET "researchIds" = $1, "text" = $2, "createdAt" = $3, "updatedAt" = $4  WHERE LOWER("subject"."text") = $5  LIMIT 1 RETURNING *

[Globegitter]

@dmarcelino Just seeing this now has all the commits that are missing on this branch - hope that's not an issue.

In any case, any idea on what the issues mentioned above could be?

[dmarcelino]

We branched 0.11 some time ago and didn't keep it in sync with master, don't worry.

No, nothing comes to mind...

[anasyb]

mergeArrays is interesting... what's the criteria for merging, is it simple concat or function based? Also does it apply just on arrays or on objects? Like this could be so powerful if one wants it to be... my mouth is watering though I don't have at the moment a use-case to back it :D
Maybe an operation transform (OT) engine!

[Globegitter]

@anasyb Right now it really is the most basic implementation it could be (and that is actually all I need for my use-case). It just iterates through all the properties in the returned results, checks if the according element is an array. If it is it checks if it exists in the given values and if it is an array as well. If not it ignores it and if it is, it just concatenates these two and updates the found record with the new array (and everything else you originally gave it)

If the find returns multiple results it just concatenates all of them to one array and will update all the results with that one 'big' array. I am mostly assuming here that you are trying to find uniquely indexed elements.

Also now just writing about it makes me realise that I can optimise that implementation - thanks :)

And yeah up for any improvements in any way ;)

Also my use case is, I am querying an external API and generally keeping track of their ids. For categories it turns out that that the same category can have different ids. I want that uniquely in my database but still keep track of all the original ids.

[Globegitter]

Thanks for actually getting me to think about it a bit more @anasyb ;) Now it is looping over the given values properties since that will be a subset of result and I could simplify some if checks - feels like a win :)

[Globegitter]

If anyone needs this functionality it has now been tested (with postgresql) in my running app and once #942 is merged should work using any of the adapters.

[Globegitter]

Ok something with rebasing to latest master seemed to go a bit wrong, but should all be looking good now and working if anyone needs it.

[tjwebb]

Maybe I'm being pedantic, but we're introducing a new term to the waterline lingo: modify. Does this mean something distinct from the current update functionality?

[devinivy]

The only distinction I can find is the mergeArray option (which defaults off), which combines (via a union) array values from a single found record (given the criteria) with array values in the values object passed to the findAndModify method. Hmmm, that's hard to say very well. Basically, this method can modify existing array values rather than overwrite them.

@Globegitter how do you justify the findOne used there, when many records might be returned given the criteria? Also, analogously to array data, would you consider allowing for json data to be merged as well?

[Globegitter]

@tjwebb @devinivy The idea behind this query was to get http://docs.mongodb.org/manual/reference/method/db.collection.findAndModify/, which they have in addition to their update query.

But my biggest reason to create this function was to add support for upserting (https://wiki.postgresql.org/wiki/UPSERT), so basically either update the current or create it. I guess following waterline naming (rather than mongo), this would be updateOrCreate which I was thinking of as well. I guess you have a point though - when I implemented the method I looked at the mongo docs and got inspired by that rather than thinking how waterline has it's defaults etc. That probably was a bit of a mistake, but happy to change things around.

@devinivy In regards to the findOne I was thinking that as well, but I worked under a deadline and just copied the logic from findOrCreate assuming that whoever implemented it has thought about why to use findOne. But you are right it could be a wrong assumption and now with a bit more time on hand is worth investigating.

And yes I was thinking about the possibility to merge json and it would make very much sense to implement it, I just didn't have the use or test-case at hand to do so.

[dmarcelino]

For reference, this will fix #790.

[dmarcelino]

I'm working on something today that could really use upsert / updateOrCreate 😭

PS: we are getting closer to wrapping up 0.10, so hopefully we'll be able to care of this soon!

[Glavin001]

+1 Would love to have upsert.

[LeonardoGentile]

Any news on this?

[didil]

+1 on upsert

[hjalet]

+1 on upsert. Will this be added to a released version?

[fed135]

+1 on upsert

[particlebanana]

Ok I get the upsert part of the feature. Upsert is on the roadmap and just needs to be completely specced out as to both the API and implementation details.

I'm not sure I understand how findAndModify is any different than update though?

[hjalet]

@particlebanana findAndModify is not that different from a Waterline update, since the update in Waterline returns the updated models. Traditional SQL updates in for instance SQL Server does not return the updated records.
The difference is that if you compare it to findAndModify in MongoDB, MongoDB has the option to either return the updated documents or the original documents.

[randallmeeker]

closed w/o comment?

@particlebanana did you get bitten by @sailsbot ?