bahman/scylla
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

alternator: implement AttributeUpdates DELETE operation with Value

Browse Source 
In the DynamoDB API, UpdateItem's AttributeUpdates parameter (the older
syntax, which was superseded by UpdateExpression) has a DELETE operation
that can do two different things: It can delete an attribute, or it can
delete elements from a set. Before this patch we only implemented the
first feature, and this patch implements the second.

Note that unlike the ordinary delete, the second feature - set subtraction -
is a read-modify-write operation. This is not only because of Alternator's
serialization (as JSON strings, not CRDTs) - but also fundementally because
of the API's guarantees - e.g., the operation is supposed to fail if the
attribute's existing value is *not* a set of the correct type, so it
needs to read the old value.

The test for this feature begins to pass, so its "xfail" mark is
removed. After this, all tests in test/alternator/test_item.py pass :-)

Fixes #5864.

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Message-Id: <20211103151206.157184-1-nyh@scylladb.com>
This commit is contained in:
Nadav Har'El
2021-11-03 17:12:06 +02:00
committed by Piotr Sarna
parent 1d84bc6c3b
commit 253387ea07
3 changed files with 41 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

49
alternator/executor.cc
View File

@@ -2336,8 +2336,11 @@ static bool check_needs_read_before_write_attribute_updates(rjson::value *attrib
if (action_s == "ADD") {
return true;
}
// FIXME: we also need to read before write in certain cases
// of the DELETE action.
// For DELETE operation, it only needs a read before write if the
// "Value" option is used. Without it, it's just a delete.
if (action_s == "DELETE" && it->value.HasMember("Value")) {
return true;
}
}
}
return false;
@@ -2696,16 +2699,42 @@ update_item_operation::apply(std::unique_ptr<rjson::value> previous_item, api::t
}
std::string action = (it->value)["Action"].GetString();
if (action == "DELETE") {
// FIXME: Currently we support only the simple case where the
// "Value" field is missing. If it were not missing, we would
// we need to verify the old type and/or value is same as
// specified before deleting... We don't do this yet.
// This is issue #5864.
// The DELETE operation can do two unrelated tasks. Without a
// "Value" option, it is used to delete an attribute. With a
// "Value" option, it is used to delete a set of elements from
// a set attribute of the same type.
if (it->value.HasMember("Value")) {
throw api_error::validation(
format("UpdateItem DELETE with checking old value not yet supported"));
// Subtracting sets needs a read of previous_item, so
// check_needs_read_before_write_attribute_updates()
// returns true in this case, and previous_item is
// available to us when the item exists.
const rjson::value* v1 = previous_item ? rjson::find(*previous_item, to_sstring_view(column_name)) : nullptr;
const rjson::value& v2 = (it->value)["Value"];
validate_value(v2, "AttributeUpdates");
std::string v2_type = get_item_type_string(v2);
if (v2_type != "SS" && v2_type != "NS" && v2_type != "BS") {
throw api_error::validation(format("AttributeUpdates DELETE operation with Value only valid for sets, got type {}", v2_type));
}
if (v1) {
std::optional<rjson::value> result = set_diff(*v1, v2);
if (result) {
do_update(std::move(column_name), *result);
} else {
// DynamoDB does not allow empty sets - if the
// result is empty, delete the attribute.
do_delete(std::move(column_name));
}
} else {
// if the attribute or item don't exist, the DELETE
// operation should silently do nothing - and not
// create an empty item. It's a waste to call
// do_delete() on an attribute we already know is
// deleted, so we can just mark any_deletes = true.
any_deletes = true;
}
} else {
do_delete(std::move(column_name));
}
do_delete(std::move(column_name));
} else if (action == "PUT") {
const rjson::value& value = (it->value)["Value"];
validate_value(value, "AttributeUpdates");

4
alternator/serialization.cc
View File

@@ -277,7 +277,7 @@ const std::pair<std::string, const rjson::value*> unwrap_set(const rjson::value&
auto it = v.MemberBegin();
const std::string it_key = it->name.GetString();
if (it_key != "SS" && it_key != "BS" && it_key != "NS") {
return {"", nullptr};
return {std::move(it_key), nullptr};
}
return std::make_pair(it_key, &(it->value));
}
@@ -347,7 +347,7 @@ std::optional<rjson::value> set_diff(const rjson::value& v1, const rjson::value&
auto [set1_type, set1] = unwrap_set(v1);
auto [set2_type, set2] = unwrap_set(v2);
if (set1_type != set2_type) {
throw api_error::validation(format("Mismatched set types: {} and {}", set1_type, set2_type));
throw api_error::validation(format("Set DELETE type mismatch: {} and {}", set1_type, set2_type));
}
if (!set1 || !set2) {
throw api_error::validation("UpdateExpression: DELETE operation can only be performed on a set");

1
test/alternator/test_item.py
View File

@@ -440,7 +440,6 @@ def test_update_item_non_existent(test_table_s):
# used to delete elements from a set attribute. The latter is a RMW operation,
# because it requires testing the existing value of the attribute, if it
# is indeed a set of the desired type.
@pytest.mark.xfail(reason="UpdateItem AttributeUpdates DELETE not implemented for sets")
def test_update_item_delete(test_table_s):
p = random_string()
a = random_string()