cql3: Fix invalid JSON parsing for JSON objects with ASCII keys

For JSON objects represented as map<ascii, int>, don't treat ASCII keys
as a nested JSON string. We were doing that prior to the patch, which
led to parsing errors.

Included the error offset where JSON parsing failed for
rjson::parse related functions to help identify parsing errors
better.

Fixes: #7949

Signed-off-by: Michael Huang <michaelhly@gmail.com>

Closes #15499

cql3/type_json.cc

@@ -151,14 +151,19 @@ static bytes from_json_object_aux(const map_type_impl& t, const rjson::value& va
     std::map<bytes, bytes, serialized_compare> raw_map(t.get_keys_type()->as_less_comparator());
     for (auto it = value.MemberBegin(); it != value.MemberEnd(); ++it) {
         bytes value = from_json_object(*t.get_values_type(), it->value);
-        if (!t.get_keys_type()->is_compatible_with(*utf8_type)) {
+        if (t.get_keys_type()->underlying_type() == ascii_type ||
+            t.get_keys_type()->underlying_type() == utf8_type) {
+            raw_map.emplace(from_json_object(*t.get_keys_type(), it->name), std::move(value));
+        } else {
             // Keys in maps can only be strings in JSON, but they can also be a string representation
             // of another JSON type, which needs to be reparsed. Example - map<frozen<list<int>>, int>
             // will be represented like this: { "[1, 3, 6]": 3, "[]": 0, "[1, 2]": 2 }
-            rjson::value map_key = rjson::parse(rjson::to_string_view(it->name));
-            raw_map.emplace(from_json_object(*t.get_keys_type(), map_key), std::move(value));
-        } else {
-            raw_map.emplace(from_json_object(*t.get_keys_type(), it->name), std::move(value));
+            try {
+                rjson::value map_key = rjson::parse(rjson::to_string_view(it->name));
+                raw_map.emplace(from_json_object(*t.get_keys_type(), map_key), std::move(value));
+            } catch (rjson::error& e) {
+                throw marshal_exception(format("Failed parsing map_key {}: {}", it->name, e.what()));
+            }
         }
     }
     return map_type_impl::serialize_to_bytes(raw_map);

test/cql-pytest/test_json.py

@@ -241,12 +241,10 @@ def test_fromjson_null_prepared(cql, table1):
 # Test that fromJson can parse a map<ascii,int>. Strangely Scylla had a bug
 # setting a map<ascii,int> with fromJson(), while map<text,int> worked well.
 # Reproduces #7949.
-@pytest.mark.xfail(reason="issue #7949")
 def test_fromjson_map_ascii_unprepared(cql, table1):
     p = unique_key_int()
     cql.execute("INSERT INTO " + table1 + " (p, mai) VALUES (" + str(p) + ", fromJson('{\"a\": 1, \"b\": 2}'))")
     assert list(cql.execute(f"SELECT p, mai from {table1} where p = {p}")) == [(p, {'a': 1, 'b': 2})]
-@pytest.mark.xfail(reason="issue #7949")
 def test_fromjson_map_ascii_prepared(cql, table1):
     p = unique_key_int()
     stmt = cql.prepare(f"INSERT INTO {table1} (p, mai) VALUES (?, fromJson(?))")

utils/rjson.cc

@@ -109,7 +109,9 @@ struct guarded_yieldable_json_handler : public Handler {
         rapidjson::GenericReader<encoding, encoding, allocator> reader(&the_allocator);
         reader.Parse(stream, *this);
         if (reader.HasParseError()) {
-            throw rjson::error(format("Parsing JSON failed: {}", rapidjson::GetParseError_En(reader.GetParseErrorCode())));
+            throw rjson::error(
+                format("Parsing JSON failed: {} at {}",
+                    rapidjson::GetParseError_En(reader.GetParseErrorCode()), reader.GetErrorOffset()));
         }
         //NOTICE: The handler has parsed the string, but in case of rapidjson::GenericDocument
         // the data now resides in an internal stack_ variable, which is private instead of
@@ -308,7 +310,8 @@ rjson::value parse(std::string_view str, size_t max_nested_level) {
     guarded_yieldable_json_handler<document, false> d(max_nested_level);
     d.Parse(str.data(), str.size());
     if (d.HasParseError()) {
-        throw rjson::error(format("Parsing JSON failed: {}", GetParseError_En(d.GetParseError())));
+        throw rjson::error(format("Parsing JSON failed: {} at {}",
+            GetParseError_En(d.GetParseError()), d.GetErrorOffset()));
     }
     rjson::value& v = d;
     return std::move(v);
@@ -318,7 +321,8 @@ rjson::value parse(chunked_content&& content, size_t max_nested_level) {
     guarded_yieldable_json_handler<document, false> d(max_nested_level);
     d.Parse(std::move(content));
     if (d.HasParseError()) {
-        throw rjson::error(format("Parsing JSON failed: {}", GetParseError_En(d.GetParseError())));
+        throw rjson::error(format("Parsing JSON failed: {} at {}",
+            GetParseError_En(d.GetParseError()), d.GetErrorOffset()));
     }
     rjson::value& v = d;
     return std::move(v);
@@ -342,7 +346,8 @@ rjson::value parse_yieldable(std::string_view str, size_t max_nested_level) {
     guarded_yieldable_json_handler<document, true> d(max_nested_level);
     d.Parse(str.data(), str.size());
     if (d.HasParseError()) {
-        throw rjson::error(format("Parsing JSON failed: {}", GetParseError_En(d.GetParseError())));
+        throw rjson::error(format("Parsing JSON failed: {} at {}",
+            GetParseError_En(d.GetParseError()), d.GetErrorOffset()));
     }
     rjson::value& v = d;
     return std::move(v);
@@ -352,7 +357,8 @@ rjson::value parse_yieldable(chunked_content&& content, size_t max_nested_level)
     guarded_yieldable_json_handler<document, true> d(max_nested_level);
     d.Parse(std::move(content));
     if (d.HasParseError()) {
-        throw rjson::error(format("Parsing JSON failed: {}", GetParseError_En(d.GetParseError())));
+        throw rjson::error(format("Parsing JSON failed: {} at {}",
+            GetParseError_En(d.GetParseError()), d.GetErrorOffset()));
     }
     rjson::value& v = d;
     return std::move(v);