mirrors/scylladb
1
0
Fork 0
mirror of https://github.com/scylladb/scylladb.git synced 2026-06-03 21:47:10 +00:00
Code Issues Packages Projects Releases Wiki Activity

alternator: parse more types of values in UpdateExpression

Browse Source 
Until this patch, in update expressions like "SET a = :val", we only
allowed the right-hand-side of the assignment to be a reference to a
value stored in the request - like ":val" in the above example.

But DynamoDB also allows the value to be an attribute path (e.g.,
"a.b[3].c", and can also be a function of a bunch of other values.
This patch adds supports for parsing all these value types.

This patch only adds the correct parsing of these additional types of
values, but they are still not supported: reading existing attributes
(i.e., read-modify-write operations) is still not supported, and
none of the two functions which UpdateExpression needs to support
are supported yet. Nevertheless, the parsing is now correct, and the
the "unknown_function" test starts to pass.

Note that DynamoDB allows the right-hand side of an assignment to be
not only a single value, but also value+value and value-value. This
possibility is not yet supported by the parser and will be added
later.

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
This commit is contained in:
Nadav Har'El
2019-06-25 19:36:36 +03:00
parent cb50207c7b
commit 9d2eba1c75
4 changed files with 97 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
alternator-test/test_update_expression.py
View File

@@ -552,7 +552,6 @@ def test_update_expression_function_plus_nesting(test_table_s):
# This means that the parser accepts any alphanumeric name as a function
# name, and only later use of this function fails because it's not one of
# the supported file.
@pytest.mark.xfail(reason="SET functions not yet implemented")
def test_update_expression_unknown_function(test_table_s):
p = random_string()
with pytest.raises(ClientError, match='ValidationException.*f'):
@@ -565,7 +564,7 @@ def test_update_expression_unknown_function(test_table_s):
# function names must also start with an alphabetic character. Trying
# to use _f as a function name will result with an actual syntax error,
# on the "_" token.
with pytest.raises(ClientError, match='ValidationException.*Syntax error'):
with pytest.raises(ClientError, match='ValidationException.*yntax error'):
test_table_s.update_item(Key={'p': p},
UpdateExpression='SET a = _f(b,c,d)')

37
alternator/executor.cc
View File

@@ -383,6 +383,35 @@ static void verify_all_are_used(const Json::Value& req, const char* field,
}
}
// Given a parsed::value, which can refer either to a constant value from
// ExpressionAttributeValues, to the value of some attribute, or to a function
// of other values, this function calculates the resulting value.
static Json::Value calculate_value(const parsed::value& v,
const Json::Value& expression_attribute_values,
std::unordered_set<std::string>& used_attribute_values) {
if (v.is_valref()) {
auto& valref = v.as_valref();
const Json::Value& value = expression_attribute_values.get(valref, Json::nullValue);
if (value.isNull()) {
throw api_error("ValidationException",
format("ExpressionAttributeValues missing entry '{}' required by UpdateExpression", valref));
}
used_attribute_values.emplace(std::move(valref));
return value;
} else if (v.is_function_call()) {
auto& f = v.as_function_call();
// FIXME: calculate parameter values and support the known functions
throw api_error("ValidationException",
format("UpdateExpression: unsupported function '{}' called.", f._function_name));
} else if (v.is_path()) {
// FIXME: support path value (read before write).
throw api_error("ValidationException", "UpdateExpression: unsupported value type.");
}
// Can't happen
return Json::Value::null;
}
future<json::json_return_type> executor::update_item(std::string content) {
_stats.api_operations.update_item++;
Json::Value update_info = json::to_json_value(content);
@@ -435,13 +464,7 @@ future<json::json_return_type> executor::update_item(std::string content) {
}
if (action.is_set()) {
auto a = action.as_set();
// FIXME: this code is for rhs being a value reference - ":val". Need to support the other options!!
const Json::Value& value = update_info["ExpressionAttributeValues"].get(a._rhs, Json::nullValue);
if (value.isNull()) {
throw api_error("ValidationException",
format("ExpressionAttributeValues missing entry '{}' required by UpdateExpression", a._rhs));
}
used_attribute_values.emplace(std::move(a._rhs));
auto value = calculate_value(a._rhs, update_info["ExpressionAttributeValues"], used_attribute_values);
bytes val = serialize_item(value);
attrs_mut.cells.emplace_back(to_bytes(column_name), atomic_cell::make_live(*bytes_type, ts, std::move(val), atomic_cell::collection_member::yes));
} else if (action.is_remove()) {

17
alternator/expressions.g
View File

@@ -148,9 +148,22 @@ path returns [parsed::path p]:
| '[' INTEGER ']' { $p.add_index(std::stoi($INTEGER.text)); }
)*;
// FIXME: set action supports additional types of rhs besides VALREF.
update_expression_set_value returns [parsed::value v]:
VALREF { $v.set_valref($VALREF.text); }
| path { $v.set_path($path.p); }
| NAME { $v.set_func_name($NAME.text); }
'(' x=update_expression_set_value { $v.add_func_parameter($x.v); }
(',' x=update_expression_set_value { $v.add_func_parameter($x.v); })*
')'
;
// TODO: rhs can also be value+value or value-value.
update_expression_set_rhs returns [parsed::value v]:
update_expression_set_value { $v = $update_expression_set_value.v; }
;
update_expression_set_action returns [parsed::update_expression::action a]:
path '=' VALREF { $a.assign_set($path.p, $VALREF.text); };
path '=' rhs=update_expression_set_rhs { $a.assign_set($path.p, $rhs.v); };
update_expression_remove_action returns [parsed::update_expression::action a]:
path { $a.assign_remove($path.p); };

55
alternator/expressions_types.hh
View File

@@ -52,14 +52,60 @@ public:
}
};
// "value" is is a value used in the right hand side of an assignment
// expression, "SET a = ...". It can be a reference to a value included in
// the request (":val"), a path to an attribute from the existing item
// (e.g., "a.b[3].c"), or a function of other such values.
// Note that the real right-hand-side of an assignment is actually a bit
// more general - it allows either a value, or a value+value or value-value.
class value {
public:
struct function_call {
std::string _function_name;
std::vector<value> _parameters;
};
private:
std::variant<std::string, path, function_call> _value;
public:
void set_valref(std::string s) {
_value = std::move(s);
}
void set_path(path p) {
_value = std::move(p);
}
void set_func_name(std::string s) {
_value = function_call {std::move(s), {}};
}
void add_func_parameter(value v) {
std::get<function_call>(_value)._parameters.emplace_back(std::move(v));
}
bool is_valref() const {
return std::holds_alternative<std::string>(_value);
}
bool is_function_call() const {
return std::holds_alternative<function_call>(_value);
}
bool is_path() const {
return std::holds_alternative<path>(_value);
}
const std::string& as_valref() const {
return std::get<std::string>(_value);
}
const function_call& as_function_call() const {
return std::get<function_call>(_value);
}
const path& as_path() const {
return std::get<path>(_value);
}
};
class update_expression {
public:
struct action {
path _path;
struct set {
// TODO: rhs can actually also be a path, function, or rhs+rhs, or rhs-rhs.
// Not just a ":valname" string.
std::string _rhs;
// TODO: needs to be rhs type, not just value (also value+value, value-value)
value _rhs;
};
struct remove {
};
@@ -71,7 +117,8 @@ public:
};
std::variant<set, remove, add, del> _action;
void assign_set(path p, std::string v) {
// FIXME: rhs type, not just one value, also value+value, value-value
void assign_set(path p, value v) {
_path = std::move(p);
_action = set { std::move(v) };
}