cql: fix SELECT toJson() or SELECT JSON of time column

The implementation of "SELECT TOJSON(t)" or "SELECT JSON t" for a column
of type "time" forgot to put the time string in quotes. The result was
invalid JSON. This is patch is a one-liner fixing this bug.

This patch also removes the "xfail" marker from one xfailing test
for this issue which now starts to pass. We also add a second test for
this issue - the existing test was for "SELECT TOJSON(t)", and the second
test shows that "SELECT JSON t" had exactly the same bug - and both are
fixed by the same patch.

We also had a test translated from Cassandra which exposed this bug,
but that test continues to fail because of other bugs, so we just
need to update the xfail string.

The patch also fixes one C++ test, test/boost/json_cql_query_test.cc,
which enshrined the *wrong* behavior - JSON output that isn't even
valid JSON - and had to be fixed. Unlike the Python tests, the C++ test
can't be run against Cassandra, and doesn't even run a JSON parser
on the output, which explains how it came to enshrine wrong output
instead of helping to discover the bug.

Fixes #7988

Signed-off-by: Nadav Har'El <nyh@scylladb.com>

Closes #16121

(cherry picked from commit 8d04032)

cql3/type_json.cc

@@ -502,7 +502,7 @@ struct to_json_string_visitor {
     sstring operator()(const tuple_type_impl& t) { return to_json_string_aux(t, bv); }
     sstring operator()(const user_type_impl& t) { return to_json_string_aux(t, bv); }
     sstring operator()(const simple_date_type_impl& t) { return quote_json_string(t.to_string(bv)); }
-    sstring operator()(const time_type_impl& t) { return t.to_string(bv); }
+    sstring operator()(const time_type_impl& t) { return quote_json_string(t.to_string(bv)); }
     sstring operator()(const empty_type_impl& t) { return "null"; }
     sstring operator()(const duration_type_impl& t) {
         auto v = t.deserialize(bv);

test/boost/json_cql_query_test.cc

@@ -104,7 +104,7 @@ SEASTAR_TEST_CASE(test_select_json_types) {
                     "\"p\": 3, "
                     "\"q\": 3, "
                     "\"r\": \"1970-01-02\", "
-                    "\"s\": 00:00:00.000000001, "
+                    "\"s\": \"00:00:00.000000001\", "
                     "\"u\": \"1y2mo25d5h6m7s8ms9us10ns\", "
                     "\"w\": null, "
                     "\"system.unixtimestampof(k)\": 1261009589805}"
@@ -136,7 +136,7 @@ SEASTAR_TEST_CASE(test_select_json_types) {
                 utf8_type->decompose("3"),
                 utf8_type->decompose("3"),
                 utf8_type->decompose("\"1970-01-02\""),
-                utf8_type->decompose("00:00:00.000000001"),
+                utf8_type->decompose("\"00:00:00.000000001\""),
                 utf8_type->decompose("\"1y2mo25d5h6m7s8ms9us10ns\""),
                 utf8_type->decompose("null"),
                 utf8_type->decompose("1261009589805"),

test/cql-pytest/cassandra_tests/validation/entities/json_test.py

@@ -680,7 +680,7 @@ def __repr__(self):
         return f'EquivalentIp("{self.obj}")'
 
 # Reproduces issue #7972, #7988, #7997, #8001
-@pytest.mark.xfail(reason="issues #7972, #7988, #7997, #8001")
+@pytest.mark.xfail(reason="issues #7972, #7997, #8001")
 def testToJsonFct(cql, test_keyspace):
     abc_tuple = collections.namedtuple('abc_tuple', ['a', 'b', 'c'])
     with create_type(cql, test_keyspace, "(a int, b uuid, c set<text>)") as type_name:

test/cql-pytest/test_json.py

@@ -34,6 +34,25 @@ def table1(cql, test_keyspace, type1):
     yield table
     cql.execute("DROP TABLE " + table)
 
+# The EquivalentJson class wraps a JSON string, and compare equal to other
+# strings if both are valid JSON strings which decode to the same object.
+# EquivalentJson("....") can be used in equality assertions below, to check
+# whether functionally-equivalent JSON is returned instead of checking for
+# identical strings.
+class EquivalentJson:
+    def __init__(self, s):
+        self.obj = json.loads(s)
+    def __eq__(self, other):
+        if isinstance(other, EquivalentJson):
+            return self.obj == other.obj
+        elif isinstance(other, str):
+            return self.obj == json.loads(other)
+        return NotImplemented
+    # Implementing __repr__ is useful because when a comparison fails, pytest
+    # helpfully prints what it tried to compare, and uses __repr__ for that.
+    def __repr__(self):
+        return f'EquivalentJson("{self.obj}")'
+
 # Test that failed fromJson() parsing an invalid JSON results in the expected
 # error - FunctionFailure - and not some weird internal error.
 # Reproduces issue #7911.
@@ -299,14 +318,22 @@ def test_tojson_double(cql, table1):
 
 # Check that toJson() correctly formats "time" values. The JSON translation
 # is a string containing the time (there is no time type in JSON), and of
-# course, a string needs to be wrapped in quotes. (issue #7988
-@pytest.mark.xfail(reason="issue #7988")
+# course, a string needs to be wrapped in quotes.
+# Reproduces issue #7988.
 def test_tojson_time(cql, table1):
     p = unique_key_int()
     stmt = cql.prepare(f"INSERT INTO {table1} (p, t) VALUES (?, ?)")
     cql.execute(stmt, [p, 123])
     assert list(cql.execute(f"SELECT toJson(t) from {table1} where p = {p}")) == [('"00:00:00.000000123"',)]
 
+# Test the same thing in test_tojson_time above, with SELECT JSON instead
+# of SELECT toJson(). Also reproduces issue #7988.
+def test_select_json_time(cql, table1):
+    p = unique_key_int()
+    stmt = cql.prepare(f"INSERT INTO {table1} (p, t) VALUES (?, ?)")
+    cql.execute(stmt, [p, 123])
+    assert list(cql.execute(f"SELECT JSON t from {table1} where p = {p}")) == [(EquivalentJson('{"t": "00:00:00.000000123"}'),)]
+
 # Check that toJson() returns timestamp string in correct cassandra compatible format (issue #7997)
 # with milliseconds and timezone specification
 def test_tojson_timestamp(cql, table1):
@@ -315,25 +342,6 @@ def test_tojson_timestamp(cql, table1):
     cql.execute(stmt, [p, datetime(2014, 1, 1, 12, 15, 45)])
     assert list(cql.execute(f"SELECT toJson(ts) from {table1} where p = {p}")) == [('"2014-01-01 12:15:45.000Z"',)]
 
-# The EquivalentJson class wraps a JSON string, and compare equal to other
-# strings if both are valid JSON strings which decode to the same object.
-# EquivalentJson("....") can be used in assert_rows() checks below, to check
-# whether functionally-equivalent JSON is returned instead of checking for
-# identical strings.
-class EquivalentJson:
-    def __init__(self, s):
-        self.obj = json.loads(s)
-    def __eq__(self, other):
-        if isinstance(other, EquivalentJson):
-            return self.obj == other.obj
-        elif isinstance(other, str):
-            return self.obj == json.loads(other)
-        return NotImplemented
-    # Implementing __repr__ is useful because when a comparison fails, pytest
-    # helpfully prints what it tried to compare, and uses __repr__ for that.
-    def __repr__(self):
-        return f'EquivalentJson("{self.obj}")'
-
 # Test that toJson() can prints a decimal type with a very high mantissa.
 # Reproduces issue #8002, where it was written as 1 and a billion zeroes,
 # running out of memory.