go-datastore icon indicating copy to clipboard operation
go-datastore copied to clipboard

Go-datastore and return values

Open ghost opened this issue 10 years ago • 6 comments

Datastore.Get returns its result as a return value, which means that the Datastore implementation needs to allocate it. But if one wishes to store arbitrary types of data into a datastore, how can Get know what type of value to allocate? The only ways I can think of are

  • either the Datastore implementation needs to memoize the reflect.Type of every value as it is Put, so it can recall the type based on the key when it is Get, or
  • the Datastore implementation takes on initialization a mapping from key prefixes/types/something else to reflect.Types, so it can again determine the correct type to allocate based on the key,

but these would bring considerable complexity to any Datastore implementation that wishes to allow storage and retrieval of arbitrarily typed data. Am I missing something obvious, or is this by design? Should any unmarshaling from datastore representations to Go objects (and vice versa) be the client code's responsibility?

ghost avatar Jul 27 '15 16:07 ghost

  • we use interface{} so that wrapper datastores (like caches, etc) do not have to be made per-type. (if we chose, say []byte, then if you wanted to use a Foo then you'd need to make a ton of new datastores which would be a pain. this may be obviated with the use of either generics, or go generate abuse)

  • typically, things will end up as []byte when sent to storage media, i.e. on disk or over the network. However, what i recommend is that -- if you use in-memory caches locally, keep those as objects, and then serialize right before the media. e.g

    myDatastore -> cacheDatastore -> jsonSerializeDS -> flatfs
    

    or something. this way you put off de/serialization till the very end. I think right now we dont have a jsonSerializeDS (or gobSerializeDS), but it's easy to write (see below).

  • either the Datastore implementation needs to memoize the reflect.Type of every value as it is Put, so it can recall the type based on the key when it is Get, or
  • the Datastore implementation takes on initialization a mapping from key prefixes/types/something else to reflect.Types, so it can again determine the correct type to allocate based on the key,
  • yeah, you can do either.

Should any unmarshaling from datastore representations to Go objects (and vice versa) be the client code's responsibility?

  • You can definitely lift it to before the datastores. that's probably most clear and easiest to reason about. (and what we do in IPFS, though for different reasons...). but you can definitely do it within / in the middle. That's how i did it in the python datastore: https://github.com/datastore/datastore

package encodingds

import (
  "encoding/json"

  ds "github.com/jbenet/go-datastore"
)

func Wrap(child ds.Datastore) *Datastore {
  return &Datastore{child}
}

type Datastore struct {
  D ds.Datastore
}

func (d *ds.Datastore) Get(k ds.Key) (interface{}, error) {
  data, err := d.D.Get(k)
  if err != nil {
    return nil, err
  }

  var v2 interface{}
  err = json.Unmarshal(data, &v)
  return v2, err
}

func (d *ds.Datastore) Put(k ds.Key, v interface{}) error {
  data, err := json.Marshal(data, &v)
  if err != nil {
    return err
  }

  return d.Child.Put(k, data)
}

func (d *ds.Datastore) Delete(k ds.Key) error {
  return d.D.Delete(k)
}

func (d *ds.Datastore) Has(k ds.Key) (bool, error) {
  return d.D.Has(k)
}

func (d *ds.Datastore) Query(k ds.Query) {
  // ok, this one is harder...
}

jbenet avatar Jul 28 '15 04:07 jbenet

FWIW, I think a more elegant API would be one where Get returns its result in one of its arguments, i.e.

Get(key Key, v interface{}) error

Now the datastore implementation doesn't need to allocate its return value, so there isn't any need for type bookkeeping either. If necessary, you could still let the datastore implementation decide the return type by passing an empty interface pointer to v. I don't think this would make wrapper datastores any harder to implement either (though having to declare a return variable before every call to Get can get a bit messy.)

ghost avatar Jul 28 '15 12:07 ghost

@jbenet @hubslave i like the idea of using an out parameter here.

whyrusleeping avatar Nov 12 '15 19:11 whyrusleeping

hmmm yeah worth considering.

jbenet avatar Nov 16 '15 17:11 jbenet

Old thread, but happy so show support for the out param ida. It would bring the store interface in line with the (now standard lib) RPC package, which might open the door to some of the other interesting stuff RPC does w Register, protobuf under the hood, etc.

b5 avatar Jun 10 '17 18:06 b5

@b5 Yeah, we've been thinking about reworking the interfaces here soon. If you're interested in pushing that forward, you could open a new issue and propose interface changes.

whyrusleeping avatar Jun 11 '17 00:06 whyrusleeping