LibSQL: Redesign Value implementation and add new types

The implemtation of the Value class was based on lambda member variables
implementing type-dependent behaviour. This was done to ensure that
Values can be used as stack-only objects; the simplest alternative,
virtual methods, forces them onto the heap. The problem with the the
lambda approach is that it bloats the Values (which are supposed to be
lightweight objects) quite considerably, because every object contains
more than a dozen function pointers.

The solution to address both problems (we want Values to be able to live
on the stack and be as lightweight as possible) chosen here is to
encapsulate type-dependent behaviour and state in an implementation
class, and let the Value be an AK::Variant of those implementation
classes. All methods of Value are now basically straight delegates to
the implementation object using the Variant::visit method.

One issue complicating matters is the addition of two aggregate types,
Tuple and Array, which each contain a Vector of Values. At this point
Tuples and Arrays (and potential future aggregate types) can't contain
these aggregate types. This is limiting and needs to be addressed.

Another area that needs attention is the nomenclature of things; it's
a bit of a tangle of 'ValueBlahBlah' and 'ImplBlahBlah'. It makes sense
right now I think but admit we probably can do better.

Other things included here:
- Added the Boolean and Null types (and Tuple and Array, see above).
- to_string now always succeeds and returns a String instead of an
  Optional. This had some impact on other sources.
- Added a lot of tests.
- Started moving the serialization mechanism more towards where I want
  it to be, i.e. a 'DataSerializer' object which just takes
  serialization and deserialization requests and knows for example how
  to store long strings out-of-line.

One last remark: There is obviously a naming clash between the Tuple
class and the Tuple Value type. This is intentional; I plan to make the
Tuple class a subclass of Value (and hence Key and Row as well).

2 files changed, 384 insertions, 43 deletions

diff --git a/Tests/LibSQL/TestSqlDatabase.cpp b/Tests/LibSQL/TestSqlDatabase.cpp
index a63187ac66..9c67267c5e 100644
--- a/Tests/LibSQL/TestSqlDatabase.cpp
+++ b/Tests/LibSQL/TestSqlDatabase.cpp
@@ -66,7 +66,7 @@ void verify_table_contents(SQL::Database& db, int expected_count)
     for (auto& row : db.select_all(*table)) {
         StringBuilder builder;
         builder.appendff("Test{}", row["IntColumn"].to_int().value());
-        EXPECT_EQ(row["TextColumn"].to_string().value(), builder.build());
+        EXPECT_EQ(row["TextColumn"].to_string(), builder.build());
         count++;
         sum += row["IntColumn"].to_int().value();
     }
diff --git a/Tests/LibSQL/TestSqlValueAndTuple.cpp b/Tests/LibSQL/TestSqlValueAndTuple.cpp
index 2c3d6e25d3..cbedb15836 100644
--- a/Tests/LibSQL/TestSqlValueAndTuple.cpp
+++ b/Tests/LibSQL/TestSqlValueAndTuple.cpp
@@ -12,76 +12,203 @@
 #include <LibSQL/Value.h>
 #include <LibTest/TestCase.h>
 
-TEST_CASE(text_value)
+TEST_CASE(null_value)
 {
-    SQL::Value v(SQL::SQLType::Text);
+    SQL::Value v(SQL::SQLType::Null);
+    EXPECT(v.type() == SQL::SQLType::Null);
+    EXPECT(v.is_null());
     v = "Test";
-    VERIFY(v.to_string().value() == "Test");
+    EXPECT(v.is_null());
+    EXPECT(v.to_string() == "(null)");
 }
 
-TEST_CASE(text_value_to_int)
+TEST_CASE(text_value)
 {
-    SQL::Value v(SQL::SQLType::Text);
-    v = "42";
-    EXPECT_EQ(v.to_int().value(), 42);
+    {
+        SQL::Value v(SQL::SQLType::Text);
+        EXPECT(v.is_null());
+        v = "Test";
+        EXPECT(!v.is_null());
+        EXPECT(v.to_string() == "Test");
+    }
+    {
+        SQL::Value v(SQL::SQLType::Text, String("String Test"));
+        EXPECT(!v.is_null());
+        EXPECT(v.to_string() == "String Test");
+    }
+    {
+        SQL::Value v(SQL::SQLType::Text, "const char * Test");
+        EXPECT(!v.is_null());
+        EXPECT_EQ(v.to_string(), "const char * Test");
+    }
+    {
+        SQL::Value v(String("String Test"));
+        EXPECT(v.type() == SQL::SQLType::Text);
+        EXPECT(!v.is_null());
+        EXPECT(v.to_string() == "String Test");
+    }
+    {
+        SQL::Value v(SQL::SQLType::Text, SQL::Value(42));
+        EXPECT(v.type() == SQL::SQLType::Text);
+        EXPECT(!v.is_null());
+        EXPECT(v.to_string() == "42");
+    }
+}
+
+TEST_CASE(assign_null)
+{
+    SQL::Value v("Test");
+    EXPECT(!v.is_null());
+    v = SQL::Value::null();
+    EXPECT(v.is_null());
+}
+
+TEST_CASE(text_value_to_other_types)
+{
+    {
+        SQL::Value v(SQL::SQLType::Text, "42");
+        EXPECT(v.to_int().has_value());
+        EXPECT_EQ(v.to_int().value(), 42);
+        EXPECT(v.to_double().has_value());
+        EXPECT(v.to_double().value() - 42.0 < NumericLimits<double>().epsilon());
+    }
+    {
+        SQL::Value v("true");
+        EXPECT(v.to_bool().has_value());
+        EXPECT(v.to_bool().value());
+    }
+    {
+        SQL::Value v("false");
+        EXPECT(v.to_bool().has_value());
+        EXPECT(!v.to_bool().value());
+    }
 }
 
 TEST_CASE(text_value_to_int_crash)
 {
-    SQL::Value v(SQL::SQLType::Text);
-    v = "Test";
-    EXPECT_CRASH("Can't convert 'Test' to integer", [&]() { (void) (int) v; return Test::Crash::Failure::DidNotCrash; });
+    SQL::Value v(SQL::SQLType::Text, "Not a valid integer");
+    EXPECT_CRASH("Can't convert 'Not a valid integer' to integer", [&]() { (void) (int) v; return Test::Crash::Failure::DidNotCrash; });
 }
 
 TEST_CASE(serialize_text_value)
 {
-    SQL::Value v(SQL::SQLType::Text);
-    v = "Test";
-    VERIFY(v.to_string().value() == "Test");
+    SQL::Value v("Test");
+    EXPECT(v.to_string() == "Test");
 
     ByteBuffer buffer;
-    v.serialize(buffer);
+    v.serialize_to(buffer);
 
     size_t offset = 0;
-    SQL::Value v2(SQL::SQLType::Text, buffer, offset);
-    VERIFY((String)v2 == "Test");
+    auto v2 = SQL::Value::deserialize_from(buffer, offset);
+    EXPECT((String)v2 == "Test");
 }
 
 TEST_CASE(integer_value)
 {
-    SQL::Value v(SQL::SQLType::Integer);
-    v = 42;
-    VERIFY(v.to_int().value() == 42);
+    {
+        SQL::Value v(SQL::SQLType::Integer);
+        EXPECT(v.is_null());
+        v = 42;
+        EXPECT(!v.is_null());
+        EXPECT(v.to_int().value() == 42);
+        EXPECT(v.to_string() == "42");
+        EXPECT(v.to_double().has_value());
+        EXPECT(v.to_double().value() - 42.0 < NumericLimits<double>().epsilon());
+        EXPECT(v.to_bool().has_value());
+        EXPECT(v.to_bool().value());
+    }
+    {
+        SQL::Value v(0);
+        EXPECT(!v.is_null());
+        EXPECT(v.to_int().value() == 0);
+        EXPECT(v.to_bool().has_value());
+        EXPECT(!v.to_bool().value());
+    }
+    {
+        SQL::Value v(SQL::SQLType::Integer, "42");
+        EXPECT_EQ(v.to_int().value(), 42);
+    }
+    {
+        SQL::Value v(SQL::SQLType::Integer, SQL::Value("42"));
+        EXPECT_EQ(v.to_int().value(), 42);
+    }
+    {
+        SQL::Value text("42");
+        SQL::Value integer(SQL::SQLType::Integer);
+        integer = text;
+        EXPECT_EQ(integer.to_int().value(), 42);
+    }
 }
 
 TEST_CASE(serialize_int_value)
 {
-    SQL::Value v(SQL::SQLType::Text);
-    v = 42;
-    VERIFY(v.to_int().value() == 42);
+    SQL::Value v(42);
+    EXPECT_EQ(v.type(), SQL::SQLType::Integer);
+    EXPECT_EQ(v.to_int().value(), 42);
 
     ByteBuffer buffer;
-    v.serialize(buffer);
+    v.serialize_to(buffer);
 
     size_t offset = 0;
-    SQL::Value v2(SQL::SQLType::Text, buffer, offset);
-    VERIFY(v2 == v);
+    auto v2 = SQL::Value::deserialize_from(buffer, offset);
+    EXPECT(!v2.is_null());
+    EXPECT_EQ(v2.type(), SQL::SQLType::Integer);
+    EXPECT_EQ(v2.to_int().value(), 42);
+    EXPECT(v2 == v);
 }
 
 TEST_CASE(float_value)
 {
-    SQL::Value v(SQL::SQLType::Float);
-    v = 3.14;
-    VERIFY(v.to_double().value() - 3.14 < 0.001);
+    {
+        SQL::Value v(SQL::SQLType::Float);
+        EXPECT(v.is_null());
+        v = 3.14;
+        EXPECT(!v.is_null());
+        EXPECT(v.to_double().has_value());
+        EXPECT(v.to_double().value() - 3.14 < NumericLimits<double>().epsilon());
+        EXPECT(v.to_int().has_value());
+        EXPECT_EQ(v.to_int().value(), 3);
+        EXPECT_EQ(v.to_string(), "3.14");
+        EXPECT(!v.to_bool().has_value());
+    }
+    {
+        SQL::Value v(3.14);
+        EXPECT(!v.is_null());
+        EXPECT(v.to_double().value() - 3.14 < NumericLimits<double>().epsilon());
+    }
+    {
+        SQL::Value v(3.51);
+        EXPECT(!v.is_null());
+        EXPECT_EQ(v.to_int().value(), 4);
+    }
+    {
+        SQL::Value v(-3.14);
+        EXPECT_EQ(v.to_int().value(), -3);
+    }
+    {
+        SQL::Value v(-3.51);
+        EXPECT_EQ(v.to_int().value(), -4);
+    }
+    {
+        SQL::Value v(SQL::SQLType::Float, "3.14");
+        EXPECT(v.to_double().value() - 3.14 < NumericLimits<double>().epsilon());
+    }
 }
 
-TEST_CASE(assign_text_value_to_int)
+TEST_CASE(serialize_float_value)
 {
-    SQL::Value text(SQL::SQLType::Text);
-    text = "42";
-    SQL::Value integer(SQL::SQLType::Integer);
-    integer = text;
-    EXPECT_EQ(integer.to_int().value(), 42);
+    SQL::Value v(3.14);
+    EXPECT_EQ(v.type(), SQL::SQLType::Float);
+    EXPECT(v.to_double().value() - 3.14 < NumericLimits<double>().epsilon());
+
+    ByteBuffer buffer;
+    v.serialize_to(buffer);
+
+    size_t offset = 0;
+    auto v2 = SQL::Value::deserialize_from(buffer, offset);
+    EXPECT(!v2.is_null());
+    EXPECT_EQ(v2.type(), SQL::SQLType::Float);
+    EXPECT(v.to_double().value() - 3.14 < NumericLimits<double>().epsilon());
 }
 
 TEST_CASE(assign_int_to_text_value)
@@ -93,8 +220,7 @@ TEST_CASE(assign_int_to_text_value)
 
 TEST_CASE(copy_value)
 {
-    SQL::Value text(SQL::SQLType::Text);
-    text = 42;
+    SQL::Value text(SQL::SQLType::Text, 42);
     SQL::Value copy(text);
     EXPECT_EQ((String)copy, "42");
 }
@@ -109,6 +235,221 @@ TEST_CASE(compare_text_to_int)
     EXPECT(integer == text);
 }
 
+TEST_CASE(bool_value)
+{
+    {
+        SQL::Value v(SQL::SQLType::Boolean);
+        EXPECT(v.is_null());
+        v = true;
+        EXPECT(!v.is_null());
+        EXPECT(v.to_bool().has_value());
+        EXPECT(v.to_bool().value());
+        EXPECT_EQ(v.to_int().value(), 1);
+        EXPECT_EQ(v.to_string(), "true");
+        EXPECT(!v.to_double().has_value());
+    }
+    {
+        SQL::Value v(SQL::SQLType::Boolean, false);
+        EXPECT(!v.is_null());
+        EXPECT(v.to_bool().has_value());
+        EXPECT(!v.to_bool().value());
+        EXPECT_EQ(v.to_int().value(), 0);
+        EXPECT_EQ(v.to_string(), "false");
+        EXPECT(!v.to_double().has_value());
+    }
+    {
+        SQL::Value v(true);
+        EXPECT_EQ(v.type(), SQL::SQLType::Boolean);
+        EXPECT(!v.is_null());
+        EXPECT(v.to_bool().has_value());
+        EXPECT(v.to_bool().value());
+    }
+}
+
+TEST_CASE(serialize_boolean_value)
+{
+    SQL::Value v(true);
+    EXPECT_EQ(v.type(), SQL::SQLType::Boolean);
+    EXPECT(bool(v));
+
+    ByteBuffer buffer;
+    v.serialize_to(buffer);
+
+    size_t offset = 0;
+    auto v2 = SQL::Value::deserialize_from(buffer, offset);
+    EXPECT(!v2.is_null());
+    EXPECT_EQ(v2.type(), SQL::SQLType::Boolean);
+    EXPECT(bool(v2));
+    EXPECT_EQ(v, v2);
+}
+
+TEST_CASE(tuple_value)
+{
+    NonnullRefPtr<SQL::TupleDescriptor> descriptor = adopt_ref(*new SQL::TupleDescriptor);
+    descriptor->append({ "col1", SQL::SQLType::Text, SQL::Order::Ascending });
+    descriptor->append({ "col2", SQL::SQLType::Integer, SQL::Order::Descending });
+
+    auto v = SQL::Value::create_tuple(descriptor);
+    Vector<SQL::Value> values;
+    values.append(SQL::Value("Test"));
+    values.append(SQL::Value(42));
+    v = values;
+
+    auto values2 = v.to_vector();
+    EXPECT(values2.has_value());
+    EXPECT_EQ(values, values2.value());
+}
+
+TEST_CASE(copy_tuple_value)
+{
+    NonnullRefPtr<SQL::TupleDescriptor> descriptor = adopt_ref(*new SQL::TupleDescriptor);
+    descriptor->append({ "col1", SQL::SQLType::Text, SQL::Order::Ascending });
+    descriptor->append({ "col2", SQL::SQLType::Integer, SQL::Order::Descending });
+
+    auto v = SQL::Value::create_tuple(descriptor);
+    Vector<SQL::Value> values;
+    values.append(SQL::Value("Test"));
+    values.append(SQL::Value(42));
+    v = values;
+
+    auto values2 = v;
+    EXPECT(values2.type() == v.type());
+    EXPECT(!values2.is_null());
+    EXPECT_EQ(values, values2.to_vector().value());
+}
+
+TEST_CASE(tuple_value_wrong_type)
+{
+    NonnullRefPtr<SQL::TupleDescriptor> descriptor = adopt_ref(*new SQL::TupleDescriptor);
+    descriptor->append({ "col1", SQL::SQLType::Text, SQL::Order::Ascending });
+
+    auto v = SQL::Value::create_tuple(descriptor);
+    Vector<SQL::Value> values;
+    values.append(SQL::Value(42));
+    v = values;
+    EXPECT(v.is_null());
+}
+
+TEST_CASE(tuple_value_too_many_values)
+{
+    NonnullRefPtr<SQL::TupleDescriptor> descriptor = adopt_ref(*new SQL::TupleDescriptor);
+    descriptor->append({ "col1", SQL::SQLType::Text, SQL::Order::Ascending });
+
+    auto v = SQL::Value::create_tuple(descriptor);
+    Vector<SQL::Value> values;
+    values.append(SQL::Value("Test"));
+    values.append(SQL::Value(42));
+    v = values;
+    EXPECT(v.is_null());
+}
+
+TEST_CASE(tuple_value_not_enough_values)
+{
+    NonnullRefPtr<SQL::TupleDescriptor> descriptor = adopt_ref(*new SQL::TupleDescriptor);
+    descriptor->append({ "col1", SQL::SQLType::Text, SQL::Order::Ascending });
+    descriptor->append({ "col2", SQL::SQLType::Integer, SQL::Order::Ascending });
+
+    auto v = SQL::Value::create_tuple(descriptor);
+    Vector<SQL::Value> values;
+    values.append(SQL::Value("Test"));
+    v = values;
+    EXPECT(!v.is_null());
+    auto values_opt = v.to_vector();
+    EXPECT(values_opt.has_value());
+    EXPECT_EQ(values_opt.value().size(), 2u);
+    auto col2 = values_opt.value()[1];
+    EXPECT_EQ(col2.type(), SQL::SQLType::Integer);
+    EXPECT(col2.is_null());
+}
+
+TEST_CASE(serialize_tuple_value)
+{
+    NonnullRefPtr<SQL::TupleDescriptor> descriptor = adopt_ref(*new SQL::TupleDescriptor);
+    descriptor->append({ "col1", SQL::SQLType::Text, SQL::Order::Ascending });
+    descriptor->append({ "col2", SQL::SQLType::Integer, SQL::Order::Descending });
+
+    auto v = SQL::Value::create_tuple(descriptor);
+    Vector<SQL::Value> values;
+    values.append(SQL::Value("Test"));
+    values.append(SQL::Value(42));
+    v = values;
+
+    ByteBuffer buffer;
+    v.serialize_to(buffer);
+
+    size_t offset = 0;
+    auto v2 = SQL::Value::deserialize_from(buffer, offset);
+    EXPECT(!v2.is_null());
+    EXPECT_EQ(v2.type(), SQL::SQLType::Tuple);
+    EXPECT_EQ(v, v2);
+}
+
+TEST_CASE(array_value)
+{
+    auto v = SQL::Value::create_array(SQL::SQLType::Text, 3);
+    Vector<SQL::Value> values;
+    values.append(SQL::Value("Test 1"));
+    values.append(SQL::Value("Test 2"));
+    v = values;
+
+    auto values2 = v.to_vector();
+    EXPECT(values2.has_value());
+    EXPECT_EQ(values, values2.value());
+}
+
+TEST_CASE(array_value_wrong_type)
+{
+    auto v = SQL::Value::create_array(SQL::SQLType::Text, 2);
+    Vector<SQL::Value> values;
+    values.append(SQL::Value("Test 1"));
+    values.append(SQL::Value(42));
+    v = values;
+    EXPECT(v.is_null());
+}
+
+TEST_CASE(array_value_too_many_values)
+{
+    auto v = SQL::Value::create_array(SQL::SQLType::Text, 2);
+    Vector<SQL::Value> values;
+    values.append(SQL::Value("Test 1"));
+    values.append(SQL::Value("Test 2"));
+    values.append(SQL::Value("Test 3"));
+    v = values;
+    EXPECT(v.is_null());
+}
+
+TEST_CASE(copy_array_value)
+{
+    auto v = SQL::Value::create_array(SQL::SQLType::Text, 3);
+    Vector<SQL::Value> values;
+    values.append(SQL::Value("Test 1"));
+    values.append(SQL::Value("Test 2"));
+    v = values;
+
+    auto values2 = v;
+    EXPECT(values2.type() == v.type());
+    EXPECT(!values2.is_null());
+    EXPECT_EQ(values, values2.to_vector().value());
+}
+
+TEST_CASE(serialize_array_value)
+{
+    auto v = SQL::Value::create_array(SQL::SQLType::Text, 3);
+    Vector<SQL::Value> values;
+    values.append(SQL::Value("Test 1"));
+    values.append(SQL::Value("Test 2"));
+    v = values;
+
+    ByteBuffer buffer;
+    v.serialize_to(buffer);
+
+    size_t offset = 0;
+    auto v2 = SQL::Value::deserialize_from(buffer, offset);
+    EXPECT(!v2.is_null());
+    EXPECT_EQ(v2.type(), SQL::SQLType::Array);
+    EXPECT_EQ(v, v2);
+}
+
 TEST_CASE(order_text_values)
 {
     SQL::Value v1(SQL::SQLType::Text);
@@ -142,8 +483,8 @@ TEST_CASE(tuple)
 
     tuple["col1"] = "Test";
     tuple["col2"] = 42;
-    VERIFY(tuple[0] == "Test");
-    VERIFY(tuple[1] == 42);
+    EXPECT(tuple[0] == "Test");
+    EXPECT(tuple[1] == 42);
 }
 
 TEST_CASE(serialize_tuple)
@@ -163,8 +504,8 @@ TEST_CASE(serialize_tuple)
 
     size_t offset = 0;
     SQL::Tuple tuple2(descriptor, buffer, offset);
-    VERIFY(tuple2[0] == "Test");
-    VERIFY(tuple2[1] == 42);
+    EXPECT(tuple2[0] == "Test");
+    EXPECT(tuple2[1] == 42);
 }
 
 TEST_CASE(copy_tuple)
@@ -179,10 +520,10 @@ TEST_CASE(copy_tuple)
 
     SQL::Tuple copy;
     copy = tuple;
-    VERIFY(tuple == copy);
+    EXPECT(tuple == copy);
 
     SQL::Tuple copy_2(copy);
-    VERIFY(tuple == copy_2);
+    EXPECT(tuple == copy_2);
 }
 
 TEST_CASE(compare_tuples)