{"action":"created","issue":{"url":"https://api.github.com/repos/cockroachdb/cockroach/issues/14528","repository_url":"https://api.github.com/repos/cockroachdb/cockroach","labels_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/14528/labels{/name}","comments_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/14528/comments","events_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/14528/events","html_url":"https://github.com/cockroachdb/cockroach/pull/14528","id":218623734,"number":14528,"title":"sql: use column IDs in composite secondary indexes","user":{"login":"mjibson","id":41181,"avatar_url":"https://avatars2.githubusercontent.com/u/41181?v=3","gravatar_id":"","url":"https://api.github.com/users/mjibson","html_url":"https://github.com/mjibson","followers_url":"https://api.github.com/users/mjibson/followers","following_url":"https://api.github.com/users/mjibson/following{/other_user}","gists_url":"https://api.github.com/users/mjibson/gists{/gist_id}","starred_url":"https://api.github.com/users/mjibson/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mjibson/subscriptions","organizations_url":"https://api.github.com/users/mjibson/orgs","repos_url":"https://api.github.com/users/mjibson/repos","events_url":"https://api.github.com/users/mjibson/events{/privacy}","received_events_url":"https://api.github.com/users/mjibson/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":9,"created_at":"2017-03-31T21:38:45Z","updated_at":"2017-04-03T18:47:43Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/cockroachdb/cockroach/pulls/14528","html_url":"https://github.com/cockroachdb/cockroach/pull/14528","diff_url":"https://github.com/cockroachdb/cockroach/pull/14528.diff","patch_url":"https://github.com/cockroachdb/cockroach/pull/14528.patch"},"body":"Change composite index encoding to always be preceeded by the column\r\nID diff. This allows both primary and secondary keys to use the same\r\ndecode path.\r\n\r\nSince the column IDs are now known, datums can now also signal if\r\nthey need a composite encoding or not, since for many cases the key\r\nencoding is reversible. Floats and decimals only do in the -0 case\r\nor when zeros are truncated during key encoding. Collated strings\r\nare not reversible and always need a composite encoding.\r\n\r\nFloat encodings have changed so 0 and -0 have the same key encodings\r\nto allow for correct semantics during index operations. Floats have\r\nbeen added to composite encodings so -0 can round trip in an index.\r\n\r\nThis is a backward-incompatible change. Any secondary indexes over\r\ncollated strings need to be dropped and recreated. They can be\r\ndropped before or after upgrading, but must be recreated on the new\r\nversion. Failing to do this will result in an error: \"pq: unknown\r\ntype NotNull\" during a query.\r\n\r\nFixes #14345\r\nSee #14328\r\nSee 743842997a428cfae6805df114abf075d3b8129d\r\n\r\n<!-- Reviewable:start -->\r\n---\r\nThis change is [<img src=\"https://reviewable.io/review_button.svg\" height=\"34\" align=\"absmiddle\" alt=\"Reviewable\"/>](https://reviewable.io/reviews/cockroachdb/cockroach/14528)\r\n<!-- Reviewable:end -->\r\n"},"comment":{"url":"https://api.github.com/repos/cockroachdb/cockroach/issues/comments/291236778","html_url":"https://github.com/cockroachdb/cockroach/pull/14528#issuecomment-291236778","issue_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/14528","id":291236778,"user":{"login":"eisenstatdavid","id":3858570,"avatar_url":"https://avatars0.githubusercontent.com/u/3858570?v=3","gravatar_id":"","url":"https://api.github.com/users/eisenstatdavid","html_url":"https://github.com/eisenstatdavid","followers_url":"https://api.github.com/users/eisenstatdavid/followers","following_url":"https://api.github.com/users/eisenstatdavid/following{/other_user}","gists_url":"https://api.github.com/users/eisenstatdavid/gists{/gist_id}","starred_url":"https://api.github.com/users/eisenstatdavid/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/eisenstatdavid/subscriptions","organizations_url":"https://api.github.com/users/eisenstatdavid/orgs","repos_url":"https://api.github.com/users/eisenstatdavid/repos","events_url":"https://api.github.com/users/eisenstatdavid/events{/privacy}","received_events_url":"https://api.github.com/users/eisenstatdavid/received_events","type":"User","site_admin":false},"created_at":"2017-04-03T18:47:43Z","updated_at":"2017-04-03T18:47:43Z","body":"<img class=\"emoji\" title=\":lgtm:\" alt=\":lgtm:\" align=\"absmiddle\" src=\"https://reviewable.io/lgtm.png\" height=\"20\" width=\"61\"/>\n\n---\n\nReviewed 4 of 4 files at r3.\nReview status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks pending.\n\n---\n\n*[pkg/sql/parser/datum.go, line 493 at r2](https://reviewable.io:443/reviews/cockroachdb/cockroach/14528#-KgoJEW46q9QfeUbKo8O:-Kgp5sr7DstxMglUh6n1:b-vro9h6) ([raw file](https://github.com/cockroachdb/cockroach/blob/c43fe50d83a58c3d244a4d2f2a3f52f8b8c8c19c/pkg/sql/parser/datum.go#L493)):*\n<details><summary><i>Previously, mjibson (Matt Jibson) wrote…</i></summary><blockquote>\n\nThat's a copy of the implementation in Signbit, but it's less obvious what exactly it's doing. Especially with the go 1.9 inlining improvements I'd prefer to keep it as is because it should get compiled down to this same thing, but we keep the benefit of using a stable API.\n</blockquote></details>\n\nIt's not just signbit; `return math.Float64bits(*d) == 1<<63` (or equivalently, `math.Float64bits(*d) == math.Float64bits(math.Copysign(0, -1))`) subsumes the comparison to zero as well, so I'm skeptical that the inliner would clean this up. IEEE 754 format and `math.Float64bits` are stable APIs.\n\nI'm fine with the code as is though.\n\n---\n\n\n*Comments from [Reviewable](https://reviewable.io:443/reviews/cockroachdb/cockroach/14528#-:-Kgp8BmIRSJPjA-dbdA8:bnfp4nl)*\n<!-- Sent from Reviewable.io -->\n"}}
{"actor":{"display_login":"eisenstatdavid"}}