WebAssembly / wasi-keyvalue

27 stars 10 forks source link

add get-range queries #4

Closed stevelr closed 1 year ago

stevelr commented 1 year ago

Signed-off-by: stevelr steve@cosmonic.com

stevelr commented 1 year ago

I wasn't able to test the syntax with the latest git version of wit-bindgen since it doesn't currently support the use keyword or some of the other syntax features in this file. Please let me know if there's another parse tool you use.

stevelr commented 1 year ago

Motivation for range support: range support enables a lot of apps that are very hard to implement otherwise. Examples cases:

Downside: why wouldn't you want to do this?

Mossaka commented 1 year ago

I wasn't able to test the syntax with the latest git version of wit-bindgen since it doesn't currently support the use keyword or some of the other syntax features in this file. Please let me know if there's another parse tool you use.

I am not using any parse tools. We can assume use syntax and later adapt to the what wit-bindgen has.

Gearme commented 1 year ago

Just my thoughts on the matter as someone who is basically building the same thing rn - why not use bounds rather than those key-ranges?

variant bound {
  inclusive(bytes),
  exclusive(bytes),
  unbounded
}

with

get-range: func(lower: bound, upper: bound)

Maps pretty well to Rust at least.

In addition (as in the example above) I'll be using bytes (list\<u8> for now) rather than strings for keys: There's cases in which I'll need a range over non-strings (such as big endian i64 timestamps).

stevelr commented 1 year ago

@Gearme the get-range query is over keys, so it should be using the same key type as the whole kvstore. I should have used key instead of string. Per a suggestion from the call earlier today, it should even be option<key> to allow for semantics of "all items up to X", or "all items starting at x".

As to whether keys are binary or strings, that's a good point to raise with the overall interface. The current spec uses strings. If you're using binary keys and had to convert them to base64 to store in this kv store, you wouldn't be able to use the get-range query at all.

@Mossaka @danbugs have you heard other feedback on whether keyvalue keys should be strings or bytes?

danbugs commented 1 year ago

As to whether keys are binary or strings, that's a good point to raise with the overall interface. The current spec uses strings. If you're using binary keys and had to convert them to base64 to store in this kv store, you wouldn't be able to use the get-range query at all.

@Mossaka @danbugs have you heard other feedback on whether keyvalue keys should be strings or bytes?

As far as I recall, this hasn't been brought up before. Regardless, I don't think every implementer supports storing binary data as keys — e.g., iirc, Memcached doesn't.

Mossaka commented 1 year ago

@stevelr could you please resolve the conflicts? I'd like to merge this PR in sometime later.

stevelr commented 1 year ago

too out of date - closing this. needs to be re-filed as a new PR