diff --git a/alternator/expressions.cc b/alternator/expressions.cc index 3af8c0468204..82703d1a6d85 100644 --- a/alternator/expressions.cc +++ b/alternator/expressions.cc @@ -418,9 +418,14 @@ void for_condition_expression_on(const parsed::condition_expression& ce, const n // calculate_size() is ConditionExpression's size() function, i.e., it takes // a JSON-encoded value and returns its "size" as defined differently for the // different types - also as a JSON-encoded number. -// It return a JSON-encoded "null" value if this value's type has no size -// defined. Comparisons against this non-numeric value will later fail. -static rjson::value calculate_size(const rjson::value& v) { +// If the value's type (e.g. number) has no size defined, there are two cases: +// 1. If from_data (the value came directly from an attribute of the data), +// It returns a JSON-encoded "null" value. Comparisons against this +// non-numeric value will later fail, so eventually the application will +// get a ConditionalCheckFailedException. +// 2. Otherwise (the value came from a constant in the query or some other +// calculation), throw a ValidationException. +static rjson::value calculate_size(const rjson::value& v, bool from_data) { // NOTE: If v is improperly formatted for our JSON value encoding, it // must come from the request itself, not from the database, so it makes // sense to throw a ValidationException if we see such a problem. @@ -449,10 +454,12 @@ static rjson::value calculate_size(const rjson::value& v) { throw api_error::validation(format("invalid byte string: {}", v)); } ret = base64_decoded_len(rjson::to_string_view(it->value)); - } else { + } else if (from_data) { rjson::value json_ret = rjson::empty_object(); rjson::add(json_ret, "null", rjson::value(true)); return json_ret; + } else { + throw api_error::validation(format("Unsupported operand type {} for function size()", it->name)); } rjson::value json_ret = rjson::empty_object(); rjson::add(json_ret, "N", rjson::from_string(std::to_string(ret))); @@ -534,7 +541,7 @@ std::unordered_map function_handlers { format("{}: size() accepts 1 parameter, got {}", caller, f._parameters.size())); } rjson::value v = calculate_value(f._parameters[0], caller, previous_item); - return calculate_size(v); + return calculate_size(v, f._parameters[0].is_path()); } }, {"attribute_exists", [] (calculate_value_caller caller, const rjson::value* previous_item, const parsed::value::function_call& f) { diff --git a/test/alternator/test_condition_expression.py b/test/alternator/test_condition_expression.py index b7a26e079a53..4de383907535 100644 --- a/test/alternator/test_condition_expression.py +++ b/test/alternator/test_condition_expression.py @@ -1328,6 +1328,55 @@ def test_update_condition_size(test_table_s): ConditionExpression='size(a, a)=:arg', ExpressionAttributeValues={':val': 1, ':arg': 5}) +# The documentation claims that the size() function requires a path parameter +# so we check that both direct and reference paths work. But it turns out +# that size() can *also* be run on values set in the query. +# Reproduces #14592. +def test_update_condition_size_parameter(test_table_s): + p = random_string() + test_table_s.update_item(Key={'p': p}, + AttributeUpdates={'a': {'Value': 'hello', 'Action': 'PUT'}}) + # size(a) - works: + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :val', + ConditionExpression='size(a)>=:arg', + ExpressionAttributeValues={':val': 1, ':arg': 2}) + assert test_table_s.get_item(Key={'p': p}, ConsistentRead=True)['Item']['z'] == 1 + # size(#zyz) - works + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :val', + ConditionExpression='size(#xyz)>=:arg', + ExpressionAttributeNames={'#xyz': 'a'}, + ExpressionAttributeValues={':val': 2, ':arg': 2}) + assert test_table_s.get_item(Key={'p': p}, ConsistentRead=True)['Item']['z'] == 2 + # size(:xyz) returns the size of the value defined as :xyz, it does NOT + # take the value of :xyz as referring to the path! If the value :xyz + # is a string, its size is defined. But it's an integer, it's not. + # Because the error is in the query (not the data from the database), + # it generates a ValidationException in this case, *not* a + # ConditionalCheckFailedException. This is different from the case we + # tested above in test_update_condition_size, of invalid type in the + # database. The error ValidationException message is "Invalid + # ConditionExpression: Incorrect operand type for operator or function; + # operator or function: size, operand type: N". + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :val', + ConditionExpression='size(:xyz)>=:arg', + ExpressionAttributeValues={':val': 3, ':arg': 2, ':xyz': 'abc'}) + assert test_table_s.get_item(Key={'p': p}, ConsistentRead=True)['Item']['z'] == 3 + with pytest.raises(ClientError, match='ValidationException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :val', + ConditionExpression='size(:xyz)>=:arg', + ExpressionAttributeValues={':val': 3, ':arg': 2, ':xyz': 123}) + # Similarly, size(size(a)) is a ValidationException as well - because + # size(a) is a number, for which size() is not defined. + with pytest.raises(ClientError, match='ValidationException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :val', + ConditionExpression='size(size(a))>=:arg', + ExpressionAttributeValues={':val': 2, ':arg': 2}) + # The above test tested conditions involving size() in a comparison. # Trying to use just size(a) as a condition (as we use the rest of the # functions supported by ConditionExpression) does not work - DynamoDB