{"action":"created","issue":{"url":"https://api.github.com/repos/cockroachdb/cockroach/issues/18893","repository_url":"https://api.github.com/repos/cockroachdb/cockroach","labels_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/18893/labels{/name}","comments_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/18893/comments","events_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/18893/events","html_url":"https://github.com/cockroachdb/cockroach/pull/18893","id":261420849,"number":18893,"title":"sql: fix handling of errors on Commit and Rollback ","user":{"login":"andreimatei","id":377201,"avatar_url":"https://avatars1.githubusercontent.com/u/377201?v=4","gravatar_id":"","url":"https://api.github.com/users/andreimatei","html_url":"https://github.com/andreimatei","followers_url":"https://api.github.com/users/andreimatei/followers","following_url":"https://api.github.com/users/andreimatei/following{/other_user}","gists_url":"https://api.github.com/users/andreimatei/gists{/gist_id}","starred_url":"https://api.github.com/users/andreimatei/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/andreimatei/subscriptions","organizations_url":"https://api.github.com/users/andreimatei/orgs","repos_url":"https://api.github.com/users/andreimatei/repos","events_url":"https://api.github.com/users/andreimatei/events{/privacy}","received_events_url":"https://api.github.com/users/andreimatei/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":{"login":"jordanlewis","id":43821,"avatar_url":"https://avatars1.githubusercontent.com/u/43821?v=4","gravatar_id":"","url":"https://api.github.com/users/jordanlewis","html_url":"https://github.com/jordanlewis","followers_url":"https://api.github.com/users/jordanlewis/followers","following_url":"https://api.github.com/users/jordanlewis/following{/other_user}","gists_url":"https://api.github.com/users/jordanlewis/gists{/gist_id}","starred_url":"https://api.github.com/users/jordanlewis/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jordanlewis/subscriptions","organizations_url":"https://api.github.com/users/jordanlewis/orgs","repos_url":"https://api.github.com/users/jordanlewis/repos","events_url":"https://api.github.com/users/jordanlewis/events{/privacy}","received_events_url":"https://api.github.com/users/jordanlewis/received_events","type":"User","site_admin":false},"assignees":[{"login":"jordanlewis","id":43821,"avatar_url":"https://avatars1.githubusercontent.com/u/43821?v=4","gravatar_id":"","url":"https://api.github.com/users/jordanlewis","html_url":"https://github.com/jordanlewis","followers_url":"https://api.github.com/users/jordanlewis/followers","following_url":"https://api.github.com/users/jordanlewis/following{/other_user}","gists_url":"https://api.github.com/users/jordanlewis/gists{/gist_id}","starred_url":"https://api.github.com/users/jordanlewis/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jordanlewis/subscriptions","organizations_url":"https://api.github.com/users/jordanlewis/orgs","repos_url":"https://api.github.com/users/jordanlewis/repos","events_url":"https://api.github.com/users/jordanlewis/events{/privacy}","received_events_url":"https://api.github.com/users/jordanlewis/received_events","type":"User","site_admin":false},{"login":"knz","id":642886,"avatar_url":"https://avatars3.githubusercontent.com/u/642886?v=4","gravatar_id":"","url":"https://api.github.com/users/knz","html_url":"https://github.com/knz","followers_url":"https://api.github.com/users/knz/followers","following_url":"https://api.github.com/users/knz/following{/other_user}","gists_url":"https://api.github.com/users/knz/gists{/gist_id}","starred_url":"https://api.github.com/users/knz/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/knz/subscriptions","organizations_url":"https://api.github.com/users/knz/orgs","repos_url":"https://api.github.com/users/knz/repos","events_url":"https://api.github.com/users/knz/events{/privacy}","received_events_url":"https://api.github.com/users/knz/received_events","type":"User","site_admin":false}],"milestone":null,"comments":11,"created_at":"2017-09-28T18:43:27Z","updated_at":"2017-10-02T15:42:23Z","closed_at":"2017-10-02T15:32:43Z","author_association":"OWNER","pull_request":{"url":"https://api.github.com/repos/cockroachdb/cockroach/pulls/18893","html_url":"https://github.com/cockroachdb/cockroach/pull/18893","diff_url":"https://github.com/cockroachdb/cockroach/pull/18893.diff","patch_url":"https://github.com/cockroachdb/cockroach/pull/18893.patch"},"body":"Fixes #18853\r\n\r\nEver since we've introduced results streaming, there's been great\r\nambiguity about the handling of errors that happen on commit and\r\nrollback statements. The execution of these statements didn't use to\r\nreturn any errors ever, but that has changed because result streaming\r\nintroduced the possibility of communication error happening during\r\nstatement execution, and these errors can never be swallowed (and so we\r\nalso stopped swallowing other errors at the time).\r\nBecause COMMIT and ROLLBACK do their own state transitions, the fact\r\nthat they started returning an error confused a higher layer\r\n(execStmtInOpenTxn()) which also does state transitions when it sees\r\nerrors. That crashed.\r\n\r\nThis patch resolves the problem by establishing a protocol for state\r\ntransitions such that particular statements can coordinate with\r\nexecStmtInOpenTxn() around the needed transitions. COMMIT and ROLLBACK\r\nuse this mechanism to inform the higher layers that they have sometimes\r\ntaken care of all transitions.\r\nMorally, this change is in the spirit of envisioned future changes to\r\nthe session state machine: state transitions should/will be centralized\r\ninto one level of the code, not sprinkled everywhere."},"comment":{"url":"https://api.github.com/repos/cockroachdb/cockroach/issues/comments/333574226","html_url":"https://github.com/cockroachdb/cockroach/pull/18893#issuecomment-333574226","issue_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/18893","id":333574226,"user":{"login":"andreimatei","id":377201,"avatar_url":"https://avatars1.githubusercontent.com/u/377201?v=4","gravatar_id":"","url":"https://api.github.com/users/andreimatei","html_url":"https://github.com/andreimatei","followers_url":"https://api.github.com/users/andreimatei/followers","following_url":"https://api.github.com/users/andreimatei/following{/other_user}","gists_url":"https://api.github.com/users/andreimatei/gists{/gist_id}","starred_url":"https://api.github.com/users/andreimatei/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/andreimatei/subscriptions","organizations_url":"https://api.github.com/users/andreimatei/orgs","repos_url":"https://api.github.com/users/andreimatei/repos","events_url":"https://api.github.com/users/andreimatei/events{/privacy}","received_events_url":"https://api.github.com/users/andreimatei/received_events","type":"User","site_admin":false},"created_at":"2017-10-02T15:42:23Z","updated_at":"2017-10-02T15:42:23Z","author_association":"OWNER","body":"\n\n\n\nReview status: 1 of 2 files reviewed at latest revision, 3 unresolved discussions, all commit checks successful.\n\n---\n\n*[pkg/sql/session_test.go, line 246 at r4](https://reviewable.io:443/reviews/cockroachdb/cockroach/18893#-Kv9eMLWxjGzSCW9mtsy:-KvSjebyww334KmYyZu3:bkdyshn) ([raw file](https://github.com/cockroachdb/cockroach/blob/6c9caad418bf1290373a179b1c3d415152536265/pkg/sql/session_test.go#L246)):*\n<details><summary><i>Previously, knz (kena) wrote…</i></summary><blockquote>\n\nIt's fine if you don't want to extend the comment in *this file*.\n\nHowever leaving this piece of knowledge to rot in a reviewable comment is **not** an option. Please find *some* good place to write this information as a comment in the source base. It might go well in `executor.go` next to the handling of commit/abort.\n</blockquote></details>\n\nWhat precisely is the \"piece of knowledge\" that you find so valuable? The way I see it, there's no particular surprise here.\n\n---\n\n\n*Comments from [Reviewable](https://reviewable.io:443/reviews/cockroachdb/cockroach/18893)*\n<!-- Sent from Reviewable.io -->\n"}}
{"actor":{"display_login":"andreimatei"}}