From 69d0eba9c78118d5e7135ea48a89ce1aa2b4c1d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Dziepak?= Date: Wed, 22 Jul 2015 15:29:54 +0200 Subject: [PATCH 1/3] cql3: fix setting collections to null MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this patch the tombstone created in setter::execute() was never applied if the new collection value was null. Signed-off-by: Paweł Dziepak --- cql3/lists.cc | 9 ++++++--- cql3/maps.cc | 10 ++++++---- cql3/sets.cc | 9 ++++++--- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/cql3/lists.cc b/cql3/lists.cc index f9ebb01057..8a865646b7 100644 --- a/cql3/lists.cc +++ b/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(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 diff --git a/cql3/maps.cc b/cql3/maps.cc index 50f0efdad4..b8ce663ac2 100644 --- a/cql3/maps.cc +++ b/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(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 diff --git a/cql3/sets.cc b/cql3/sets.cc index ed9791f2aa..62633c7f9e 100644 --- a/cql3/sets.cc +++ b/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(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 From 6788a7afdbd53bd5f5dcd4c344cf6db5319bffe6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Dziepak?= Date: Wed, 22 Jul 2015 15:30:13 +0200 Subject: [PATCH 2/3] cql3: remove tombstone argument from do_{add, put, append} MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Paweł Dziepak --- cql3/lists.cc | 4 +--- cql3/lists.hh | 3 +-- cql3/maps.cc | 3 +-- cql3/maps.hh | 2 +- cql3/sets.cc | 3 +-- cql3/sets.hh | 2 +- 6 files changed, 6 insertions(+), 11 deletions(-) diff --git a/cql3/lists.cc b/cql3/lists.cc index 8a865646b7..acece4cc93 100644 --- a/cql3/lists.cc +++ b/cql3/lists.cc @@ -294,8 +294,7 @@ lists::do_append(shared_ptr 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(value); auto&& ltype = dynamic_pointer_cast(column.type); @@ -308,7 +307,6 @@ lists::do_append(shared_ptr 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(); diff --git a/cql3/lists.hh b/cql3/lists.hh index 317c0adbce..dc357016fd 100644 --- a/cql3/lists.hh +++ b/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: diff --git a/cql3/maps.cc b/cql3/maps.cc index b8ce663ac2..4c41801ac7 100644 --- a/cql3/maps.cc +++ b/cql3/maps.cc @@ -291,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 t, const column_definition& column, tombstone ts) { + shared_ptr t, const column_definition& column) { auto value = t->bind(params._options); auto map_value = dynamic_pointer_cast(value); if (column.type->is_multi_cell()) { collection_type_impl::mutation mut; - mut.tomb = ts; if (!value) { return; diff --git a/cql3/maps.hh b/cql3/maps.hh index 1a0d3f4ff0..1e4182e938 100644 --- a/cql3/maps.hh +++ b/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 t, const column_definition& column, tombstone ts = {}); + shared_ptr t, const column_definition& column); class discarder_by_key : public operation { public: diff --git a/cql3/sets.cc b/cql3/sets.cc index 62633c7f9e..f7acd3dfe3 100644 --- a/cql3/sets.cc +++ b/cql3/sets.cc @@ -223,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 t, const column_definition& column, tombstone ts) { + shared_ptr t, const column_definition& column) { auto&& value = t->bind(params._options); auto set_value = dynamic_pointer_cast(std::move(value)); auto set_type = dynamic_pointer_cast(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; diff --git a/cql3/sets.hh b/cql3/sets.hh index 3dd7ff4adf..9b645dddc9 100644 --- a/cql3/sets.hh +++ b/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 t, const column_definition& column, tombstone ts = {}); + shared_ptr t, const column_definition& column); }; // Note that this is reused for Map subtraction too (we subtract a set from a map) From 2221604fed7815420fdbe04c7fc54f078f6883b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Dziepak?= Date: Wed, 22 Jul 2015 15:30:22 +0200 Subject: [PATCH 3/3] tests/cql3: test setting collections to null MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Paweł Dziepak --- tests/urchin/cql_query_test.cc | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/urchin/cql_query_test.cc b/tests/urchin/cql_query_test.cc index 42db8a2798..a0619544a3 100644 --- a/tests/urchin/cql_query_test.cc +++ b/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({})); }); }); }