Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

libs/db: disallow empty keys #2

Closed
melekes opened this issue May 3, 2019 · 4 comments · Fixed by #99
Closed

libs/db: disallow empty keys #2

melekes opened this issue May 3, 2019 · 4 comments · Fixed by #99
Labels
T:enhancement Type: Enhancement, New feature or request

Comments

@melekes
Copy link
Contributor

melekes commented May 3, 2019

See tendermint/tendermint#3610 (comment)

Reasons to do that:

  • many KV stores does not support empty keys
  • I don't know why would someone want to set en empty key (and not call it default or something else)
@Lawliet-Chan
Copy link

@jaekwon

@melekes
Copy link
Contributor Author

melekes commented Jun 24, 2019

Found this commit 4ce8448d7fcf92b040046f894474ce2f7e779b67, but it does not explain why nil keys are OK.

@tac0turtle
Copy link
Contributor

The SDK does not use empty keys. will see others then we can look at moving forward with this.

@tac0turtle tac0turtle transferred this issue from tendermint/tendermint Jul 19, 2019
@tac0turtle tac0turtle added the T:enhancement Type: Enhancement, New feature or request label Feb 25, 2020
@erikgrinaker
Copy link
Contributor

I'm making some breaking changes to the interface shortly, suggest we do this at the same time since there are a few issues with empty keys (notably BoltDB not supporting it, and cLevelDB panics). Any objections?

erikgrinaker added a commit that referenced this issue May 18, 2020
Modify batch `Set`, `Delete`, and `Close` to return errors. The main error condition currently is using closed batches, but future ones include databases which implement batches as transactions (e.g. BadgerDB) and rejecting empty keys (#2).
erikgrinaker added a commit that referenced this issue May 19, 2020
faddat referenced this issue in notional-labs/tm-db Apr 12, 2022
tac0turtle added a commit that referenced this issue Aug 25, 2022
* rename go mod

* rename go mod
yihuang added a commit to yihuang/tm-db that referenced this issue Jan 18, 2023
changelog

Signed-off-by: Tomas Tauber <2410580+tomtau@users.noreply.github.com>
faddat pushed a commit to faddat/tm-db that referenced this issue Feb 21, 2024
…endermint#2)

Bumps [golangci/golangci-lint-action](https://github.com/golangci/golangci-lint-action) from 3.2.0 to 3.3.1.
- [Release notes](https://github.com/golangci/golangci-lint-action/releases)
- [Commits](golangci/golangci-lint-action@v3.2.0...v3.3.1)

---
updated-dependencies:
- dependency-name: golangci/golangci-lint-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
T:enhancement Type: Enhancement, New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants