{"action":"created","issue":{"url":"https://api.github.com/repos/npgsql/npgsql/issues/447","labels_url":"https://api.github.com/repos/npgsql/npgsql/issues/447/labels{/name}","comments_url":"https://api.github.com/repos/npgsql/npgsql/issues/447/comments","events_url":"https://api.github.com/repos/npgsql/npgsql/issues/447/events","html_url":"https://github.com/npgsql/npgsql/issues/447","id":53205767,"number":447,"title":"Switch to 100% binary and extended protocol everywhere?","user":{"login":"roji","id":1862641,"avatar_url":"https://avatars.githubusercontent.com/u/1862641?v=3","gravatar_id":"","url":"https://api.github.com/users/roji","html_url":"https://github.com/roji","followers_url":"https://api.github.com/users/roji/followers","following_url":"https://api.github.com/users/roji/following{/other_user}","gists_url":"https://api.github.com/users/roji/gists{/gist_id}","starred_url":"https://api.github.com/users/roji/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/roji/subscriptions","organizations_url":"https://api.github.com/users/roji/orgs","repos_url":"https://api.github.com/users/roji/repos","events_url":"https://api.github.com/users/roji/events{/privacy}","received_events_url":"https://api.github.com/users/roji/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"roji","id":1862641,"avatar_url":"https://avatars.githubusercontent.com/u/1862641?v=3","gravatar_id":"","url":"https://api.github.com/users/roji","html_url":"https://github.com/roji","followers_url":"https://api.github.com/users/roji/followers","following_url":"https://api.github.com/users/roji/following{/other_user}","gists_url":"https://api.github.com/users/roji/gists{/gist_id}","starred_url":"https://api.github.com/users/roji/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/roji/subscriptions","organizations_url":"https://api.github.com/users/roji/orgs","repos_url":"https://api.github.com/users/roji/repos","events_url":"https://api.github.com/users/roji/events{/privacy}","received_events_url":"https://api.github.com/users/roji/received_events","type":"User","site_admin":false},"milestone":{"url":"https://api.github.com/repos/npgsql/npgsql/milestones/3","labels_url":"https://api.github.com/repos/npgsql/npgsql/milestones/3/labels","id":620657,"number":3,"title":"3.0","description":"","creator":{"login":"franciscojunior","id":829924,"avatar_url":"https://avatars.githubusercontent.com/u/829924?v=3","gravatar_id":"","url":"https://api.github.com/users/franciscojunior","html_url":"https://github.com/franciscojunior","followers_url":"https://api.github.com/users/franciscojunior/followers","following_url":"https://api.github.com/users/franciscojunior/following{/other_user}","gists_url":"https://api.github.com/users/franciscojunior/gists{/gist_id}","starred_url":"https://api.github.com/users/franciscojunior/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/franciscojunior/subscriptions","organizations_url":"https://api.github.com/users/franciscojunior/orgs","repos_url":"https://api.github.com/users/franciscojunior/repos","events_url":"https://api.github.com/users/franciscojunior/events{/privacy}","received_events_url":"https://api.github.com/users/franciscojunior/received_events","type":"User","site_admin":false},"open_issues":37,"closed_issues":24,"state":"open","created_at":"2014-04-04T16:37:19Z","updated_at":"2015-01-13T20:17:40Z","due_on":null,"closed_at":null},"comments":49,"created_at":"2014-12-31T22:06:40Z","updated_at":"2015-01-15T15:15:32Z","closed_at":null,"body":"In #370 we discussed various aspects of switching to the extended query protocol, and also to binary encoding. @Emill and I discussed this again recently on our [gitter channel](https://gitter.im/npgsql/npgsql), and I wanted to get your opinions (@franciscojunior @jbcooley @glenebob). I'm sorry if revisiting this subject again is annoying.\r\n\r\nTo recall the situation, we can only request binary values using the extended query protocol, and we currently use extended query protocol only in prepared statements. In #370 we already discussed switching to the extended query protocol for non-prepared parameterized queries; this would remove a big chunk of complexity in client-side query rewriting, allow use of binary encoding when receiving values, etc. Once we do this we can also drop text encoding entirely for *sending* values to the database.\r\n\r\nSo after thinking about the whole thing some more, I now think that v3.0 should be a 100% binary-only, extended-query-only provider - I can see no real reasons for maintaining either text encoding or the simple protocol. The new read_refactor branch implements binary encoding for almost all PostgreSQL built-in types, including exotic types such as geometric and networking types, and the few types that haven't been binarized can be done for v3.0.\r\n\r\nNow, the main argument against going binary-only was unknown types - types defined by extensions are unknown to Npgsql so they're binary representation can't be decoded. With text encoding we can at least hand off the string to the user. To address this problem, we can allow a user to turn on a \"text-only\" flag on a command before executing it. This would make Npgsql request all results in text, and the user can access them as strings. The only drawback I can see is that one can't mix text and binary in the same query, so even simple ints have to be accessed as strings. The user has the option of either separating \"unknown-type queries\" from normal queries, or to deal with text representations of known types themselves - IMHO this is a very acceptable situation for the extremely rare unknown type problem (which will become even rarer as we implement support for more and more types).\r\n\r\nNote that as @franciscojunior suggested, the unknown type problem can be partially mitigated also by allowing external type handlers to be registered with Npgsql (although I think it's a bit early to actually allow that in 3.0).\r\n\r\nPros:\r\n* A transparent performance boost thanks to binary encoding. With the current situation, users have to be aware that binary encoding requires preparing the query, the fact that this entails two roundtrips... It's complicated and it involves users in the inner workings of both Npgsql and the PostgreSQL protocol. Making everything binary and extended-protocol makes the performance boost accessible to everyone in a transparent and easy way.\r\n* No more mucking around with locale settings which affect text representations (e.g. lc_monetary for money which causes #163, DateStyle=ISO...). Note that Npgsql users may want to work with PostgreSQL functions which depend on these settings, and Npgsql interferes with them to make text decoding possible.\r\n* No more loss of precision converting floating point values to and from text. Binary \"conversion\" is 100% accurate. (see #324).\r\n* Greatly simplifies our code base: text encoding and the simple protocol can simply be cut out, no more need to maintain two encoding paths for each type, etc. Npgsql becomes very lean and lightweight without losing any real functionality.\r\n\r\nCons:\r\n* Unknown (or text-only) types are a bit clumsy to use - they can't be easily mixed with known types (a query is either all-binary or all-text).\r\n* Multi-resultset queries aren't supported with the extended protocol. We'd have to fake them by sending multiple extended-protocol queries (but this can still be done in a single message, so only one roundtrip).\r\n* There was some mention of the extended protocol being less efficient than the simple protocol. We need to evaluate this with an actual performance test, but I also think the performance gains due to binary encoding should at the very least offset this.\r\n"},"comment":{"url":"https://api.github.com/repos/npgsql/npgsql/issues/comments/70100076","html_url":"https://github.com/npgsql/npgsql/issues/447#issuecomment-70100076","issue_url":"https://api.github.com/repos/npgsql/npgsql/issues/447","id":70100076,"user":{"login":"franciscojunior","id":829924,"avatar_url":"https://avatars.githubusercontent.com/u/829924?v=3","gravatar_id":"","url":"https://api.github.com/users/franciscojunior","html_url":"https://github.com/franciscojunior","followers_url":"https://api.github.com/users/franciscojunior/followers","following_url":"https://api.github.com/users/franciscojunior/following{/other_user}","gists_url":"https://api.github.com/users/franciscojunior/gists{/gist_id}","starred_url":"https://api.github.com/users/franciscojunior/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/franciscojunior/subscriptions","organizations_url":"https://api.github.com/users/franciscojunior/orgs","repos_url":"https://api.github.com/users/franciscojunior/repos","events_url":"https://api.github.com/users/franciscojunior/events{/privacy}","received_events_url":"https://api.github.com/users/franciscojunior/received_events","type":"User","site_admin":false},"created_at":"2015-01-15T15:15:32Z","updated_at":"2015-01-15T15:15:32Z","body":"While playing with the latest master code, I think I found a little bug:\r\n\r\nWhere there is:\r\n```\r\nvar asSimpleWriter = Handler as ISimpleTypeWriter;\r\n                    if (asSimpleWriter != null)\r\n                    {\r\n                        BoundSize = ((ISimpleTypeWriter)Handler).GetLength(Value);\r\n                    }\r\n                    throw PGUtil.ThrowIfReached(String.Format(\"Handler {0} doesn't implement a known writer interface\", Handler));\r\n```\r\n\r\nI think it should be:\r\n```\r\nvar asSimpleWriter = Handler as ISimpleTypeWriter;\r\n                    if (asSimpleWriter != null)\r\n                    {\r\n                        BoundSize = ((ISimpleTypeWriter)Handler).GetLength(Value);\r\n                    }\r\n                    else\r\n                        throw PGUtil.ThrowIfReached(String.Format(\"Handler {0} doesn't implement a known writer interface\", Handler));\r\n```\r\n\r\nI think it is missing an `else` clause. This was giving errors when running tests. "}}