Docstore/memdocstore: nested query

[eqinox76]

Fixes #3506 docstore/memdocstore query for nested documents with slices does not work

Added a test to the drivertest. It is working fine with the fixed memdocstore implementation but i can't test it against the other cloud providers.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[vangent]

can you update your PR to remove the drivertest changes and add that option? You should be able to add it to the existing Options struct

[eqinox76]

Thank you @vangent and @jba for your reviews! I amended the pr and i think only the question about the removed function is open. Please let me know if i missed anything.

[vangent]

@jba I'd like to wait for your review/LGTM.

[codecov]

Codecov Report

Attention: Patch coverage is 91.22807% with 5 lines in your changes missing coverage. Please review.

Project coverage is 70.63%. Comparing base (01e0447) to head (cd9090a).
Report is 13 commits behind head on master.

Files with missing lines	Patch %	Lines
docstore/memdocstore/mem.go	88.46%	2 Missing and 1 partial ⚠️
docstore/memdocstore/query.go	92.30%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3508      +/-   ##
==========================================
+ Coverage   70.60%   70.63%   +0.02%     
==========================================
  Files         113      115       +2     
  Lines       14384    14521     +137     
==========================================
+ Hits        10156    10257     +101     
- Misses       3501     3536      +35     
- Partials      727      728       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[jba]

I'll try to get to it this weekend/early next week.

[jba]

Thanks for doing this work. Sorry it took me a while to give it careful look. The only major change is improving the test, otherwise looks good!

[jba]

I think the tests would be improved by actually comparing the results. Also, use table-driven style. See testGetQuery for an example.

[eqinox76]

Agreed, i rewrote it to a table-driven style. Please check if its more readable

[vangent]

@eqinox76 can you address the open comments and sync to head?

[eqinox76]

Hi @vangent , @jba ,
sorry for the long delay. I missed the notifications. On the plus side we found a few omissions in the feature. Especially when the nesting is deeper than one layer of slices or for more complex operations like '>='.
I updated to head, fixed the bugs and amended unit tests. Sadly the change got a bit bigger and especially in getAtFieldPath (mem.go:410) a bit unwiedly for the nested branch.

Please let me know if something else is missing. And many thanks for your time and work!

[vangent]

@jba when you have a chance can you re-review, sounds like there have been non-trivial changes since your approval.

[eqinox76]

Thanks for the review. Shadowing the variable name makes it actually more readable. I commit the changes and fixed the comment style. Please let me know when there is something else that i missed.

[jba]

Thank you! Sorry my reviewing has taken so long.

[eqinox76]

Hi, I fixed a flaky test that your test pipeline correctly identified and pushed the changes.