{"action":"created","issue":{"url":"https://api.github.com/repos/cockroachdb/cockroach/issues/24573","repository_url":"https://api.github.com/repos/cockroachdb/cockroach","labels_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/24573/labels{/name}","comments_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/24573/comments","events_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/24573/events","html_url":"https://github.com/cockroachdb/cockroach/pull/24573","id":312112950,"number":24573,"title":"distsql: Refactor aggregator into a state machine","user":{"login":"abhimadan","id":7970201,"avatar_url":"https://avatars2.githubusercontent.com/u/7970201?v=4","gravatar_id":"","url":"https://api.github.com/users/abhimadan","html_url":"https://github.com/abhimadan","followers_url":"https://api.github.com/users/abhimadan/followers","following_url":"https://api.github.com/users/abhimadan/following{/other_user}","gists_url":"https://api.github.com/users/abhimadan/gists{/gist_id}","starred_url":"https://api.github.com/users/abhimadan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/abhimadan/subscriptions","organizations_url":"https://api.github.com/users/abhimadan/orgs","repos_url":"https://api.github.com/users/abhimadan/repos","events_url":"https://api.github.com/users/abhimadan/events{/privacy}","received_events_url":"https://api.github.com/users/abhimadan/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":5,"created_at":"2018-04-06T20:48:43Z","updated_at":"2018-04-09T18:03:20Z","closed_at":null,"author_association":"MEMBER","pull_request":{"url":"https://api.github.com/repos/cockroachdb/cockroach/pulls/24573","html_url":"https://github.com/cockroachdb/cockroach/pull/24573","diff_url":"https://github.com/cockroachdb/cockroach/pull/24573.diff","patch_url":"https://github.com/cockroachdb/cockroach/pull/24573.patch"},"body":"This lays the groundwork for extending the aggregator to stream results\r\nif some of the group columns have an ordering.\r\n\r\nRelease note: None\r\n\r\n----\r\n\r\nThis is the first commit from #24113, extracted into its own PR."},"comment":{"url":"https://api.github.com/repos/cockroachdb/cockroach/issues/comments/379841024","html_url":"https://github.com/cockroachdb/cockroach/pull/24573#issuecomment-379841024","issue_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/24573","id":379841024,"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":"2018-04-09T18:03:20Z","updated_at":"2018-04-09T18:03:20Z","author_association":"OWNER","body":"\n\n\n\nReview status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.\n\n---\n\n*[pkg/sql/distsqlrun/aggregator.go, line 96 at r2](https://beta.reviewable.io/reviews/cockroachdb/cockroach/24573#-L9fYB8j3qke-scQ4jNN:-L9fYB8j3qke-scQ4jNO:bogyjj9) ([raw file](https://github.com/cockroachdb/cockroach/blob/719da1370d1a37c3aa91b69022e83eaef181247e/pkg/sql/distsqlrun/aggregator.go#L96)):*\n> ```Go\n> \trowSourceBase\n> \n> \taccumulating aggregatorState\n> ```\n\nplease spinkle some comments here. How do these fields interact with each other and why are they function pointers? Do the values ever change? Why do these fields exist exactly?\n\n---\n\n*[pkg/sql/distsqlrun/aggregator.go, line 245 at r2](https://beta.reviewable.io/reviews/cockroachdb/cockroach/24573#-L9fYStm1GIoiFYU2wNJ:-L9fYStm1GIoiFYU2wNK:b18phqn) ([raw file](https://github.com/cockroachdb/cockroach/blob/719da1370d1a37c3aa91b69022e83eaef181247e/pkg/sql/distsqlrun/aggregator.go#L245)):*\n> ```Go\n> // accumulateRows continually reads rows from the input and accumulates them\n> // into intermediary aggregate results. If it encounters metadata, the metadata\n> // is immediately returned. Subsequent calls of this function will resume row\n> ```\n\nplease document the first ret val\n\n---\n\n*[pkg/sql/distsqlrun/aggregator.go, line 358 at r2](https://beta.reviewable.io/reviews/cockroachdb/cockroach/24573#-L9fZKph1RlhCVuixx-6:-L9fZKph1RlhCVuixx-7:b-frlolw) ([raw file](https://github.com/cockroachdb/cockroach/blob/719da1370d1a37c3aa91b69022e83eaef181247e/pkg/sql/distsqlrun/aggregator.go#L358)):*\n> ```Go\n> \n> \t\t// If row == nil and meta == nil, but we aren't done, we have a state\n> \t\t// transition without any output. As a result, we need to execute the next\n> ```\n\nthis comment is confusing because it talks about what the code doesn't do (return), instead of what it does. Consider rephrasing somehow.\n\n---\n\n\n*Comments from [Reviewable](https://beta.reviewable.io/reviews/cockroachdb/cockroach/24573)*\n<!-- Sent from Reviewable.io -->\n"}}
{"actor":{"display_login":"andreimatei"}}