Add checks to make sure Update Operators do not use conflicting/overlapping paths

[tatu-at-datastax]

What this PR does:

Improves checking of dotted paths used with update operator to ensure overlapping paths are not allowed (same as with other mongoose backends).

Which issue(s) this PR fixes:
Fixes #232

Checklist

• Changes manually tested
• Automated Tests added/updated
• Documentation added/updated
• CLA Signed: DataStax CLA

[vkarpov15]

Just to confirm, would updateOne({}, { $inc: { 'root.x': 2 }, $mul: { 'root.mul': 3 } }) still work?

[tatu-at-datastax]

Yes. I should add a test case to verify.

[tatu-at-datastax]

Added another IT that checks nested.old ($unset) and nested.new ($set) (same logic between all operators).

[vkarpov15]

I don't remember off the top of my head which characters we allow in field names, but I don't think this will work if we allow spaces, $, or + in field names. Because "path.x$foo" comes before "path.x.foo" in lexicographical order.

Is there any way we can use a different approach here? I'd rather not add code that blocks us from supporting "$" in field names.

[tatu-at-datastax]

I am not sure why dollar sign or other characters would change logic here: . is separator character and nothing else? Or are you saying paths path.x$foo and path.x.foo conflict? (I wouldn't think they do if "$" is part of path segment?)

That is, sorting for purpose of finding parent/child paths seems to work as far as I can see.

[vkarpov15]

@tatu-at-datastax the issue is your alphabetical sorting assumption. The assumption here is that path.x.some.more would immediately follow path.x in the sort order, unless there's some other conflicting path. But in ASCII, $ is less than .. So conventional string sorting would put fields with dollar paths before conflicting paths. Below is an example in JavaScript

> ['path.x', 'path.x.some.more', 'path.x$foo'].sort()
[ 'path.x', 'path.x$foo', 'path.x.some.more' ]

Not sure if that applies here, just figured I'd call that out.

[tatu-at-datastax]

@vkarpov15 Ah. Ok, let me think this over. Good catch.

[tatu-at-datastax]

I can make sorting work with some more work. Two obvious extensions:

1. Use (implement) modified String comparison where . is sorted like null byte (value 0)
2. Sort split segments (which Locator already has)

both would work; I may go with (2).

[tatu-at-datastax]

Added segment-aware sorting, testing to verify functioning.

[vkarpov15]

Nice, thanks. It looks like the reason why this works though is because you've included a loop over path segments in the compare() method - any worries about this being potentially slow? You'll have to iterate over path segments O(nlogn) times, creating a map of all parent paths will likely be faster but more memory intensive. I don't think this is a big deal either way, just food for thought.

[tatu-at-datastax]

@vkarpov15 that's a reasonable question. I guess it all depends on depth of document. My thinking was that for deeper documents most differences are at first segment(s) anyway so that it evens out, compared to String comparison. But my main consideration was, I guess, between straight String comparison (just changing sort value of .) vs segmented, where I think performance is similar. I did not consider possibility of building lookup for covered paths: in that case it does come back to both memory usage added and (more importantly), construction of new String instances to reconstruct parent paths. I guess it can go either way wrt overall performance, in the end.

Fundamentally I guess it also comes down to how many updates do we usually have: I suspect differences are not meaningful until into thousands of operations.

Since this logic is fairly isolated, I think I'll keep the question in mind and can reconsider re-implementation if we find a hotspot.