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

SERVER-991: Added support for $trim #181

Closed
wants to merge 2 commits into from
Closed

Conversation

boivie
Copy link

@boivie boivie commented Feb 25, 2012

The $trim operator will limit the length of the arrays to which
it is applied. A positive number indicates how many items to keep
starting counting from the beginning of the array whereas a negative
number keeps items from the end of the array.

array: [1 2 3 4 5 6 7 8]
trim(5) -> keep 1 2 3 4 5, drop 6 7 8 -> [1, 2, 3, 4, 5]
trim(-5) -> drop 1 2 3, keep 4 5 6 7 8 -> [4, 5, 6, 7, 8]

$trim is special as it is allowed (and required) to reference the same
field as another update modifier. It can not be used by itself, but
the same effect can be used by doing a $pushAll: [](pushing an empty
list) at the same time as using $trim.

$trim is always performed after updating the field with any other
modifier.

Test cases have been added.

The $trim operator will limit the length of the arrays to which
it is applied. A positive number indicates how many items to keep
starting counting from the beginning of the array whereas a negative
number keeps items from the end of the array.

  array: [1 2 3 4 5 6 7 8]
  trim(5)  -> keep 1 2 3 4 5, drop 6 7 8 -> [1, 2, 3, 4, 5]
  trim(-5) -> drop 1 2 3, keep 4 5 6 7 8 -> [4, 5, 6, 7, 8]

$trim is special as it is allowed (and required) to reference the same
field as another update modifier. It can not be used by itself, but
the same effect can be used by doing a $pushAll: [] (pushing an empty
list) at the same time as using $trim.

$trim is always performed after updating the field with any other
modifier.

Test cases have been added.
@cwestin
Copy link
Contributor

cwestin commented Feb 25, 2012

Trim is often used for a function to remove leading or trailing whitespace from strings. The functionality described here is more often known as slice. Perhaps that would be a better name, in order to be more consistent with other well-known library functions in various languages.

@boivie
Copy link
Author

boivie commented Feb 25, 2012

Thanks for your comment. I chose 'trim' as it's used in e.g. redis (http://redis.io/commands/ltrim) but renaming it is simple.

@cwestin
Copy link
Contributor

cwestin commented Feb 25, 2012

@cwestin
Copy link
Contributor

cwestin commented Feb 25, 2012

Silly me, I should have searched mongodb.org first. What you want is already there: http://www.mongodb.org/display/DOCS/Retrieving+a+Subset+of+Fields#RetrievingaSubsetofFields-RetrievingaSubrangeofArrayElements . Apparently that has been there since 1.6: http://www.mongodb.org/display/DOCS/What%27s+New+by+Version .

@boivie
Copy link
Author

boivie commented Feb 25, 2012

Cool - then $slice is a good name. The current $slice (as you link to) is only for retrieving fields and not for updating them, right?

@cwestin
Copy link
Contributor

cwestin commented Feb 25, 2012

I couldn't find any references for updating. If that's included in what you've done, some portion of that is still relevant after renaming.

@colinmollenhour
Copy link

Having $slice/$trim supported for updates as supported in this patch would be great. It would fix SERVER-991 which has 103 votes.

Redis calls it 'trim', but 'slice' is more commonly used.

This is a continuation of SERVER-991
@boivie
Copy link
Author

boivie commented Feb 26, 2012

The 10gen crew can happily squash these two commits as one if they wish.

@connois
Copy link

connois commented Feb 27, 2012

We have been waiting for a long time for something like this for solving SERVER-991. What release version can we expect on this one ? Thanks guys. Looks great !

@erh
Copy link
Contributor

erh commented Mar 25, 2012

I'm not sure $slice/$trim makes 991 go away.
What we really want to add is a $pushToFixedSize or something with those exact semantics so things can be done in a single update so the size is never above X.
With this, it would bounce between X and X + Y where Y could be as high as the number of threads updating the array.

@boivie
Copy link
Author

boivie commented Mar 25, 2012

@erh: Please have a look again. The update should be atomic as the "push" and "trim" is (read: should and must) be present in the same update operation.

To achieve this, I had to relax the rule that two update modifiers are not allowed to touch the same field. If I wouldn't have done that, you would be correct.

@boivie
Copy link
Author

boivie commented Mar 25, 2012

@erh: I think it would be better to have a look at the test cases I have written. The commit message isn't very good - I'll certainly agree to that.

@erh
Copy link
Contributor

erh commented Mar 25, 2012

Relaxing the rule that two modifiers can't touch the same field is not something we want to do right now.
It opens up a whole bunch of potential ordering semantic issues, etc...
Maybe in the future - but definitely not right now.

@boivie
Copy link
Author

boivie commented Mar 25, 2012

@erh: Sorry, I meant that I have only relaxed it for $trim (aka $slice), which by (my) definition states that it will always be evaluated after any other modifier. So it is backwards compatible.

@erh
Copy link
Contributor

erh commented Mar 25, 2012

Right - I saw, but then its a special case.
We'd really want things to be consistent in this regards.

i.e. there isn't a theoretical reasons why you can't do $addToSet and $pull on the same element, but the ordering is very important.

For $trim $push order is also important as it determines if you get X or X + 1

So you either have to say $tirm always is after $push, which may or may not be what people want.
Or say the order in the doc matters, which is doesn't today.

@erh
Copy link
Contributor

erh commented Mar 25, 2012

$trim does make sense independently anyway, but that should be a different jira.
It should also support trimming from front or back of the array.

@boivie
Copy link
Author

boivie commented Mar 25, 2012

@erh: $trim does support trimming from front or back by supplying it with a positive or negative length.

By the definition that $trim is a sort of "post-update" modifier, it should be obvious what happens. They can still get the results they want by either providing a positive length (keep the first X elements) or a negative length (keep the last X elements).

Yes, having it independently would be a good thing - I agree. Today you can simulate it by using {$pushAll: [], $trim: X}, but it's sort of a kludge that can be removed at any time while still not breaking any promises or backwards compatibility.

Documentation seems to be a key here. But I personally like the idea of having a separate operation, instead of creating "fixed-size"-operator copies (push, pushAll, addToSet etc would have to be copied many times). I don't want to take credit for that idea though - @colinmollenhour wrote about it in the jira.

@connois
Copy link

connois commented Mar 25, 2012

I'm sad to see reluctance to commit this. There are hundred of possible uses for this. How many time we need to store "the last n things" or "las n unique things" of something in a dev... Las 10 unique ips, the las 10 urls, last 10 user sessions. Today this require 2 write which is not very efficient.

@erh, I understand this principle of "two modifiers can't touch the same field" is important, but it is like a special case, and isn't it the also the whole purpose of this product ? To evolve with such good improvements requests from the developer community ? This is a really good one !

Could you still think about it before making up your mind and see how many posibilities that could open up ? @boivie already wrote the code and a lot of us are already mouth watered by it. This would be a waste not to commit it.

And if this is that critical not to modify the same field with 2 modifiers, mayne we could have some alternative in one single modifier ?

@erh
Copy link
Contributor

erh commented Mar 25, 2012

The feature makes sense, we just need to make sure the syntax/semantics are correct.

If you look at https://jira.mongodb.org/browse/SERVER-991 there is a version/api that fits in correctly with the rest of the api and that's the direction we're going to go.

@colinmollenhour
Copy link

The problem with the syntax originally proposed is that if you want to do multiple push and trim operations in one update you can't choose different lengths for different keys. I just proposed a new syntax on SERVER-991 that uses one operator with a 2-tuple (value, length) as the value for each key.

db.user.update({name: 'ibwhite'},{
  $pushTrim: {last_10_urls: ['/category/apple', -10]},
  $addToSetTrim: {last_5_unique_urls: ['/category/apple', -5]}
});

@kangas
Copy link
Contributor

kangas commented Jul 16, 2013

Closing this old pull request since SERVER-991 shipped in 2.3.2 and ticket was closed in March 2013.

@kangas kangas closed this Jul 16, 2013
BradBarnich pushed a commit to BradBarnich/mongo that referenced this pull request Feb 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants