{"action":"created","issue":{"url":"https://api.github.com/repos/cockroachdb/cockroach/issues/25154","repository_url":"https://api.github.com/repos/cockroachdb/cockroach","labels_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/25154/labels{/name}","comments_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/25154/comments","events_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/25154/events","html_url":"https://github.com/cockroachdb/cockroach/pull/25154","id":318727021,"number":25154,"title":"opt: add WITH ORDINALITY via a RowNumber operator","user":{"login":"justinj","id":409075,"avatar_url":"https://avatars2.githubusercontent.com/u/409075?v=4","gravatar_id":"","url":"https://api.github.com/users/justinj","html_url":"https://github.com/justinj","followers_url":"https://api.github.com/users/justinj/followers","following_url":"https://api.github.com/users/justinj/following{/other_user}","gists_url":"https://api.github.com/users/justinj/gists{/gist_id}","starred_url":"https://api.github.com/users/justinj/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/justinj/subscriptions","organizations_url":"https://api.github.com/users/justinj/orgs","repos_url":"https://api.github.com/users/justinj/repos","events_url":"https://api.github.com/users/justinj/events{/privacy}","received_events_url":"https://api.github.com/users/justinj/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":11,"created_at":"2018-04-29T15:45:32Z","updated_at":"2018-05-01T18:51:14Z","closed_at":null,"author_association":"OWNER","pull_request":{"url":"https://api.github.com/repos/cockroachdb/cockroach/pulls/25154","html_url":"https://github.com/cockroachdb/cockroach/pull/25154","diff_url":"https://github.com/cockroachdb/cockroach/pull/25154.diff","patch_url":"https://github.com/cockroachdb/cockroach/pull/25154.patch"},"body":"WITH ORDINALITY is has the behaviour of a special case of the window\r\nfunction RANK with the empty set of partitioning columns. This commit\r\nleaves most of window functions unimplemented, only implementing enough\r\nto do WITH ORDINALITY.\r\n\r\nWITH ORDINALITY introduces a new column `ordinality` to the input set,\r\nwhose value corresponds to a given row's position in that input set.\r\nThe semantics of the ordering are briefly discussed at\r\nhttps://www.cockroachlabs.com/docs/stable/query-order.html#order-preservation.\r\n\r\nThis commit introduces window functions in a very limited form - only as\r\nrequired for WITH ORDINALITY, so no partitioning or window functions\r\nbesides ROW_NUMBER() are supported. In addition, a given window function\r\ncan only have a single windowing operator.\r\n\r\nRelease note: None"},"comment":{"url":"https://api.github.com/repos/cockroachdb/cockroach/issues/comments/385755422","html_url":"https://github.com/cockroachdb/cockroach/pull/25154#issuecomment-385755422","issue_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/25154","id":385755422,"user":{"login":"rytaft","id":3288732,"avatar_url":"https://avatars2.githubusercontent.com/u/3288732?v=4","gravatar_id":"","url":"https://api.github.com/users/rytaft","html_url":"https://github.com/rytaft","followers_url":"https://api.github.com/users/rytaft/followers","following_url":"https://api.github.com/users/rytaft/following{/other_user}","gists_url":"https://api.github.com/users/rytaft/gists{/gist_id}","starred_url":"https://api.github.com/users/rytaft/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rytaft/subscriptions","organizations_url":"https://api.github.com/users/rytaft/orgs","repos_url":"https://api.github.com/users/rytaft/repos","events_url":"https://api.github.com/users/rytaft/events{/privacy}","received_events_url":"https://api.github.com/users/rytaft/received_events","type":"User","site_admin":false},"created_at":"2018-05-01T18:51:14Z","updated_at":"2018-05-01T18:51:14Z","author_association":"CONTRIBUTOR","body":"\n\n\n\nReviewed 18 of 19 files at r4.\nReview status: all files reviewed at latest revision, 18 unresolved discussions, some commit checks failed.\n\n---\n\n*[pkg/sql/opt/exec/execbuilder/testdata/ordinality, line 28 at r3](https://reviewable.io/reviews/cockroachdb/cockroach/25154#-LBNBNaJ3U33Nj3l2M0v:-LBRyY7FEwOpq_-htDE4:b-ihj7cu) ([raw file](https://github.com/cockroachdb/cockroach/blob/3751418279112d12807933ea8ac11194c5b317b0/pkg/sql/opt/exec/execbuilder/testdata/ordinality#L28)):*\n<details><summary><i>Previously, justinj (Justin Jaffray) wrote…</i></summary><blockquote>\n\nDone.\n</blockquote></details>\n\ncan use the `hide-colnames` directive after `exec` if you don't want to see the column names at the top\n\n---\n\n*[pkg/sql/opt/memo/statistics_builder.go, line 983 at r4](https://reviewable.io/reviews/cockroachdb/cockroach/25154#-LBRz3SW9Xo8nFxOz683:-LBRz3SW9Xo8nFxOz684:bogkmx9) ([raw file](https://github.com/cockroachdb/cockroach/blob/fc4556f83bd5cd6087b4d5bb26454b37a707dd69/pkg/sql/opt/memo/statistics_builder.go#L983)):*\n> ```Go\n> \n> func (sb *statisticsBuilder) buildRowNumber(inputStats *opt.Statistics) {\n> \t*sb.s = *inputStats\n> ```\n\nI think you should just set the row count -- I think we want to avoid reusing the column stats maps across operators\n\n---\n\n*[pkg/sql/opt/memo/statistics_builder.go, line 998 at r4](https://reviewable.io/reviews/cockroachdb/cockroach/25154#-LBS-sIs5a8hJoN4XnEj:-LBS-sIs5a8hJoN4XnEk:bdrckxw) ([raw file](https://github.com/cockroachdb/cockroach/blob/fc4556f83bd5cd6087b4d5bb26454b37a707dd69/pkg/sql/opt/memo/statistics_builder.go#L998)):*\n> ```Go\n> \tif colSet.Contains(int(def.ColID)) {\n> \t\t// The ordinality column is a key, so every row is distinct.\n> \t\tcolStat.DistinctCount = uint64(sb.ev.Logical().Relational.Stats.RowCount)\n> ```\n\nI think the row count is already a uint64 - this will probably give you a linter error\n\n---\n\n*[pkg/sql/opt/memo/testdata/stats/ordinality, line 28 at r4](https://reviewable.io/reviews/cockroachdb/cockroach/25154#-LBS0YbVBCChKdU4m6wu:-LBS0YbVBCChKdU4m6wv:b-z5b5m7) ([raw file](https://github.com/cockroachdb/cockroach/blob/fc4556f83bd5cd6087b4d5bb26454b37a707dd69/pkg/sql/opt/memo/testdata/stats/ordinality#L28)):*\n> ```\n> \n> build\n> SELECT * FROM (SELECT * FROM a WITH ORDINALITY) WHERE ordinality > 0 AND ordinality <= 10\n> ```\n\nThis is good, but can you also add a similar test with a predicate like `WHERE y > 0 AND y <= 10` so that you test that you're correctly retrieving the stats from the input?\n\n---\n\n\n*Comments from [Reviewable](https://reviewable.io/reviews/cockroachdb/cockroach/25154)*\n<!-- Sent from Reviewable.io -->\n"}}
{"actor":{"display_login":"rytaft"}}