First steps towards implementing the PostgreSQL protocol. #2491
Conversation
| } | ||
| // size includes itself. | ||
| size -= 4 | ||
| data := make([]byte, size) |
There was a problem hiding this comment.
Should we perform sanity checking on the value of size?
There was a problem hiding this comment.
Maybe, but what limit would we want? We don't have hard limits anywhere else (including in our rpc server). We can at least check that it's not negative.
There was a problem hiding this comment.
We limit the size of keys and values at the KV layer. Perhaps some multiple of that.
|
LGTM |
Only supports simple queries with no placeholders. No support yet for authentication or transactions.
| err = c.handleParse(buf) | ||
|
|
||
| default: | ||
| log.Fatalf("unrecognized client message type %c", typ) |
There was a problem hiding this comment.
Should this be a Fatal? Seems that it would crash the server based on a malformed client message, unless the resulting panic is being caught somewhere.
There was a problem hiding this comment.
Fatal doesn't panic, I believe it just calls os.Exit(something).
On Sep 14, 2015 2:01 PM, "mrtracy" notifications@github.com wrote:
In sql/pgwire/v3.go
#2491 (comment):
return err}typ, msg, err := readTypedMsg(rd)if err != nil {return err}buf := bytes.NewBuffer(msg)switch typ {case clientMsgSimpleQuery:err = c.handleSimpleQuery(buf)
case clientMsgParse:err = c.handleParse(buf)
default:log.Fatalf("unrecognized client message type %c", typ)Should this be a Fatal? Seems that it would crash the server based on a
malformed client message, unless the resulting panic is being caught
somewhere.—
Reply to this email directly or view it on GitHub
https://github.com/cockroachdb/cockroach/pull/2491/files#r39425437.
|
Could you put a link to the spec somewhere in there? maybe a barebones readme? |
|
LGTM. nice. |
|
Closing this since the work has been picked up by @petermattis in https://github.com/cockroachdb/cockroach/tree/pmattis/pgwire |
Only supports simple queries with no placeholders. No support yet for
authentication or transactions.