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

Always persist IndexConfig #288

Merged
merged 1 commit into from
Mar 27, 2020
Merged

Always persist IndexConfig #288

merged 1 commit into from
Mar 27, 2020

Conversation

asutula
Copy link
Member

@asutula asutula commented Mar 27, 2020

Adds tests for adding indexes as well

fixes #285
fixes #284

Signed-off-by: Aaron Sutula <hi@asutula.com>
@asutula asutula self-assigned this Mar 27, 2020
@asutula asutula added this to the Sprint 33 milestone Mar 27, 2020
db/db.go Show resolved Hide resolved
db/db.go Show resolved Hide resolved
Copy link
Member

@sanderpick sanderpick left a comment

Choose a reason for hiding this comment

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

LGTM! 💣

db/db.go Show resolved Hide resolved
@asutula
Copy link
Member Author

asutula commented Mar 27, 2020

One question I have:

Are there any consequences with the user overwriting an existing index and changing Unique from false to true, for example?

@asutula
Copy link
Member Author

asutula commented Mar 27, 2020

That all kinda makes my head hurt. For sure there could be issues once we support back-indexing already existing data. But since that's not yet the case, does switching the value of Unique have any consequences from the time it is switched going forward?

@carsonfarmer
Copy link
Member

There are no knock-on effects to changing an existing index like that @asutula, it will just no longer check if a key is unique. The only place that the 'uniqueness' is persisted is in the index definition. The indexed data itself doesn't care about this directly, just the index is responsible check checking this.

@carsonfarmer
Copy link
Member

The only consequences of course, are that if Unique was false previously, and then set to true, those past values that may or may not be unique will no longer be checked, only new data will be checked for uniqueness. So this won't throw, but it might be unexpected behavior. Hence why something like #286 would be good here as well.

Copy link
Member

@carsonfarmer carsonfarmer left a comment

Choose a reason for hiding this comment

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

Nice dude, I hope this, plus something like #290 will increase usability of this stuff!

db/collection.go Show resolved Hide resolved
@asutula asutula merged commit 07555d0 into master Mar 27, 2020
@asutula asutula deleted the asutula/persist-index-config branch March 27, 2020 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Index definition isn't persisted Add ID index on Collection creation
3 participants