oliver-oloughlin / kvdex

A high-level abstraction layer for Deno KV with zero third-party dependencies by default 🦕🗝️
https://jsr.io/@olli/kvdex
MIT License
196 stars 7 forks source link

Atomization operations cannot set keys that already exist #236

Open LaneSun opened 2 weeks ago

LaneSun commented 2 weeks ago

I recently used KVDex in my own project and I found that the non-atomic Set operation is different from KV in that when the key already exists, you need to specify the Overwrite attribute for it to succeed, and similarly, when using the atomic Set operation, I found it impossible to successfully overwrite an existing key.

Here is the example

oliver-oloughlin commented 1 week ago

Thanks for bringing my attention to this. As for the normal set() operation, that is the expected behaviour. I can start by explaining my reasoning for making it behave different than the native set() by default:

  1. It mimics the behaviour of traditional databases, where data by a primary key cannot be overwritten by default, and the developer is forced to explicitly handle cases where you wish to overwrite existing values.
  2. The add() method should act exactly the same as set(), but with an automatically generated id. If add() were to overwrite data by default, this could cause it to randomly overwrite existing values, which I consider very bad behaviour. So to keep these two methods aligned, set() must not overwrite by default.

As for the atomic set() operation, I understand that not being able to overwrite existing values can be an issue. Right now this limitation is due to indexable collections, as if you are overwriting a value that has index entries, these index entries would not be deleted, which can cause collision issues when inserting new data. This probably can be fixed though by handling standard collections and indexable collections differently, likely by throwing an error for the case of indexable collections when using atomic set() with overwrite = true. I will look into this.