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

Index keys consistency #568

Merged
merged 7 commits into from
Dec 7, 2021
Merged

Index keys consistency #568

merged 7 commits into from
Dec 7, 2021

Conversation

maurolacy
Copy link
Contributor

@maurolacy maurolacy commented Dec 6, 2021

Closes #532.

This simplifies the key returned by MultiIndex, so that it is consistent with the UniqueIndex key.

In principle the other approach is also possible: making UniqueIndex return the same than MultiIndex; see https://github.com/CosmWasm/cw-plus/tree/532-index-keys-consistency.

But this approach is simpler: we just return a tuple with the associated primary key, and the value. Given that the index key is a combination of the data fields present in the value, this makes sense, both to avoid returning redundant data and for simplicity.

This PR also introduces a generic type for signaling the type for primary key deserialization, like in UniqueIndex. So, it also closes (the first part of) #531.

Overall, the end result is that both indices are now behaving more similarly. Both in their returned values and also in their specification.

Copy link
Contributor

@ueco-jb ueco-jb left a comment

Choose a reason for hiding this comment

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

lgtm, I like those changes very much.
Indexing's getting simpler and simpler with every such change.

packages/storage-plus/src/indexed_map.rs Outdated Show resolved Hide resolved
packages/storage-plus/src/indexes/multi.rs Outdated Show resolved Hide resolved
@maurolacy
Copy link
Contributor Author

Merging this. Please take a look at the follow-up (#569). After that and #460, I think this would be ready for cutting 0.11.

@maurolacy maurolacy merged commit e91bfee into main Dec 7, 2021
@maurolacy maurolacy deleted the 532-index-keys-consistency-alt branch December 7, 2021 05:27
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.

UniqueIndex / MultiIndex key consistency
2 participants