Decoding and Encoding Enhancements & Fixes #6
Conversation
Several types still do not work for decoding; 106, 107, 108, 109, 110, 111. One small breaking change: KU, KV, KT (minute, second, timespan) have all been reverted to integers, as negatives were being stored (and consequently encoded) incorrectly.
Simply call kdb.ListenAndServer(":12345", kdb.EchoHandler)
This allows for getting raw byte data without sending it over a connection.
|
Thanks for contributing this! |
|
would it be easy for you to break it down to several changes? i.e. breaking changes/server/other |
| case KDYNLOAD: | ||
| // 112 - dynamic load | ||
| return nil, errors.New("type is unsupported") | ||
| _, _ = readData(r, order) | ||
| return nil, UnsupportedType |
There was a problem hiding this comment.
it is not possible to send 112 over ipc and there is no need to do readData - it is likely mistake somewhere else. Should be more of 'protocol error'
| case KEACH, KOVER, KSCAN, KPRIOR, KEACHRIGHT, KEACHLEFT: | ||
| return readData(r, order) | ||
| _, _ = readData(r, order) | ||
| return nil, UnsupportedType |
There was a problem hiding this comment.
this is not really an error and decoder should keep going. it does need wrapping to K struct though
&K{msgtype,NONE,readData(r,order)} (except readData should be handled separately
| ) | ||
|
|
||
| // Listen and serve client requests | ||
| func ListenAndServe(addr string, handler func(*K, KDBConn) error) error { |
There was a problem hiding this comment.
this is unfortunate name is this might imply net/http.ListenAndServe support via composition. If your intention is doing rpc style here, maybe modelling https://golang.org/pkg/net/rpc/#ServeCodec is a way to go.
There was a problem hiding this comment.
I modeled it after http.ListenAndServe to make it easy to see what it's doing.
It's just some boiler plate code that compiles as is, but would need additional work to make it useful. Adding in some function multiplexing would make it look a lot like the http version.
I can change the naming or get rid of it completely to remove some confusion.
| return &header, nil | ||
| } | ||
|
|
||
| func DecodeRaw(src *bufio.Reader) ([]byte, *ipcHeader, error) { |
There was a problem hiding this comment.
is it for decoding from disk/files? Do you have example for this?
There was a problem hiding this comment.
Specifically, I used it to bypass all parsing (apart from the header) and have the code pass-through all data while incurring minimal overhead.
This comment was in direct reference to this commit:
I'm not sure how people feel about commit 5975019; it adds in the functionality to encode and decode directly to/from bytes without having a connection open. I found this functionality useful, but it might be too use-case specific.
Additionally, you can turn a byte slice into a bufio.Reader using: bufio.NewReader(bytes.NewReader(byteSlice))
| @@ -73,6 +73,8 @@ func TestCharArray(t *testing.T) { | |||
| var TimestampAsBytes = []byte{0x01, 0x00, 0x00, 0x00, 0x11, 0x00, 0x00, 0x00, 0xf4, 0x28, 0xbf, 0xce, 0x27, 0x35, 0xec, 0xe9, 0x07} | |||
| var TimestampAsTime = time.Date(2018, 1, 26, 1, 49, 0, 884361000, time.UTC) | |||
| var TimestampAsInt64 = int64(570246540884361000) | |||
| var DatetimeAsTime = time.Date(2013, 6, 10, 22, 03, 49, 713000000, time.UTC) | |||
| var DateAsTime = time.Date(2013, 6, 10, 0, 0, 0, 00000000, time.UTC) | |||
There was a problem hiding this comment.
could you add couple of examples with negative data/datetime/timestamp where it breaks?
There was a problem hiding this comment.
The issue with negatives I was referring to was regarding datatypes U, V, and T.
Each one is treated as a duration of time by the q interpreter but the decode function was turning them into a time object.
So, for example, sending -01:00 will result in it being decoded as 23:00
|
Do you want to split this into smaller pieces, so we can merge one by one? |
Some commits have additional notes attached to them.
Breaking changes:
KU, KV, KT were changed to integers from time objects, as this was not handling negative values correctly. -KU, -KV, -KT never had this functionality and are unchanged.
All datatypes now work, minus except KEACH, KOVER, KSCAN, KPRIOR, KEACHRIGHT, KEACHLEFT, KDYNLOAD (106 through 112).
I'm not sure how people feel about commit 5975019; it adds in the functionality to encode and decode directly to/from bytes without having a connection open. I found this functionality useful, but it might be too use-case specific.