{"action":"created","issue":{"url":"https://api.github.com/repos/lib/pq/issues/357","labels_url":"https://api.github.com/repos/lib/pq/issues/357/labels{/name}","comments_url":"https://api.github.com/repos/lib/pq/issues/357/comments","events_url":"https://api.github.com/repos/lib/pq/issues/357/events","html_url":"https://github.com/lib/pq/pull/357","id":65305357,"number":357,"title":"Add support for binary_mode","user":{"login":"johto","id":951744,"avatar_url":"https://avatars.githubusercontent.com/u/951744?v=3","gravatar_id":"","url":"https://api.github.com/users/johto","html_url":"https://github.com/johto","followers_url":"https://api.github.com/users/johto/followers","following_url":"https://api.github.com/users/johto/following{/other_user}","gists_url":"https://api.github.com/users/johto/gists{/gist_id}","starred_url":"https://api.github.com/users/johto/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/johto/subscriptions","organizations_url":"https://api.github.com/users/johto/orgs","repos_url":"https://api.github.com/users/johto/repos","events_url":"https://api.github.com/users/johto/events{/privacy}","received_events_url":"https://api.github.com/users/johto/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":23,"created_at":"2015-03-30T21:24:13Z","updated_at":"2015-05-06T08:22:08Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/lib/pq/pulls/357","html_url":"https://github.com/lib/pq/pull/357","diff_url":"https://github.com/lib/pq/pull/357.diff","patch_url":"https://github.com/lib/pq/pull/357.patch"},"body":"Here's my work so far towards making #209 happen.  It passes all tests with both binary_mode off and on, which suggests that we don't have enough tests.\r\n\r\nThis functionality is split into three separate commits:\r\n  1. Refactor writeBuf into something which can be used to send multiple messages in a single write() system call.  This should reduce the overhead a bit when there are a lot of threads writing at the same time.\r\n  2. Decoupling `rows` and `stmt` so that a `rows` object can live without having an associated `stmt` object.  Needed for the single round trip mode.\r\n  3. The implementation of binary_mode, where all []byte values are sent over and marked to be \"binary\", and everything else is sent over in text.  Something like this is necessary since if we want to do only a single round-trip to the server per a call to *sql.Query(), we don't know the SQL types of the input parameters.\r\n\r\nThere's still likely a lot of work to do:\r\n\r\n  1. There's no documentation\r\n  2. There are no additional tests\r\n  3. This might break some `driver.Value`rs which return a byte slice which isn't actually valid as input for the type in binary mode.  If we can adopt a general guideline of \"string means text, []byte means binary\" then things might work out great, but e.g. allocation penalties of such a guideline are not clear to me.  This has some impact on e.g. the ongoing work for supporting arrays.  Another approach might be to give up on binary mode altogether and instead always send everything over as text, but provide a special Valuer for bytea values (similarly to how we're likely going to have to have a separate values for bytea arrays).  Both approached would\r\n\r\nI'd really like to stress point 3 directly above; this is *not* the only way to achieve single roundtrip mode, and at the moment I'm not convinced it's the best one, either.  The performance gains from using binary mode have been in my tests almost negligible (though I've mostly tested with bytea values, since this patch doesn't use binary mode for e.g. ints even though it theoretically could), so don't get too hung up on that.\r\n\r\nAny thoughts?"},"comment":{"url":"https://api.github.com/repos/lib/pq/issues/comments/99374852","html_url":"https://github.com/lib/pq/pull/357#issuecomment-99374852","issue_url":"https://api.github.com/repos/lib/pq/issues/357","id":99374852,"user":{"login":"johto","id":951744,"avatar_url":"https://avatars.githubusercontent.com/u/951744?v=3","gravatar_id":"","url":"https://api.github.com/users/johto","html_url":"https://github.com/johto","followers_url":"https://api.github.com/users/johto/followers","following_url":"https://api.github.com/users/johto/following{/other_user}","gists_url":"https://api.github.com/users/johto/gists{/gist_id}","starred_url":"https://api.github.com/users/johto/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/johto/subscriptions","organizations_url":"https://api.github.com/users/johto/orgs","repos_url":"https://api.github.com/users/johto/repos","events_url":"https://api.github.com/users/johto/events{/privacy}","received_events_url":"https://api.github.com/users/johto/received_events","type":"User","site_admin":false},"created_at":"2015-05-06T08:22:08Z","updated_at":"2015-05-06T08:22:08Z","body":"Sorry, I had way less time available than I expected during last weekend, and I poured all of it into improving Listener and ListenerConn (and didn't finish even that).\r\n\r\n> @johto, any direction as to what I should focus on? if not I'll look and see what tests I can contribute.\r\n\r\nHmm.  I think the way this should work is to have a test-only environment variable which controls whether to use binary mode in tests or not, and then run the tests with binary_mode both on and off in Travis.   I think there was one test which didn't work with binary mode on, so that test would either have to be fixed to work with both (possibly not a good idea, I forget which exact test it was).  That should cover a lot of the new code.\r\n\r\nThen on top of that, a couple of tests for cases which only work with binary_parameters (forcing it on regardless of the env var), e.g. sending a binary []byte blob into a `SELECT $1::uuid`.  And perhaps some error conditions, such as `SELECT $1::int` with an 8-byte []byte.\r\n\r\nDoes that sound reasonable?"}}