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

Fix PkvStore::get not returning NotFound on empty db when building for native (redb) #46

Merged
merged 1 commit into from
Jan 12, 2024

Conversation

richardhozak
Copy link
Contributor

@richardhozak richardhozak commented Jan 9, 2024

Previously when doing PkvStore::get on just created (empty) database would return internal redb table error instead of GetError::NotFound. This behavior is not consistent with all the other backends which return GetError::NotFound. The error that gets returned is GetError::ReDbTableError(redb::TableError::TableDoesNotExist(_)).

redb returns this error as it does not create tables when calling open_table on read transactions, it creates tables only when calling open_table on write transactions.

The two workarounds are:

  • Do not handle Err variant when doing PkvStore::get and just handle Ok variant
    • (Not always applicable, sometimes we do not want to write if key does not exist and want to know if it is NotFound or other critical error)
  • Write to database first, before doing any reads

This forces users of bevy_pkv to add explicit dependency on redb if they want to handle this error, making it harder to support cross platform apps.

The solution is to create the table beforehand when creating database.

@richardhozak richardhozak changed the title Fix PkvStore::get not returning NotFound on empty db Fix PkvStore::get not returning NotFound on empty db when building for native (redb) Jan 10, 2024
@johanhelsing
Copy link
Owner

Thanks for the PR and detailed write-up!

@johanhelsing johanhelsing merged commit b5beb2b into johanhelsing:main Jan 12, 2024
8 checks passed
@johanhelsing
Copy link
Owner

Added a test for it in #47

@johanhelsing johanhelsing added the bug Something isn't working label Jan 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants