-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
proposal: encoding/json: Encode channel as array #11940
Comments
I guess you are suggesting that enc.Encode not always terminate with a newline? Other than that there doesn't seem to be an issue here. |
It would be nice if it didn't terminate with a newline, but I'm assuming that can't be changed, right? What I'm suggesting in this issue is to support encoding a channel - encode it as an array as soon as values are received. With my use-case I'm streaming a very large json document to a mobile client - which stream decodes the document. The goal is to never have the entire document in memory on the server or client. The server pushes each row onto the channel, which gets encoded on the wire, which gets decoded and inserted into a local database. |
I wrote a utility method to handle this: func jsonEncodeChan(w io.Writer, vc interface{}) (err error) {
cval := reflect.ValueOf(vc)
_, err = w.Write([]byte{'['})
if err != nil {
return
}
var buf *bytes.Buffer
var enc *json.Encoder
v, ok := cval.Recv()
if !ok {
goto End
}
// create buffer & encoder only if we have a value
buf = new(bytes.Buffer)
enc = json.NewEncoder(buf)
goto Encode
Loop:
v, ok = cval.Recv()
if !ok {
goto End
}
if _, err = w.Write([]byte{','}); err != nil {
return
}
Encode:
err = enc.Encode(v.Interface())
if err == nil {
_, err = w.Write(bytes.TrimRight(buf.Bytes(), "\n"))
buf.Reset()
}
if err != nil {
return
}
goto Loop
End:
_, err = w.Write([]byte{']'})
return
} That's how I would envision encoding/json handling a channel. Although it would be a bit simpler using the internal |
was running into this today when trying to stream out json. if we had something like the following, the mapping into and and out of chans might be compatible with the corresponding slice of the same type. im going to look at the source and see if there is any hope of this working.
|
CL https://golang.org/cl/33591 mentions this issue. |
In general there are lots of ways a channel might expect to be used. This would hard-code one and break all the other ways. Also people tend to think of Marshal as a read-only operation on the underlying data. Consuming from a channel would not be. These two seem like serious problems, and I don't see a corresponding serious need. You can do this today by defining a custom channel type with a MarshalJSON method. |
Since no one argued against this, declining proposal. |
Sorry, I've had a busy week and haven't had a chance to respond to this. @rsc could you expand on this:
Currently a channel is not handled at all, correct? And if the default behavior would be to treat it as an array, wouldn't Marshaler/Unmarshaler override the default behavior? The Marshaler interface also doesn't work for streaming because you have to hold the entire contents in memory before returning it: // Note: This code is abbreviated and not tested
func (c ChannelType) MarshalJSON() ([]byte, error) {
buf := new(bytes.Buffer)
buf.WriteString("[")
enc = json.NewEncoder(buf)
for v := range c {
enc.Encode(v)
buf.WriteString(",")
}
buf.WriteString("]")
return buf.Bytes()
} This makes streaming not possible with json's Marshaler. That's why I wrote the utility method in a previous post. The encoding/xml Marshaler is actually more flexible: type Marshaler interface {
MarshalXML(e *Encoder, start StartElement) error
} It passes the encoder so you can 1) add to the output stream, and 2) you don't have to define another encoder. I would like to see a channel treated as an array... But if the Marshaler looked like encoding/xml's, it wouldn't be needed: type Marshaler interface {
MarshalJSON(e *Encoder) error
} (The Encoder would have a method for writing tokens, or you could encode individual values) Of course there is a compatibility issue with changing the interface.. which is why it was suggested to treat channel as an array. Which, if I'm not mistaken, is an error unless you write a Marshaler for it. |
Re uses: What if the channel is in the struct because it is for sending to, not receiving from? What if there are a known number of values to receive over the channel, and it will never be closed? Lots of things that could reasonably go wrong. Better to reject channels by default: an error message is better than a deadlock because the channel is being used wrong. As for json streaming, the current encoder does not write any part of the JSON response to the underlying writer until the entire JSON is constructed. (Otherwise it might encounter some problem midway and return an error but have written a half-JSON to the underlying writer. It avoids this by buffering until it knows the JSON conversion has succeeded.) Some people do want streaming, of course, and if we add streaming we should also add some way to stream during a custom encoding method. But that makes streaming a non-argument for built-in channel handling. |
Ah, good points on the channels. I did not think about that. If we had a way to do streaming without involving channels, that would solve the problem for me 👍 . Is there a proposal that covers the Marshaler? Does one need to be created? |
I don't believe there is any proposal, but I think we'd rather see that developed as an external package first. |
I'm currency doing something like this to encode a channel:
I have a couple issues with this approach:
1 - It's verbose
2 - enc.Encode terminates each object with a newline character, so I end up with something like:
Instead of:
Which is a bit odd because json.Marshal does not terminate with a newline character. I would have expected them both to do the same thing, just Marshal allocates a writer/buffer for you.
The text was updated successfully, but these errors were encountered: