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

Reconsider treatment of unexported embedded structs with exported fields #25

Open
albrow opened this issue Jan 23, 2016 · 2 comments
Open
Labels

Comments

@albrow
Copy link
Owner

albrow commented Jan 23, 2016

After reading about changes to the reflect package in the upcoming go 1.6 release, I've realized that Zoom does not handle unexported embedded structs with exported fields the same way that the encoding/json and encoding/xml packages do. For example, consider the following:

type inner struct {
  A int
  B string
}
type Outer struct {
  inner
}

The encoding/json package will consider the fields of inner as if they are fields of Outer, producing the following JSON:

{
  "A": 42,
  "B": "foo"
}

This is to support the struct composition pattern (an analogue of inheritance in classical object oriented languages).

However, Zoom does not make this consideration, and treats inner as if it were any other unexported field. As a result it will not save the A and B fields of inner in the database.

If the inner struct type were exported, Zoom would save it in the database. But it would treat it as an inconvertible type and use the fallback encoding (either encoding/json or encoding/gob) instead of saving each field as a field in the Redis hash. As a result, you would not be able to run queries on the fields of inner.

Should Zoom work more similarly to the encoding/json and encoding/xml packages and consider the fields of inner as if they were top-level fields of Outer?

@rykov
Copy link

rykov commented Sep 7, 2016

It looks like it's more than just a bug with visibility. Even when the embedded struct is exported, zoom sees it as an opaque field. For this particular case:

type Inner struct {
  A int
  B string
}
type Outer struct {
  Inner
}

It will save Outer to Redis as follows:

{ "Inner": "...binary blob of Inner..." }

Instead of:

{ "A": 42, "B": "foo" }

@albrow
Copy link
Owner Author

albrow commented Jan 10, 2017

Thanks @rykov. I'm not sure if you just meant for this to be a clarifying comment, but the behavior you described is something I'm aware of and it is addressed in the original issue text:

If the inner struct type were exported, Zoom would save it in the database. But it would treat it as an inconvertible type and use the fallback encoding (either encoding/json or encoding/gob) instead of saving each field as a field in the Redis hash. As a result, you would not be able to run queries on the fields of inner.

The behavior is also documented under CollectionOptions.FallbackMarshalerUnmarshaler.

You could argue that this is not desirable and Zoom should do something to make embedded struct fields more accessible (whether they are exported or unexported). The purpose of this issue is to discuss that possibility. What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants