Fix comparison with unset fields

[chilagrow]

Description

Closes #1023.

This PR is about making unset work the same way as mongoDB. In the issue there is a link to a close #1024 which shows that unset comparison had issues. In this PR, we compare unset field with null field using compat dataset and tests. See Unset field being un-commented in the Scalar shareddata, so every compat test is run on Unset bson.

For a field v which is used in integration test, unset BSON looks like {_id: "unset"}, while null field BSON looks like {_id: "null", v: null}.

What was done before this PR:
We checked if filter was null filter, if it was it returned true indicating match, otherwise it returned false. So it returned early based on the filter value being null or not. And this would fail on operator such as $gt, $lt, $nin and $in. For example when comparing with filter {v: {$gt: null} on {_id: "unset"} we returned true before this PR. But compat test tells us it should be false.

What we change:
The field is set to null on unset value and let the comparison Compare(a, b any) do the work on returning result.

Readiness checklist

• I added tests for new functionality or bugfixes.
• I ran task all, and it passed.
• I added/updated comments for both exported and unexported top-level declarations (functions, types, etc).
• I checked comments rendering with task godocs.
• I ensured that the title is good enough for the changelog.
• (for maintainers only) I set Reviewers (@FerretDB/core), Assignee, Labels, Project and project's Sprint fields.
• I marked all done items in this checklist.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
ferret-db	✅ Ready (Inspect)	Visit Preview	Dec 13, 2022 at 3:24AM (UTC)

[codecov]

Codecov Report

Merging #1634 (b627c71) into main (028f9e7) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1634      +/-   ##
==========================================
- Coverage   68.49%   68.48%   -0.02%     
==========================================
  Files         285      285              
  Lines       13857    13857              
==========================================
- Hits         9492     9490       -2     
+ Misses       3433     3432       -1     
- Partials      932      935       +3     

Impacted Files	Coverage Δ
integration/shareddata/scalars.go	100.00% <ø> (ø)
integration/shareddata/shareddata.go	85.33% <100.00%> (ø)
internal/handlers/common/filter.go	85.72% <100.00%> (+0.41%)	⬆️
internal/handlers/common/sort.go	91.83% <100.00%> (+8.50%)	⬆️
internal/handlers/common/update.go	86.03% <100.00%> (ø)
internal/util/version/version.go	70.58% <0.00%> (-4.42%)	⬇️
internal/clientconn/conn.go	44.02% <0.00%> (-2.63%)	⬇️
internal/clientconn/listener.go	80.64% <0.00%> (-2.42%)	⬇️

Flag	Coverage Δ
integration	64.31% <100.00%> (-0.02%)	⬇️
mongodb	14.12% <4.54%> (-0.04%)	⬇️
pg	53.27% <100.00%> (-0.02%)	⬇️
tigris	37.20% <54.54%> (+0.10%)	⬆️
unit	27.21% <0.00%> (-0.04%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

[rumyantseva]

I think I need more context to review the PR. What issues does it fix?
I see the one in elemMatch where we check filterKey in the document, what else?

[noisersup]

Yeah I'm also struggling a little bit with understanding the scope of this PR.
If I understand there's an issue that we handle unset values as nulls, and mongodb differentiate between them, right? Also there's a new TestQueryCompatSort test which is a result of finding some sorting bug in our compat test?

[chilagrow]

@rumyantseva @noisersup thanks for the feedback, I added description of PR and comments in the code.

[chilagrow]

Yeah I'm also struggling a little bit with understanding the scope of this PR. If I understand there's an issue that we handle unset values as nulls, and mongodb differentiate between them, right? Also there's a new TestQueryCompatSort test which is a result of finding some sorting bug in our compat test?

@noisersup if we see null in the filter and unset value in the docValue, our filter returned true before this PR. In reality, we should apply operator and returns the result based on the filter operator. This is what the PR is about. And yes, mongoDB differentiate the result based on the operator. The way I did it, most operators treat null and missing key equal [see] (https://www.mongodb.com/docs/manual/reference/bson-type-comparison-order/#non-existent-fields). I handle the operators that shouldn't treat null and unset equal upon getting error. Other operators should handle null and unset equally but each operator has to apply either logic, for that I set missing fieldValue as null to allow that.

[chilagrow]

I think I need more context to review the PR. What issues does it fix? I see the one in elemMatch where we check filterKey in the document, what else?

@rumyantseva fix in this PR is about errors that was produced by adding Unset to Scalar shareddata so they are used in compat test.

• upon updating unset document which only has _id key, it has to return that the collection was modified.
• before this PR is filter was null and document has missing value, it returned true. We no longer do that, instead we set null to document and let each operator do its logic. So we don't need special case such as $all anymore. unset and null are equivalent for comparison. But there are exception for $not, $elemMatch and $exists operators.
• $type operator if the filter is not null we let the operator handle errors, compat test caught that. If filter is null we return immediately because null and unset are not the same for $type
• For sorting null and missing field is equivalent. Compat caught that.

And added a new test case MultipleQueriesSorted because QueriesSorted failed after adding unset and I think the reason is deeper, not sure yet.

[rumyantseva]

LGTM

[w84thesun]

Thanks for the extended explanation, it's made it easier to understand what's going on here 😃