From fc946ddfbac324cc3cc2550ab7fade5002fced01 Mon Sep 17 00:00:00 2001 From: Nadav Har'El Date: Sun, 1 Sep 2019 13:28:32 +0300 Subject: [PATCH] alternator: clean error, not a crash, on reserved column name Currently, we reserve the name ATTRS_COLUMN_NAME ("attrs") - the user cannot use it as a key column name (key of the base table or GSI or LSI) because we use this name for the attribute map we add to the schema. Currently, if the user does attempt to create such a key column, the result is undefined (sometimes corrupt sstables, sometimes outright crashes). This patches fixes it to become a clean error, saying that this column name is currently reserved. The test test_create_table_special_column_name now cleanly fails, instead of crashing Scylla, so it is converted from "skip" to "xfail". Eventually we need to solve this issue completely (e.g., in rare cases rename columns to allow us to reserve a name like ATTRS_COLUMN_NAME, or alternatively, instead of using a fixed name ATTRS_COLUMN_NAME pick a different one different from the key column names). But until we do, better fail with a clear error instead of a crash. Signed-off-by: Nadav Har'El Message-Id: <20190901102832.7452-1-nyh@scylladb.com> --- alternator-test/test_table.py | 2 +- alternator/executor.cc | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/alternator-test/test_table.py b/alternator-test/test_table.py index 92a0e00598..fbeaf31c67 100644 --- a/alternator-test/test_table.py +++ b/alternator-test/test_table.py @@ -247,7 +247,7 @@ def test_table_special_column_name(dynamodb): ) yield table table.delete() -@pytest.mark.skip(reason="special attrs column not yet hidden correctly") +@pytest.mark.xfail(reason="special attrs column not yet hidden correctly") def test_create_table_special_column_name(test_table_special_column_name): s = random_string() c = random_string() diff --git a/alternator/executor.cc b/alternator/executor.cc index c640b7e4b2..853422daea 100644 --- a/alternator/executor.cc +++ b/alternator/executor.cc @@ -317,6 +317,13 @@ static data_type parse_key_type(const std::string& type) { static void add_column(schema_builder& builder, const std::string& name, const rjson::value& attribute_definitions, column_kind kind) { + // FIXME: Currently, the column name ATTRS_COLUMN_NAME is not allowed + // because we use it for our untyped attribute map, and we can't have a + // second column with the same name. We should fix this, by renaming + // some column names which we want to reserve. + if (name == executor::ATTRS_COLUMN_NAME) { + throw api_error("ValidationException", format("Column name '{}' is currently reserved. FIXME.", name)); + } for (auto it = attribute_definitions.Begin(); it != attribute_definitions.End(); ++it) { const rjson::value& attribute_info = *it; if (attribute_info["AttributeName"].GetString() == name) {