{"action":"created","issue":{"url":"https://api.github.com/repos/scylladb/scylla/issues/1495","repository_url":"https://api.github.com/repos/scylladb/scylla","labels_url":"https://api.github.com/repos/scylladb/scylla/issues/1495/labels{/name}","comments_url":"https://api.github.com/repos/scylladb/scylla/issues/1495/comments","events_url":"https://api.github.com/repos/scylladb/scylla/issues/1495/events","html_url":"https://github.com/scylladb/scylla/issues/1495","id":167257561,"number":1495,"title":"Column value incorrectly identified as static with compact storage","user":{"login":"duarten","id":501746,"avatar_url":"https://avatars.githubusercontent.com/u/501746?v=3","gravatar_id":"","url":"https://api.github.com/users/duarten","html_url":"https://github.com/duarten","followers_url":"https://api.github.com/users/duarten/followers","following_url":"https://api.github.com/users/duarten/following{/other_user}","gists_url":"https://api.github.com/users/duarten/gists{/gist_id}","starred_url":"https://api.github.com/users/duarten/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/duarten/subscriptions","organizations_url":"https://api.github.com/users/duarten/orgs","repos_url":"https://api.github.com/users/duarten/repos","events_url":"https://api.github.com/users/duarten/events{/privacy}","received_events_url":"https://api.github.com/users/duarten/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":2,"created_at":"2016-07-24T22:17:16Z","updated_at":"2016-07-26T12:43:06Z","closed_at":null,"body":"*Installation details*\r\nScylla version (or git commit hash): 2f4cd86\r\n\r\nConsider the following table:\r\n\r\n```\r\ncqlsh> create keyspace ks1 with replication = { 'class' : 'SimpleStrategy', 'replication_factor' : 1 };\r\ncqlsh> create table ks1.test(p blob, c blob, v blob, primary key (p, c)) with compact storage;\r\ncqlsh> insert into ks1.test(p, c, v) values(0x0000000000000003, 0xfffffffffffeca, 0x);\r\ncqlsh> select * from ks1.test;\r\n\r\n p                  | c                | v\r\n--------------------+------------------+----\r\n 0x0000000000000003 | 0xfffffffffffeca | 0x\r\n\r\n(1 rows)\r\n```\r\n\r\nIf we now reboot Scylla, we get the following behaviour when reading the value from its sstable:\r\n\r\n```\r\ncqlsh> select * from ks1.test;\r\nReadTimeout: code=1200 [Coordinator node timed out waiting for replica nodes' responses] message=\"Operation timed out for ks1.test - received only 0 responses from 1 CL=ONE.\" info={'received_responses': 0, 'required_responses': 1, 'consistency': 'ONE'}\r\n```\r\n\r\nIn Scylla's log:\r\n\r\n```\r\nERROR 2016-07-24 18:29:48,569 [shard 1] storage_proxy - Exception when communicating with 127.0.0.1: Static row has clustering key information. I didn't expect that!\r\n```\r\n\r\nThis is the relevant code:\r\n\r\n```c++\r\nstatic bool check_static(bytes_view col) {\r\n    static bytes static_row(static_size, 0xff);\r\n    return col.compare(0, static_size, static_row) == 0;\r\n}\r\n\r\ncolumn(const schema& schema, bytes_view col)\r\n    : is_static(check_static(col))\r\n    , col_name(fix_static_name(col))\r\n    , clustering(extract_clustering_key(schema))\r\n    , collection_extra_data(is_collection(schema) ? pop_back(clustering) : bytes()) // collections are not supported with COMPACT STORAGE, so this is fine\r\n    , cell(!schema.is_dense() ? pop_back(clustering) : (*(schema.regular_begin())).name()) // dense: cell name is not provided. It is the only regular column\r\n    , cdef(schema.get_column_definition(cell))\r\n{\r\n\r\n    if (is_static) {\r\n        for (auto& e: clustering) {\r\n            if (e.size() != 0) {\r\n                throw malformed_sstable_exception(\"Static row has clustering key information. I didn't expect that!\");\r\n            }\r\n        }\r\n    }\r\n}\r\n```\r\n\r\nIt feels like check_static should take into account a) the schema's compoundness if the schema.is_dense(), or b) the regular column information, as in:\r\n\r\n```c++\r\nstatic bool check_static(const schema& s, bytes_view col) {\r\n    static bytes static_row(static_size, 0xff);\r\n    return ((s.is_dense() && s.is_compound()) || (cdef && cdef.is_static())) && col.compare(0, static_size, static_row) == 0;\r\n}\r\n```"},"comment":{"url":"https://api.github.com/repos/scylladb/scylla/issues/comments/235255622","html_url":"https://github.com/scylladb/scylla/issues/1495#issuecomment-235255622","issue_url":"https://api.github.com/repos/scylladb/scylla/issues/1495","id":235255622,"user":{"login":"tgrabiec","id":283695,"avatar_url":"https://avatars.githubusercontent.com/u/283695?v=3","gravatar_id":"","url":"https://api.github.com/users/tgrabiec","html_url":"https://github.com/tgrabiec","followers_url":"https://api.github.com/users/tgrabiec/followers","following_url":"https://api.github.com/users/tgrabiec/following{/other_user}","gists_url":"https://api.github.com/users/tgrabiec/gists{/gist_id}","starred_url":"https://api.github.com/users/tgrabiec/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tgrabiec/subscriptions","organizations_url":"https://api.github.com/users/tgrabiec/orgs","repos_url":"https://api.github.com/users/tgrabiec/repos","events_url":"https://api.github.com/users/tgrabiec/events{/privacy}","received_events_url":"https://api.github.com/users/tgrabiec/received_events","type":"User","site_admin":false},"created_at":"2016-07-26T12:43:06Z","updated_at":"2016-07-26T12:43:06Z","body":"@duarten You're right, the code incorrectly assumes that cell names are always compound. It should check `s.thrift().has_compound_comparator`."}}
{"actor":{"display_login":"tgrabiec"}}