Merge "Fix setting collections to null" from Paweł

"The following patches fix a bug in collections setters where the tombstone
was created but never applied if the collection was set to null.

This series makes cql_tests.py:TestCQL.null_support_test pass."

cql3/lists.cc

@@ -215,12 +215,15 @@ lists::precision_time::get_next(db_clock::time_point millis) {
 
 void
 lists::setter::execute(mutation& m, const exploded_clustering_prefix& prefix, const update_parameters& params) {
-    tombstone ts;
     if (column.type->is_multi_cell()) {
         // delete + append
-        ts = params.make_tombstone_just_before();
+        collection_type_impl::mutation mut;
+        mut.tomb = params.make_tombstone_just_before();
+        auto ctype = static_pointer_cast<const list_type_impl>(column.type);
+        auto col_mut = ctype->serialize_mutation_form(std::move(mut));
+        m.set_cell(prefix, column, std::move(col_mut));
     }
-    do_append(_t, m, prefix, column, params, ts);
+    do_append(_t, m, prefix, column, params);
 }
 
 bool
@@ -291,8 +294,7 @@ lists::do_append(shared_ptr<term> t,
         mutation& m,
         const exploded_clustering_prefix& prefix,
         const column_definition& column,
-        const update_parameters& params,
-        tombstone ts) {
+        const update_parameters& params) {
     auto&& value = t->bind(params._options);
     auto&& list_value = dynamic_pointer_cast<lists::value>(value);
     auto&& ltype = dynamic_pointer_cast<const list_type_impl>(column.type);
@@ -305,7 +307,6 @@ lists::do_append(shared_ptr<term> t,
 
         auto&& to_add = list_value->_elements;
         collection_type_impl::mutation appended;
-        appended.tomb = ts;
         appended.cells.reserve(to_add.size());
         for (auto&& e : to_add) {
             auto uuid1 = utils::UUID_gen::get_time_UUID_bytes();

cql3/lists.hh

@@ -152,8 +152,7 @@ public:
             mutation& m,
             const exploded_clustering_prefix& prefix,
             const column_definition& column,
-            const update_parameters& params,
-            tombstone ts = {});
+            const update_parameters& params);
 
     class prepender : public operation {
     public:

cql3/maps.cc

@@ -243,13 +243,15 @@ maps::marker::bind(const query_options& options) {
 
 void
 maps::setter::execute(mutation& m, const exploded_clustering_prefix& row_key, const update_parameters& params) {
-    tombstone ts;
     if (column.type->is_multi_cell()) {
         // delete + put
-        // delete + append
-        ts = params.make_tombstone_just_before();
+        collection_type_impl::mutation mut;
+        mut.tomb = params.make_tombstone_just_before();
+        auto ctype = static_pointer_cast<const map_type_impl>(column.type);
+        auto col_mut = ctype->serialize_mutation_form(std::move(mut));
+        m.set_cell(row_key, column, std::move(col_mut));
     }
-    do_put(m, row_key, params, _t, column, ts);
+    do_put(m, row_key, params, _t, column);
 }
 
 void
@@ -289,12 +291,11 @@ maps::putter::execute(mutation& m, const exploded_clustering_prefix& prefix, con
 
 void
 maps::do_put(mutation& m, const exploded_clustering_prefix& prefix, const update_parameters& params,
-        shared_ptr<term> t, const column_definition& column, tombstone ts) {
+        shared_ptr<term> t, const column_definition& column) {
     auto value = t->bind(params._options);
     auto map_value = dynamic_pointer_cast<maps::value>(value);
     if (column.type->is_multi_cell()) {
         collection_type_impl::mutation mut;
-        mut.tomb = ts;
 
         if (!value) {
             return;

cql3/maps.hh

@@ -121,7 +121,7 @@ public:
     };
 
     static void do_put(mutation& m, const exploded_clustering_prefix& prefix, const update_parameters& params,
-            shared_ptr<term> t, const column_definition& column, tombstone ts = {});
+            shared_ptr<term> t, const column_definition& column);
 
     class discarder_by_key : public operation {
     public:

cql3/sets.cc

@@ -204,12 +204,15 @@ sets::marker::bind(const query_options& options) {
 
 void
 sets::setter::execute(mutation& m, const exploded_clustering_prefix& row_key, const update_parameters& params) {
-    tombstone ts;
     if (column.type->is_multi_cell()) {
         // delete + add
-        ts = params.make_tombstone_just_before();
+        collection_type_impl::mutation mut;
+        mut.tomb = params.make_tombstone_just_before();
+        auto ctype = static_pointer_cast<const set_type_impl>(column.type);
+        auto col_mut = ctype->serialize_mutation_form(std::move(mut));
+        m.set_cell(row_key, column, std::move(col_mut));
     }
-    adder::do_add(m, row_key, params, _t, column, ts);
+    adder::do_add(m, row_key, params, _t, column);
 }
 
 void
@@ -220,14 +223,13 @@ sets::adder::execute(mutation& m, const exploded_clustering_prefix& row_key, con
 
 void
 sets::adder::do_add(mutation& m, const exploded_clustering_prefix& row_key, const update_parameters& params,
-        shared_ptr<term> t, const column_definition& column, tombstone ts) {
+        shared_ptr<term> t, const column_definition& column) {
     auto&& value = t->bind(params._options);
     auto set_value = dynamic_pointer_cast<sets::value>(std::move(value));
     auto set_type = dynamic_pointer_cast<const set_type_impl>(column.type);
     if (column.type->is_multi_cell()) {
         // FIXME: mutation_view? not compatible with params.make_cell().
         collection_type_impl::mutation mut;
-        mut.tomb = ts;
 
         if (!set_value || set_value->_elements.empty()) {
             return;

cql3/sets.hh

@@ -105,7 +105,7 @@ public:
         }
         virtual void execute(mutation& m, const exploded_clustering_prefix& row_key, const update_parameters& params) override;
         static void do_add(mutation& m, const exploded_clustering_prefix& row_key, const update_parameters& params,
-                shared_ptr<term> t, const column_definition& column, tombstone ts = {});
+                shared_ptr<term> t, const column_definition& column);
     };
 
     // Note that this is reused for Map subtraction too (we subtract a set from a map)

tests/urchin/cql_query_test.cc

@@ -756,6 +756,11 @@ SEASTAR_TEST_CASE(test_map_insert_update) {
         }).then([&e] {
             return e.require_column_has_value("cf", {sstring("key1")}, {},
                                               "map1", map_type_impl::native_type({{{1009, 5009}}}));
+        }).then([&e] {
+            return e.execute_cql("insert into cf (p1, map1) values ('key1', null);").discard_result();
+        }).then([&e] {
+            return e.require_column_has_value("cf", {sstring("key1")}, {},
+                    "map1", map_type_impl::native_type({}));
         });
     });
 }
@@ -812,6 +817,11 @@ SEASTAR_TEST_CASE(test_set_insert_update) {
         }).then([&e] {
             return e.require_column_has_value("cf", {sstring("key1")}, {},
                                             "set1", set_type_impl::native_type({1009}));
+        }).then([&e] {
+            return e.execute_cql("insert into cf (p1, set1) values ('key1', null);").discard_result();
+        }).then([&e] {
+            return e.require_column_has_value("cf", {sstring("key1")}, {},
+                    "set1", set_type_impl::native_type({}));
         });
     });
 }
@@ -865,6 +875,11 @@ SEASTAR_TEST_CASE(test_list_insert_update) {
         }).then([&e] {
             return e.require_column_has_value("cf", {sstring("key1")}, {},
                     "list1", list_type_impl::native_type({2001, 2002, 2008, 2010, 2012, 2019}));
+        }).then([&e] {
+            return e.execute_cql("insert into cf (p1, list1) values ('key1', null);").discard_result();
+        }).then([&e] {
+            return e.require_column_has_value("cf", {sstring("key1")}, {},
+                    "list1", list_type_impl::native_type({}));
         });
     });
 }