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

Fix validation process for creating duplicate _id index #3990

Merged
merged 7 commits into from
Jan 23, 2024

Conversation

kropidlowsky
Copy link
Contributor

@kropidlowsky kropidlowsky commented Jan 19, 2024

Description

Align behavior of index creation commands for key: _id to meet MongoDB requirements -those operations should not raise Index already exists with a different name error when key: _id (no matter if there is name in the query).

Closes #3921.

Readiness checklist

  • I added/updated unit tests (and they pass).
  • I added/updated integration/compatibility tests (and they pass).
  • I added/updated comments and checked rendering.
  • I made spot refactorings.
  • I updated user documentation.
  • I ran task all, and it passed.
  • I ensured that PR title is good enough for the changelog.
  • (for maintainers only) I set Reviewers (@FerretDB/core), Milestone (Next), Labels, Project and project's Sprint fields.
  • I marked all done items in this checklist.

@kropidlowsky kropidlowsky requested a review from a team as a code owner January 19, 2024 00:29
Copy link

codecov bot commented Jan 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (10815ae) 74.53% compared to head (f5a4ae7) 74.59%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3990      +/-   ##
==========================================
+ Coverage   74.53%   74.59%   +0.06%     
==========================================
  Files         337      337              
  Lines       27015    27015              
==========================================
+ Hits        20136    20153      +17     
+ Misses       5623     5610      -13     
+ Partials     1256     1252       -4     
Files Coverage Δ
internal/handler/msg_createindexes.go 88.75% <100.00%> (ø)

... and 4 files with indirect coverage changes

Flag Coverage Δ
filter-true 70.86% <100.00%> (+0.06%) ⬆️
integration 70.86% <100.00%> (+0.06%) ⬆️
mongodb-1 5.25% <0.00%> (ø)
postgresql-1 49.92% <100.00%> (+0.06%) ⬆️
postgresql-2 50.48% <100.00%> (+<0.01%) ⬆️
postgresql-3 52.84% <100.00%> (+0.15%) ⬆️
sqlite-1 48.96% <100.00%> (-0.08%) ⬇️
sqlite-2 49.57% <100.00%> (+<0.01%) ⬆️
sqlite-3 51.96% <100.00%> (+0.02%) ⬆️
unit 32.56% <0.00%> (ø)

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

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.

The test looks good to me, and the fix is also reasonable.

However, now an SQLite test task test-integration-sqlite TEST_RUN='TestCreateIndexesCompat' fails, so we need some more fixes. Please feel free to ping me if you have any questions about it.

@rumyantseva rumyantseva added the code/bug Some user-visible feature works incorrectly label Jan 19, 2024
@rumyantseva rumyantseva added this to the Next milestone Jan 19, 2024
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.

Ok, I see all index-related tests pass, so the fix works!

@rumyantseva rumyantseva requested review from a team, henvic and noisersup January 22, 2024 07:53
@rumyantseva rumyantseva enabled auto-merge (squash) January 22, 2024 07:53
@rumyantseva rumyantseva disabled auto-merge January 22, 2024 07:54
@rumyantseva rumyantseva changed the title Fix create index commands for key: _id Fix validation process for creating duplicate _id index Jan 22, 2024
@rumyantseva rumyantseva enabled auto-merge (squash) January 22, 2024 07:56
@henvic henvic disabled auto-merge January 22, 2024 09:25
@henvic henvic enabled auto-merge (squash) January 22, 2024 09:25
@henvic henvic merged commit d282fb1 into FerretDB:main Jan 23, 2024
28 checks passed
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
Archived in project
Development

Successfully merging this pull request may close these issues.

Creating an existing _id index causes a name conflict error
4 participants