From 7e6c5394f3fc7433663ddf0c727ac911a67a0cd2 Mon Sep 17 00:00:00 2001
From: Nadav Har'El <nyh@scylladb.com>
Date: Wed, 3 Nov 2021 09:35:51 +0200
Subject: [PATCH] alternator: move list_concatenate() function

The list_concatenate() function was only used for UpdateExpression's
ADD operation, so we made it a static function in the source file where
it was used. In the next patch, we'll want to use it in another place
(AttributeUpdates' ADD operation), so let's move it to the same file
where similar functions for sets exist.

This patch is almost entirely a code move, but also makes one small
change: list_concatenate() used to throw an exception if one of the
arguments wasn't a list, but the text of this exception was specific to
UpdateExpression. So in the new version, we return a null value in this
case - and the caller checks for it and throws the right exception.

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
---
 alternator/expressions.cc   | 24 +++++-------------------
 alternator/serialization.cc | 19 +++++++++++++++++++
 alternator/serialization.hh |  6 ++++++
 3 files changed, 30 insertions(+), 19 deletions(-)

diff --git a/alternator/expressions.cc b/alternator/expressions.cc
index a948a18874..5d58aea8d2 100644
--- a/alternator/expressions.cc
+++ b/alternator/expressions.cc
@@ -428,24 +428,6 @@ void for_condition_expression_on(const parsed::condition_expression& ce, const n
 // expression. The parsed expression is assumed to have been "resolved", with
 // the matching resolve_* function.
 
-// Take two JSON-encoded list values (remember that a list value is
-// {"L": [...the actual list]}) and return the concatenation, again as
-// a list value.
-static rjson::value list_concatenate(const rjson::value& v1, const rjson::value& v2) {
-    const rjson::value* list1 = unwrap_list(v1);
-    const rjson::value* list2 = unwrap_list(v2);
-    if (!list1 || !list2) {
-        throw api_error::validation("UpdateExpression: list_append() given a non-list");
-    }
-    rjson::value cat = rjson::copy(*list1);
-    for (const auto& a : list2->GetArray()) {
-        rjson::push_back(cat, rjson::copy(a));
-    }
-    rjson::value ret = rjson::empty_object();
-    rjson::set(ret, "L", std::move(cat));
-    return ret;
-}
-
 // 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.
@@ -530,7 +512,11 @@ std::unordered_map<std::string_view, function_handler_type*> function_handlers {
             }
             rjson::value v1 = calculate_value(f._parameters[0], caller, previous_item);
             rjson::value v2 = calculate_value(f._parameters[1], caller, previous_item);
-            return list_concatenate(v1, v2);
+            rjson::value ret = list_concatenate(v1, v2);
+            if (ret.IsNull()) {
+                throw api_error::validation("UpdateExpression: list_append() given a non-list");
+            }
+            return ret;
         }
     },
     {"if_not_exists", [] (calculate_value_caller caller, const rjson::value* previous_item, const parsed::value::function_call& f) {
diff --git a/alternator/serialization.cc b/alternator/serialization.cc
index 050b445106..a223bd8b55 100644
--- a/alternator/serialization.cc
+++ b/alternator/serialization.cc
@@ -373,4 +373,23 @@ std::optional<rjson::value> set_diff(const rjson::value& v1, const rjson::value&
     return ret;
 }
 
+// Take two JSON-encoded list values (remember that a list value is
+// {"L": [...the actual list]}) and return the concatenation, again as
+// a list value.
+// Returns a null value if one of the arguments is not actually a list.
+rjson::value list_concatenate(const rjson::value& v1, const rjson::value& v2) {
+    const rjson::value* list1 = unwrap_list(v1);
+    const rjson::value* list2 = unwrap_list(v2);
+    if (!list1 || !list2) {
+        return rjson::null_value();
+    }
+    rjson::value cat = rjson::copy(*list1);
+    for (const auto& a : list2->GetArray()) {
+        rjson::push_back(cat, rjson::copy(a));
+    }
+    rjson::value ret = rjson::empty_object();
+    rjson::set(ret, "L", std::move(cat));
+    return ret;
+}
+
 }
diff --git a/alternator/serialization.hh b/alternator/serialization.hh
index 3d460f5961..5701523737 100644
--- a/alternator/serialization.hh
+++ b/alternator/serialization.hh
@@ -85,5 +85,11 @@ rjson::value set_sum(const rjson::value& v1, const rjson::value& v2);
 // DynamoDB does not allow empty sets, so if resulting set is empty, return
 // an unset optional instead.
 std::optional<rjson::value> set_diff(const rjson::value& v1, const rjson::value& v2);
+// Take two JSON-encoded list values (remember that a list value is
+// {"L": [...the actual list]}) and return the concatenation, again as
+// a list value.
+// Returns a null value if one of the arguments is not actually a list.
+rjson::value list_concatenate(const rjson::value& v1, const rjson::value& v2);
+
 
 }