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

Handle findAndModify and update correctly when collection doesn't exist #1087

Merged
merged 8 commits into from
Aug 31, 2022

Conversation

noisersup
Copy link
Member

@noisersup noisersup commented Aug 26, 2022

Description

This PR closes #651.

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.
  • (for maintainers only) I set Reviewers (@FerretDB/core), Assignee, Labels, Project and project's Sprint fields.
  • I marked all done items in this checklist.

@codecov
Copy link

codecov bot commented Aug 26, 2022

Codecov Report

Merging #1087 (ab0ea25) into main (9d99140) will increase coverage by 0.15%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1087      +/-   ##
==========================================
+ Coverage   65.57%   65.72%   +0.15%     
==========================================
  Files         248      248              
  Lines       11816    11817       +1     
==========================================
+ Hits         7748     7767      +19     
+ Misses       3198     3182      -16     
+ Partials      870      868       -2     
Impacted Files Coverage Δ
internal/handlers/pg/msg_findandmodify.go 72.90% <100.00%> (+0.09%) ⬆️
internal/util/version/version.go 55.00% <0.00%> (-5.00%) ⬇️
internal/handlers/common/filter.go 85.38% <0.00%> (ø)
internal/wire/msg_body.go 38.09% <0.00%> (+1.58%) ⬆️
internal/wire/msg_header.go 71.42% <0.00%> (+2.38%) ⬆️
internal/handlers/tigris/msg_update.go 46.71% <0.00%> (+2.63%) ⬆️
internal/clientconn/listener.go 79.16% <0.00%> (+3.12%) ⬆️
internal/handlers/pg/msg_update.go 54.06% <0.00%> (+3.48%) ⬆️
internal/handlers/pg/pgdb/settings.go 44.18% <0.00%> (+4.65%) ⬆️
Flag Coverage Δ
integration 61.96% <100.00%> (+0.28%) ⬆️
mongodb 18.38% <0.00%> (-0.06%) ⬇️
pg 63.91% <100.00%> (+0.42%) ⬆️
tigris 31.86% <0.00%> (+<0.01%) ⬆️
unit 24.65% <0.00%> (-0.03%) ⬇️

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

@noisersup noisersup self-assigned this Aug 26, 2022
@noisersup noisersup force-pushed the no-collection-fix-651 branch 2 times, most recently from 9f97c3c to fbb91ca Compare August 30, 2022 17:51
@noisersup noisersup added the code/bug Some user-visible feature works incorrectly label Aug 30, 2022
@noisersup noisersup marked this pull request as ready for review August 30, 2022 17:57
@noisersup noisersup requested a review from a team as a code owner August 30, 2022 17:57
@noisersup noisersup requested review from AlekSi, rumyantseva, a team and w84thesun and removed request for a team August 30, 2022 17:57
Copy link
Contributor

@rumyantseva rumyantseva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As our idea was to understand how mongo behaves and offer the same behavior, this looks good enough to me.

Copy link
Contributor

@w84thesun w84thesun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@w84thesun w84thesun merged commit 0c96f23 into FerretDB:main Aug 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code/bug Some user-visible feature works incorrectly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Calling a non-existing collection causes internal error
3 participants