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

Implement dropIndexes for SQLite #3329

Merged
merged 105 commits into from Sep 13, 2023

Conversation

rumyantseva
Copy link
Member

@rumyantseva rumyantseva commented Sep 8, 2023

Description

Closes #3287.

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.

internal/backends/collection.go Show resolved Hide resolved
internal/backends/collection.go Outdated Show resolved Hide resolved
internal/backends/decorators/dummy/collection.go Outdated Show resolved Hide resolved
internal/backends/error.go Outdated Show resolved Hide resolved
internal/backends/error.go Outdated Show resolved Hide resolved
internal/backends/decorators/oplog/collection.go Outdated Show resolved Hide resolved
@AlekSi AlekSi modified the milestones: v1.10.0, Next Sep 11, 2023
Copy link
Contributor

@chilagrow chilagrow left a comment

Choose a reason for hiding this comment

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

Just some suggestions 🤗

great work!

internal/handlers/sqlite/msg_dropindexes.go Outdated Show resolved Hide resolved
internal/backends/sqlite/collection.go Outdated Show resolved Hide resolved
internal/backends/collection.go Outdated Show resolved Hide resolved
internal/backends/sqlite/metadata/registry.go Outdated Show resolved Hide resolved
@rumyantseva
Copy link
Member Author

Neunundneunzig Luftballons :D

Copy link
Contributor

@chilagrow chilagrow left a comment

Choose a reason for hiding this comment

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

Just a few comments, great work simplifying it! 🤗

internal/backends/postgresql/collection.go Outdated Show resolved Hide resolved
for _, index := range existing {
for i, key := range index.Key {
if key.Field == spec[i].Field && key.Descending == spec[i].Descending {
return []string{index.Name}, false, nil
Copy link
Contributor

Choose a reason for hiding this comment

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

We are returning if first field matches with spec[i].Field. Don't we want to check if all Field match? 🤔

For example, if index contains two fields, it will match as long as first field is the same.

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice catch! Our tests didn't find it, so I think I should add a test where the first field matches and the second doesn't.

internal/handlers/sqlite/msg_dropindexes.go Show resolved Hide resolved
command,
)
}

Copy link
Contributor

Choose a reason for hiding this comment

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

// TODO cannot drop ID index index == "_id_"

@AlekSi AlekSi modified the milestones: Next, v1.10.0 Sep 13, 2023
@AlekSi AlekSi merged commit 82c1919 into FerretDB:main Sep 13, 2023
29 checks passed
@AlekSi
Copy link
Member

AlekSi commented Sep 13, 2023

@rumyantseva PTAL on Chi's comments and send a follow up RP, if needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code/feature Some user-visible feature is not implemented yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Implement dropIndexes for SQLite
4 participants