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

Go-datastore and return values #28

Open
ghost opened this issue Jul 27, 2015 · 6 comments
Open

Go-datastore and return values #28

ghost opened this issue Jul 27, 2015 · 6 comments
Labels
need/analysis Needs further analysis before proceeding

Comments

@ghost
Copy link

ghost commented Jul 27, 2015

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?

@jbenet
Copy link
Member

jbenet commented Jul 28, 2015

  • 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...
}

@ghost
Copy link
Author

ghost commented Jul 28, 2015

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.)

@whyrusleeping
Copy link
Member

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

@jbenet
Copy link
Member

jbenet commented Nov 16, 2015

hmmm yeah worth considering.

@whyrusleeping whyrusleeping added the need/analysis Needs further analysis before proceeding label Sep 14, 2016
@b5
Copy link
Contributor

b5 commented Jun 10, 2017

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.

@whyrusleeping
Copy link
Member

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/analysis Needs further analysis before proceeding
Projects
None yet
Development

No branches or pull requests

3 participants