Skip to content

Commit

Permalink
cql3 - as_json() - Deserialize Alternator :attrs
Browse files Browse the repository at this point in the history
This commit changes the behavior of SELECT JSON calls when run on top of
ScyllaDB Alternator tables.

By definition, DynamoDB Attributes not part of a PRIMARY KEY (Hash or
Sort Key in Dynamo, nor GSIs) are encapsulated in CQL in a 'map<text,
blob>' field, in turn making it hard for one to reason about the data
stored there.

After this commit, SELECT JSON via the CQL binary protocol will
deserialize the attrs column and present it back to the user.

We use the as_json() machinery as it takes care of returning the
entire ResultSet as a String, which in turn makes it easy for drivers to
parse it.

Note that this commit does NOT transform the entire ResultSet as it
would be returned by DynamoDB. It only deserializes the ":attrs" column
and does nothing else.

Arguably, implementing the remaining machinery wouldn't be so hard, but
it is not part of this single experiment done for personal learning
purposes. :-)

Also, note some changes to tests were needed to prevent link breakage.
This was particularly needed given that this commit simply includes alternator
under cql3.
  • Loading branch information
Felipe Mendes authored and root committed Aug 30, 2024
1 parent 67b2485 commit a60a524
Show file tree
Hide file tree
Showing 8 changed files with 70 additions and 41 deletions.
25 changes: 12 additions & 13 deletions configure.py
Original file line number Diff line number Diff line change
Expand Up @@ -1335,7 +1335,7 @@ def find_ninja():
'test/lib/key_utils.cc',
]

scylla_raft_dependencies = scylla_raft_core + ['utils/uuid.cc', 'utils/error_injection.cc', 'utils/exceptions.cc']
scylla_raft_dependencies = scylla_raft_core + alternator + ['utils/uuid.cc', 'utils/error_injection.cc', 'utils/exceptions.cc']

scylla_tools = ['tools/read_mutation.cc',
'tools/scylla-types.cc',
Expand Down Expand Up @@ -1489,7 +1489,7 @@ def find_ninja():
"test/boost/linearizing_input_stream_test.cc",
"test/lib/log.cc",
]
deps['test/boost/expr_test'] = ['test/boost/expr_test.cc', 'test/lib/expr_test_utils.cc'] + scylla_core
deps['test/boost/expr_test'] = ['test/boost/expr_test.cc', 'test/lib/expr_test_utils.cc'] + scylla_core + alternator
deps['test/boost/rate_limiter_test'] = ['test/boost/rate_limiter_test.cc', 'db/rate_limiter.cc']
deps['test/boost/exceptions_optimized_test'] = ['test/boost/exceptions_optimized_test.cc', 'utils/exceptions.cc']
deps['test/boost/exceptions_fallback_test'] = ['test/boost/exceptions_fallback_test.cc', 'utils/exceptions.cc']
Expand All @@ -1500,20 +1500,19 @@ def find_ninja():

deps['test/boost/group0_cmd_merge_test'] += ['test/lib/expr_test_utils.cc']

deps['test/raft/replication_test'] = ['test/raft/replication_test.cc', 'test/raft/replication.cc', 'test/raft/helpers.cc'] + scylla_raft_dependencies
deps['test/raft/raft_server_test'] = ['test/raft/raft_server_test.cc', 'test/raft/replication.cc', 'test/raft/helpers.cc'] + scylla_raft_dependencies
deps['test/raft/randomized_nemesis_test'] = ['test/raft/randomized_nemesis_test.cc', 'direct_failure_detector/failure_detector.cc', 'test/raft/helpers.cc'] + scylla_raft_dependencies
deps['test/raft/failure_detector_test'] = ['test/raft/failure_detector_test.cc', 'direct_failure_detector/failure_detector.cc', 'test/raft/helpers.cc'] + scylla_raft_dependencies
deps['test/raft/many_test'] = ['test/raft/many_test.cc', 'test/raft/replication.cc', 'test/raft/helpers.cc'] + scylla_raft_dependencies
deps['test/raft/fsm_test'] = ['test/raft/fsm_test.cc', 'test/raft/helpers.cc', 'test/lib/log.cc'] + scylla_raft_dependencies
deps['test/raft/etcd_test'] = ['test/raft/etcd_test.cc', 'test/raft/helpers.cc', 'test/lib/log.cc'] + scylla_raft_dependencies
deps['test/raft/replication_test'] = ['test/raft/replication_test.cc', 'test/raft/replication.cc', 'test/raft/helpers.cc'] + alternator + scylla_core
deps['test/raft/raft_server_test'] = ['test/raft/raft_server_test.cc', 'test/raft/replication.cc', 'test/raft/helpers.cc'] + alternator + scylla_core
deps['test/raft/randomized_nemesis_test'] = ['test/raft/randomized_nemesis_test.cc', 'test/raft/helpers.cc'] + alternator + scylla_core
deps['test/raft/failure_detector_test'] = ['test/raft/failure_detector_test.cc', 'test/raft/helpers.cc'] + alternator + scylla_core
deps['test/raft/many_test'] = ['test/raft/many_test.cc', 'test/raft/replication.cc', 'test/raft/helpers.cc'] + alternator + scylla_core
deps['test/raft/fsm_test'] = ['test/raft/fsm_test.cc', 'test/raft/helpers.cc', 'test/lib/log.cc'] + alternator + scylla_core
deps['test/raft/etcd_test'] = ['test/raft/etcd_test.cc', 'test/raft/helpers.cc', 'test/lib/log.cc'] + scylla_core + alternator
deps['test/raft/raft_sys_table_storage_test'] = ['test/raft/raft_sys_table_storage_test.cc'] + \
scylla_core + scylla_tests_generic_dependencies
deps['test/raft/raft_address_map_test'] = ['test/raft/raft_address_map_test.cc'] + scylla_core
scylla_core + scylla_tests_generic_dependencies + alternator
deps['test/raft/raft_address_map_test'] = ['test/raft/raft_address_map_test.cc'] + scylla_core + alternator
deps['test/raft/discovery_test'] = ['test/raft/discovery_test.cc',
'test/raft/helpers.cc',
'test/lib/log.cc',
'service/raft/discovery.cc'] + scylla_raft_dependencies
'test/lib/log.cc'] + scylla_core + alternator

wasm_deps = {}

Expand Down
21 changes: 14 additions & 7 deletions cql3/functions/as_json_function.hh
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,10 @@ namespace functions {
class as_json_function : public scalar_function {
std::vector<sstring> _selector_names;
std::vector<data_type> _selector_types;
bool is_alternator;
public:
as_json_function(std::vector<sstring>&& selector_names, std::vector<data_type> selector_types)
: _selector_names(std::move(selector_names)), _selector_types(std::move(selector_types)) {
as_json_function(std::vector<sstring>&& selector_names, std::vector<data_type> selector_types, bool is_alternator)
: _selector_names(std::move(selector_names)), _selector_types(std::move(selector_types)), is_alternator(std::move(is_alternator)) {
}

virtual bool requires_thread() const override;
Expand All @@ -62,11 +63,17 @@ public:
encoded_row.write("\\\"", 2);
}
encoded_row.write("\": ", 3);
sstring row_sstring = to_json_string(*_selector_types[i], parameters[i]);
encoded_row.write(row_sstring.c_str(), row_sstring.size());
}
encoded_row.write("}", 1);
return bytes(encoded_row.linearize());
if (is_alternator) {
sstring row_sstring = to_json_string(*_selector_types[i], parameters[i], true);
encoded_row.write(row_sstring.c_str(), row_sstring.size());
} else {
sstring row_sstring = to_json_string(*_selector_types[i], parameters[i]);
encoded_row.write(row_sstring.c_str(), row_sstring.size());
}
}

encoded_row.write("}", 1);
return bytes(encoded_row.linearize());
}

virtual const function_name& name() const override {
Expand Down
4 changes: 3 additions & 1 deletion cql3/statements/select_statement.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
#include "utils/assert.hh"
#include "utils/result_combinators.hh"
#include "utils/result_loop.hh"
#include "alternator/executor.hh"
#include "replica/database.hh"
#include "replica/mutation_dump.hh"

Expand Down Expand Up @@ -1930,7 +1931,8 @@ select_statement::maybe_jsonize_select_clause(std::vector<selection::prepared_se
for (const auto& prepared_selector : prepared_selectors) {
args.push_back(std::move(prepared_selector.expr));
}
auto as_json = ::make_shared<functions::as_json_function>(std::move(selector_names), std::move(selector_types));
bool is_alternator = alternator::is_alternator_keyspace(schema->ks_name());
auto as_json = ::make_shared<functions::as_json_function>(std::move(selector_names), std::move(selector_types), std::move(is_alternator));
auto as_json_selector = selection::prepared_selector{.expr = expr::function_call{as_json, std::move(args)}, .alias = nullptr};
prepared_selectors.clear();
prepared_selectors.push_back(as_json_selector);
Expand Down
43 changes: 29 additions & 14 deletions cql3/type_json.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "types/listlike_partial_deserializing_iterator.hh"
#include "utils/managed_bytes.hh"
#include "exceptions/exceptions.hh"
#include "alternator/serialization.hh"
#include <limits>
#include <utility>
#include <boost/algorithm/string/trim_all.hpp>
Expand Down Expand Up @@ -369,7 +370,7 @@ template <typename T> static T compose_value(const integer_type_impl<T>& t, byte
return read_be<T>(reinterpret_cast<const char*>(bv.data()));
}

static sstring to_json_string_aux(const map_type_impl& t, bytes_view bv) {
static sstring to_json_string_aux(const map_type_impl& t, bytes_view bv, std::optional<bool> alternator) {
std::ostringstream out;

out << '{';
Expand All @@ -383,7 +384,7 @@ static sstring to_json_string_aux(const map_type_impl& t, bytes_view bv) {
}

// Valid keys in JSON map must be quoted strings
sstring string_key = to_json_string(*t.get_keys_type(), kb);
sstring string_key = to_json_string(*t.get_keys_type(), kb, bool(alternator));
bool is_unquoted = string_key.empty() || string_key[0] != '"';
if (is_unquoted) {
out << '"';
Expand All @@ -393,7 +394,20 @@ static sstring to_json_string_aux(const map_type_impl& t, bytes_view bv) {
out << '"';
}
out << ": ";
out << to_json_string(*t.get_values_type(), vb);
if (bool(alternator)) {
// TODO: Ideally, just deserialize bv earlier, and retrieve the whole key in DDB format instead
try {
rjson::value v = alternator::deserialize_item(vb);
rapidjson::StringBuffer sb;
rapidjson::Writer<rapidjson::StringBuffer> writer( sb );
v.Accept( writer );
out << sb.GetString();
} catch (const std::exception &exc) {
std::cout << exc.what();
}
} else {
out << to_json_string(*t.get_values_type(), vb, bool(alternator));
}
}
out << '}';
return std::move(out).str();
Expand All @@ -412,7 +426,7 @@ static sstring to_json_string_aux(const set_type_impl& t, bytes_view bv) {
out << ", ";
}
if (e) {
out << to_json_string(*t.get_elements_type(), *e);
out << to_json_string(*t.get_elements_type(), *e, false);
} else {
// Impossible in sets, but let's not insist here.
out << "null";
Expand All @@ -435,7 +449,7 @@ static sstring to_json_string_aux(const list_type_impl& t, bytes_view bv) {
out << ", ";
}
if (e) {
out << to_json_string(*t.get_elements_type(), *e);
out << to_json_string(*t.get_elements_type(), *e, false);
} else {
out << "null";
}
Expand All @@ -456,7 +470,7 @@ static sstring to_json_string_aux(const tuple_type_impl& t, bytes_view bv) {
}
if (*vi) {
// TODO(sarna): We can avoid copying if to_json_string accepted bytes_view
out << to_json_string(**ti, **vi);
out << to_json_string(**ti, **vi, false);
} else {
out << "null";
}
Expand All @@ -482,7 +496,7 @@ static sstring to_json_string_aux(const user_type_impl& t, bytes_view bv) {
out << quote_json_string(t.field_name_as_string(i)) << ": ";
if (*vi) {
//TODO(sarna): We can avoid copying if to_json_string accepted bytes_view
out << to_json_string(**ti, **vi);
out << to_json_string(**ti, **vi, false);
} else {
out << "null";
}
Expand All @@ -498,7 +512,8 @@ static sstring to_json_string_aux(const user_type_impl& t, bytes_view bv) {
namespace {
struct to_json_string_visitor {
bytes_view bv;
sstring operator()(const reversed_type_impl& t) { return to_json_string(*t.underlying_type(), bv); }
std::optional<bool> alternator;
sstring operator()(const reversed_type_impl& t) { return to_json_string(*t.underlying_type(), bv, alternator.value()); }
template <typename T> sstring operator()(const integer_type_impl<T>& t) { return to_sstring(compose_value(t, bv)); }
template <typename T> sstring operator()(const floating_type_impl<T>& t) {
if (bv.empty()) {
Expand All @@ -518,7 +533,7 @@ struct to_json_string_visitor {
sstring operator()(const boolean_type_impl& t) { return t.to_string(bv); }
sstring operator()(const timestamp_date_base_class& t) { return quote_json_string(timestamp_to_json_string(t, bv)); }
sstring operator()(const timeuuid_type_impl& t) { return quote_json_string(t.to_string(bv)); }
sstring operator()(const map_type_impl& t) { return to_json_string_aux(t, bv); }
sstring operator()(const map_type_impl& t) { return to_json_string_aux(t, bv, alternator); }
sstring operator()(const set_type_impl& t) { return to_json_string_aux(t, bv); }
sstring operator()(const list_type_impl& t) { return to_json_string_aux(t, bv); }
sstring operator()(const tuple_type_impl& t) { return to_json_string_aux(t, bv); }
Expand All @@ -535,7 +550,7 @@ struct to_json_string_visitor {
}
sstring operator()(const counter_type_impl& t) {
// It will be called only from cql3 layer while processing query results.
return to_json_string(*counter_cell_view::total_value_type(), bv);
return to_json_string(*counter_cell_view::total_value_type(), bv, false);
}
sstring operator()(const decimal_type_impl& t) {
if (bv.empty()) {
Expand All @@ -554,10 +569,10 @@ struct to_json_string_visitor {
};
}

sstring to_json_string(const abstract_type& t, bytes_view bv) {
return visit(t, to_json_string_visitor{bv});
sstring to_json_string(const abstract_type& t, bytes_view bv, std::optional<bool> alternator) {
return visit(t, to_json_string_visitor{bv, alternator});
}

sstring to_json_string(const abstract_type& t, const managed_bytes_view& mbv) {
return visit(t, to_json_string_visitor{linearized(mbv)});
sstring to_json_string(const abstract_type& t, const managed_bytes_view& mbv, std::optional<bool> alternator) {
return visit(t, to_json_string_visitor{linearized(mbv), alternator});
}
12 changes: 6 additions & 6 deletions cql3/type_json.hh
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@
#include "utils/rjson.hh"

bytes from_json_object(const abstract_type &t, const rjson::value& value);
sstring to_json_string(const abstract_type &t, bytes_view bv);
sstring to_json_string(const abstract_type &t, const managed_bytes_view& bv);
sstring to_json_string(const abstract_type &t, bytes_view bv, std::optional<bool> alternator);
sstring to_json_string(const abstract_type &t, const managed_bytes_view& bv, std::optional<bool> alternator);

inline sstring to_json_string(const abstract_type &t, const bytes& b) {
return to_json_string(t, bytes_view(b));
inline sstring to_json_string(const abstract_type &t, const bytes& b, std::optional<bool> alternator = std::nullopt) {
return to_json_string(t, bytes_view(b), alternator);
}

inline sstring to_json_string(const abstract_type& t, const bytes_opt& b) {
return b ? to_json_string(t, *b) : "null";
inline sstring to_json_string(const abstract_type& t, const bytes_opt& b, std::optional<bool> alternator = std::nullopt) {
return b ? to_json_string(t, *b, alternator) : "null";
}
2 changes: 2 additions & 0 deletions test/boost/expr_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
#include "test/lib/test_utils.hh"
#include "cql3/expr/evaluate.hh"
#include "cql3/expr/expr-utils.hh"
#include "alternator/serialization.hh"
#include "alternator/executor.hh"

using namespace cql3;
using namespace cql3::expr;
Expand Down
2 changes: 2 additions & 0 deletions test/raft/raft_address_map_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
#include "service/raft/raft_address_map.hh"
#include "gms/inet_address.hh"
#include "utils/UUID.hh"
#include "alternator/serialization.hh"
#include "alternator/executor.hh"

#include <seastar/core/coroutine.hh>
#include <seastar/core/manual_clock.hh>
Expand Down
2 changes: 2 additions & 0 deletions test/raft/raft_sys_table_storage_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
#include "cql3/query_processor.hh"

#include "gms/inet_address_serializer.hh"
#include "alternator/serialization.hh"
#include "alternator/executor.hh"

namespace raft{

Expand Down

0 comments on commit a60a524

Please sign in to comment.