alternator: fix error path for size() function on constants

The DynamoDB documentation for the size() function claims that it only
works on paths (attribute names or references), but it actually works on
constants from the query (e.g., ":val") as well.

It turns out that Alternator supports this undocumented case already, but
gets the error path wrong: Usually, when size() is calculated on the data,
if the data has the wrong type of size() (e.g., an integer), the condition
simply doesn't match. But if the value comes from the query - it should
generate an error that the query is wrong - ValidationException.

This patch fixes this case, and also adds tests for it that pass on both
DynamoDB and Alternator (after this patch).

Fixes #14592

Signed-off-by: Nadav Har'El <nyh@scylladb.com>

Closes #14593

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<std::string_view, function_handler_type*> 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) {

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