mirror of
https://github.com/scylladb/scylladb.git
synced 2026-05-28 10:41:12 +00:00
bc4d0fd5ad
Traditionally in Scylla and in Cassandra, an empty partition key is mapped to minimum_token() instead of the empty key's usual hash function (0). The reasons for this are unknown (to me), but one possibility is that having one known key that maps to the minimal token is useful for various iterations. In murmur3_partitioner.cc we have two variants of the token calculation function - the first is get_token(bytes_view) and the second is get_token(schema, partition_key_view). The first includes that empty- key special case, but the second was missing this special case! As Kamil first noted in #9352, the second variant is used when looking up partitions in the index file - so if a partition with an empty-string key is saved under one token, it will be looked up under a different token and not found. I reproduced exactly this problem when fixing issues #9364 and #9375 (empty-string keys in materialized views and indexes) - where a partition with an empty key was visible in a full-table scan but couldn't be found by looking up its key because of the wrong index lookup. I also tried an alternative fix - changing both implementations to return minimum_token (and not 0) for the empty key. But this is undesirable - minimum_token is not supposed to be a valid token, so the tokenizer and sharder may not return a valid replica or shard for it, so we shouldn't store data under such token. We also have have code (such as an increasing- key sanity check in the flat mutation reader) which assumes that no real key in the data can be minimum_token, and our plan is to start allowing data with an empty key (at least for materialized views). This patch does not risk a backward-incompatible disk format changes for two reasons: 1. In the current Scylla, there was no valid case where an empty partition key may appear. CQL and Thrift forbid such keys, and materialized-views and indexes also (incorrectly - see #9364, #9375) drop such rows. 2. Although Cassandra *does* allow empty partition keys, they is only allowed in materialized views and indexes - and we don't support reading materialized views generated by Cassandra (the user must re-generate them in Scylla). When #9364 and #9375 will be fixed by the next patch, empty partition keys will start appearing in Scylla (in materialized views and in the materialized view backing a secondary index), and this fix will become important. Fixes #9352 Refs #9364 Refs #9375 Signed-off-by: Nadav Har'El <nyh@scylladb.com>