-
Notifications
You must be signed in to change notification settings - Fork 409
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
Conversation
There was a problem hiding this 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!
Neunundneunzig Luftballons :D |
There was a problem hiding this 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! 🤗
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
command, | ||
) | ||
} | ||
|
There was a problem hiding this comment.
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_"
Co-authored-by: Chi Fujii <chi.fujii@ferretdb.io>
@rumyantseva PTAL on Chi's comments and send a follow up RP, if needed |
Description
Closes #3287.
Readiness checklist
task all
, and it passed.@FerretDB/core
), Milestone (Next
), Labels, Project and project's Sprint fields.