LibSQL: Rewrite the SQL::Value type to be contained within one class

Currently, the Value class is essentially a "pImpl" wrapper around the
ValueImpl hierarchy of classes. This is a bit difficult to follow and
reason about, as methods jump between the Value class and its impl
classes.

This changes the Variant held by Value to instead store the specified
types (String, int, etc.) directly. In doing so, the ValueImpl classes
are removed, and all methods are now just concise Variant visitors.

As part of this rewrite, support for the "array" type is dropped (or
rather, just not re-implemented) as it was unused. If it's needed in the
future, support can be re-added.

This does retain the ability for non-NULL types to store NULL values
(i.e. an empty Optional). I tried dropping this support as well, but it
is depended upon by the on-disk storage classes in non-trivial ways.

Tests/LibSQL/TestSqlValueAndTuple.cpp

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2021, Jan de Visser <jan@de-visser.net>
+ * Copyright (c) 2022, Tim Flynn <trflynn89@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -15,121 +16,170 @@
 TEST_CASE(null_value)
 {
     SQL::Value v(SQL::SQLType::Null);
-    EXPECT(v.type() == SQL::SQLType::Null);
+    EXPECT_EQ(v.type(), SQL::SQLType::Null);
+    EXPECT_EQ(v.to_string(), "(null)"sv);
+    EXPECT(!v.to_bool().has_value());
+    EXPECT(!v.to_int().has_value());
+    EXPECT(!v.to_u32().has_value());
+    EXPECT(!v.to_double().has_value());
+}
+
+TEST_CASE(assign_null)
+{
+    SQL::Value v("Test");
+    EXPECT_EQ(v.type(), SQL::SQLType::Text);
+    EXPECT(!v.is_null());
+
+    v = SQL::Value();
+    EXPECT_EQ(v.type(), SQL::SQLType::Null);
     EXPECT(v.is_null());
-    v = "Test";
-    EXPECT(v.is_null());
-    EXPECT(v.to_string() == "(null)");
 }
 
 TEST_CASE(text_value)
 {
     {
         SQL::Value v(SQL::SQLType::Text);
+        EXPECT_EQ(v.type(), 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)
+        v = "Test"sv;
-{
+        EXPECT_EQ(v.type(), SQL::SQLType::Text);
-    SQL::Value v("Test");
+        EXPECT_EQ(v.to_string(), "Test"sv);
-    EXPECT(!v.is_null());
+    }
-    v = SQL::Value::null();
+    {
-    EXPECT(v.is_null());
+        SQL::Value v(String("String Test"sv));
+        EXPECT_EQ(v.type(), SQL::SQLType::Text);
+        EXPECT_EQ(v.to_string(), "String Test"sv);
+
+        v = String("String Test 2"sv);
+        EXPECT_EQ(v.type(), SQL::SQLType::Text);
+        EXPECT_EQ(v.to_string(), "String Test 2"sv);
+    }
+    {
+        SQL::Value v("const char * Test");
+        EXPECT_EQ(v.type(), SQL::SQLType::Text);
+        EXPECT_EQ(v.to_string(), "const char * Test"sv);
+
+        v = "const char * Test 2";
+        EXPECT_EQ(v.type(), SQL::SQLType::Text);
+        EXPECT_EQ(v.to_string(), "const char * Test 2"sv);
+    }
 }
 
 TEST_CASE(text_value_to_other_types)
 {
     {
-        SQL::Value v(SQL::SQLType::Text, "42");
+        SQL::Value v("42");
+        EXPECT_EQ(v.type(), SQL::SQLType::Text);
+
         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());
+        EXPECT((v.to_double().value() - 42.0) < NumericLimits<double>().epsilon());
     }
     {
         SQL::Value v("true");
+        EXPECT_EQ(v.type(), SQL::SQLType::Text);
+
         EXPECT(v.to_bool().has_value());
         EXPECT(v.to_bool().value());
     }
     {
         SQL::Value v("false");
+        EXPECT_EQ(v.type(), SQL::SQLType::Text);
+
         EXPECT(v.to_bool().has_value());
         EXPECT(!v.to_bool().value());
     }
+    {
+        SQL::Value v("foo");
+        EXPECT_EQ(v.type(), SQL::SQLType::Text);
+
+        EXPECT(!v.to_bool().has_value());
+        EXPECT(!v.to_int().has_value());
+        EXPECT(!v.to_u32().has_value());
+        EXPECT(!v.to_double().has_value());
+    }
+    {
+        SQL::Value v("3.14");
+        EXPECT_EQ(v.type(), SQL::SQLType::Text);
+
+        EXPECT(v.to_double().has_value());
+        EXPECT((v.to_double().value() - 3.14) < NumericLimits<double>().epsilon());
+    }
+}
+
+TEST_CASE(assign_int_to_text_value)
+{
+    SQL::Value v(SQL::SQLType::Text);
+    EXPECT_EQ(v.type(), SQL::SQLType::Text);
+    EXPECT(v.is_null());
+
+    v = 42;
+    EXPECT_EQ(v.type(), SQL::SQLType::Integer);
+    EXPECT_EQ(v, 42);
 }
 
 TEST_CASE(serialize_text_value)
 {
     SQL::Value v("Test");
-    EXPECT(v.to_string() == "Test");
+    EXPECT_EQ(v.type(), SQL::SQLType::Text);
+    EXPECT_EQ(v, "Test"sv);
 
     SQL::Serializer serializer;
     serializer.serialize<SQL::Value>(v);
 
     serializer.rewind();
     auto v2 = serializer.deserialize<SQL::Value>();
-    EXPECT(v2.to_string() == "Test");
+    EXPECT_EQ(v2.type(), SQL::SQLType::Text);
+    EXPECT_EQ(v2, "Test"sv);
+    EXPECT_EQ(v2, v);
 }
 
 TEST_CASE(integer_value)
 {
     {
         SQL::Value v(SQL::SQLType::Integer);
+        EXPECT_EQ(v.type(), SQL::SQLType::Integer);
         EXPECT(v.is_null());
+
         v = 42;
-        EXPECT(!v.is_null());
+        EXPECT_EQ(v.type(), SQL::SQLType::Integer);
-        EXPECT(v.to_int().value() == 42);
+
-        EXPECT(v.to_string() == "42");
+        EXPECT(v.to_int().has_value());
+        EXPECT_EQ(v.to_int().value(), 42);
+        EXPECT_EQ(v.to_string(), "42"sv);
+
         EXPECT(v.to_double().has_value());
-        EXPECT(v.to_double().value() - 42.0 < NumericLimits<double>().epsilon());
+        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_EQ(v.type(), SQL::SQLType::Integer);
-        EXPECT(v.to_int().value() == 0);
+
+        EXPECT(v.to_int().has_value());
+        EXPECT_EQ(v.to_int().value(), 0);
+
         EXPECT(v.to_bool().has_value());
         EXPECT(!v.to_bool().value());
     }
     {
-        SQL::Value v(SQL::SQLType::Integer, "42");
+        SQL::Value v(42);
-        EXPECT_EQ(v.to_int().value(), 42);
+        EXPECT_EQ(v.type(), SQL::SQLType::Integer);
-    }
+
-    {
+        EXPECT(v.to_int().has_value());
-        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(integer.to_int().has_value());
         EXPECT_EQ(integer.to_int().value(), 42);
     }
 }
@@ -138,66 +188,77 @@ TEST_CASE(serialize_int_value)
 {
     SQL::Value v(42);
     EXPECT_EQ(v.type(), SQL::SQLType::Integer);
-    EXPECT_EQ(v.to_int().value(), 42);
+    EXPECT_EQ(v, 42);
 
     SQL::Serializer serializer;
     serializer.serialize<SQL::Value>(v);
 
     serializer.rewind();
     auto v2 = serializer.deserialize<SQL::Value>();
-    EXPECT(!v2.is_null());
     EXPECT_EQ(v2.type(), SQL::SQLType::Integer);
-    EXPECT_EQ(v2.to_int().value(), 42);
+    EXPECT_EQ(v2, 42);
-    EXPECT(v2 == v);
+    EXPECT_EQ(v2, v);
 }
 
 TEST_CASE(float_value)
 {
     {
         SQL::Value v(SQL::SQLType::Float);
+        EXPECT_EQ(v.type(), SQL::SQLType::Float);
         EXPECT(v.is_null());
+
         v = 3.14;
-        EXPECT(!v.is_null());
+        EXPECT_EQ(v.type(), SQL::SQLType::Float);
+
         EXPECT(v.to_double().has_value());
-        EXPECT(v.to_double().value() - 3.14 < NumericLimits<double>().epsilon());
+        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());
         EXPECT(v.to_bool().value());
 
         v = 0.0;
-        EXPECT(!v.is_null());
+        EXPECT_EQ(v.type(), SQL::SQLType::Float);
+
         EXPECT(v.to_double().has_value());
         EXPECT(v.to_double().value() < NumericLimits<double>().epsilon());
+
         EXPECT(v.to_int().has_value());
         EXPECT_EQ(v.to_int().value(), 0);
-        EXPECT_EQ(v.to_string(), "0");
+        EXPECT_EQ(v.to_string(), "0"sv);
+
         EXPECT(v.to_bool().has_value());
         EXPECT(!v.to_bool().value());
     }
     {
         SQL::Value v(3.14);
-        EXPECT(!v.is_null());
+        EXPECT_EQ(v.type(), SQL::SQLType::Float);
-        EXPECT(v.to_double().value() - 3.14 < NumericLimits<double>().epsilon());
+        EXPECT((v.to_double().value() - 3.14) < NumericLimits<double>().epsilon());
     }
     {
         SQL::Value v(3.51);
-        EXPECT(!v.is_null());
+        EXPECT_EQ(v.type(), SQL::SQLType::Float);
+
+        EXPECT(v.to_int().has_value());
         EXPECT_EQ(v.to_int().value(), 4);
     }
     {
         SQL::Value v(-3.14);
+        EXPECT_EQ(v.type(), SQL::SQLType::Float);
+
+        EXPECT(v.to_int().has_value());
         EXPECT_EQ(v.to_int().value(), -3);
     }
     {
         SQL::Value v(-3.51);
+        EXPECT_EQ(v.type(), SQL::SQLType::Float);
+
+        EXPECT(v.to_int().has_value());
         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(serialize_float_value)
@@ -211,63 +272,73 @@ TEST_CASE(serialize_float_value)
 
     serializer.rewind();
     auto v2 = serializer.deserialize<SQL::Value>();
-    EXPECT(!v2.is_null());
     EXPECT_EQ(v2.type(), SQL::SQLType::Float);
-    EXPECT(v.to_double().value() - 3.14 < NumericLimits<double>().epsilon());
+    EXPECT((v.to_double().value() - 3.14) < NumericLimits<double>().epsilon());
-}
+    EXPECT_EQ(v2, v);
-
-TEST_CASE(assign_int_to_text_value)
-{
-    SQL::Value text(SQL::SQLType::Text);
-    text = 42;
-    EXPECT_EQ(text.to_string(), "42");
 }
 
 TEST_CASE(copy_value)
 {
-    SQL::Value text(SQL::SQLType::Text, 42);
+    SQL::Value text("42");
     SQL::Value copy(text);
-    EXPECT_EQ(copy.to_string(), "42");
+    EXPECT_EQ(copy, "42"sv);
 }
 
 TEST_CASE(compare_text_to_int)
 {
-    SQL::Value text(SQL::SQLType::Text);
+    SQL::Value text("42");
-    text = 42;
+    SQL::Value integer(42);
-    SQL::Value integer(SQL::SQLType::Integer);
+    EXPECT_EQ(text, integer);
-    integer = 42;
+    EXPECT_EQ(integer, text);
-    EXPECT(text == integer);
-    EXPECT(integer == text);
 }
 
 TEST_CASE(bool_value)
 {
     {
         SQL::Value v(SQL::SQLType::Boolean);
+        EXPECT_EQ(v.type(), SQL::SQLType::Boolean);
         EXPECT(v.is_null());
+
         v = true;
-        EXPECT(!v.is_null());
+        EXPECT_EQ(v.type(), SQL::SQLType::Boolean);
+
         EXPECT(v.to_bool().has_value());
         EXPECT(v.to_bool().value());
+
+        EXPECT(v.to_int().has_value());
         EXPECT_EQ(v.to_int().value(), 1);
-        EXPECT_EQ(v.to_string(), "true");
+        EXPECT_EQ(v.to_string(), "true"sv);
-        EXPECT(!v.to_double().has_value());
+
+        EXPECT(v.to_double().has_value());
+        EXPECT((v.to_double().value() - 1.0) < NumericLimits<double>().epsilon());
     }
     {
-        SQL::Value v(SQL::SQLType::Boolean, false);
+        SQL::Value v(false);
-        EXPECT(!v.is_null());
+        EXPECT_EQ(v.type(), SQL::SQLType::Boolean);
+
         EXPECT(v.to_bool().has_value());
         EXPECT(!v.to_bool().value());
+
+        EXPECT(v.to_int().has_value());
         EXPECT_EQ(v.to_int().value(), 0);
-        EXPECT_EQ(v.to_string(), "false");
+        EXPECT_EQ(v.to_string(), "false"sv);
-        EXPECT(!v.to_double().has_value());
+
+        EXPECT(v.to_double().has_value());
+        EXPECT(v.to_double().value() < NumericLimits<double>().epsilon());
     }
     {
         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());
+
+        EXPECT(v.to_int().has_value());
+        EXPECT_EQ(v.to_int().value(), 1);
+        EXPECT_EQ(v.to_string(), "true"sv);
+
+        EXPECT(v.to_double().has_value());
+        EXPECT((v.to_double().value() - 1.0) < NumericLimits<double>().epsilon());
     }
 }
 
@@ -282,7 +353,6 @@ TEST_CASE(serialize_boolean_value)
 
     serializer.rewind();
     auto v2 = serializer.deserialize<SQL::Value>();
-    EXPECT(!v2.is_null());
     EXPECT_EQ(v2.type(), SQL::SQLType::Boolean);
     EXPECT_EQ(v2.to_bool(), true);
     EXPECT_EQ(v, v2);
@@ -293,12 +363,12 @@ TEST_CASE(tuple_value)
     NonnullRefPtr<SQL::TupleDescriptor> descriptor = adopt_ref(*new SQL::TupleDescriptor);
     descriptor->append({ "schema", "table", "col1", SQL::SQLType::Text, SQL::Order::Ascending });
     descriptor->append({ "schema", "table", "col2", SQL::SQLType::Integer, SQL::Order::Descending });
+    auto v = MUST(SQL::Value::create_tuple(move(descriptor)));
 
-    auto v = SQL::Value::create_tuple(descriptor);
     Vector<SQL::Value> values;
-    values.append(SQL::Value("Test"));
+    values.empend("Test");
-    values.append(SQL::Value(42));
+    values.empend(42);
-    v = values;
+    MUST(v.assign_tuple(values));
 
     auto values2 = v.to_vector();
     EXPECT(values2.has_value());
@@ -310,16 +380,16 @@ TEST_CASE(copy_tuple_value)
     NonnullRefPtr<SQL::TupleDescriptor> descriptor = adopt_ref(*new SQL::TupleDescriptor);
     descriptor->append({ "schema", "table", "col1", SQL::SQLType::Text, SQL::Order::Ascending });
     descriptor->append({ "schema", "table", "col2", SQL::SQLType::Integer, SQL::Order::Descending });
+    auto v = MUST(SQL::Value::create_tuple(move(descriptor)));
 
-    auto v = SQL::Value::create_tuple(descriptor);
     Vector<SQL::Value> values;
-    values.append(SQL::Value("Test"));
+    values.empend("Test");
-    values.append(SQL::Value(42));
+    values.empend(42);
-    v = values;
+    MUST(v.assign_tuple(values));
 
     auto values2 = v;
-    EXPECT(values2.type() == v.type());
+    EXPECT_EQ(values2.type(), v.type());
-    EXPECT(!values2.is_null());
+    EXPECT_EQ(v.type(), SQL::SQLType::Tuple);
     EXPECT_EQ(values, values2.to_vector().value());
 }
 
@@ -327,25 +397,27 @@ TEST_CASE(tuple_value_wrong_type)
 {
     NonnullRefPtr<SQL::TupleDescriptor> descriptor = adopt_ref(*new SQL::TupleDescriptor);
     descriptor->append({ "schema", "table", "col1", SQL::SQLType::Text, SQL::Order::Ascending });
+    auto v = MUST(SQL::Value::create_tuple(move(descriptor)));
 
-    auto v = SQL::Value::create_tuple(descriptor);
     Vector<SQL::Value> values;
-    values.append(SQL::Value(42));
+    values.empend(42);
-    v = values;
+
-    EXPECT(v.is_null());
+    auto result = v.assign_tuple(move(values));
+    EXPECT(result.is_error());
 }
 
 TEST_CASE(tuple_value_too_many_values)
 {
     NonnullRefPtr<SQL::TupleDescriptor> descriptor = adopt_ref(*new SQL::TupleDescriptor);
     descriptor->append({ "schema", "table", "col1", SQL::SQLType::Text, SQL::Order::Ascending });
+    auto v = MUST(SQL::Value::create_tuple(move(descriptor)));
 
-    auto v = SQL::Value::create_tuple(descriptor);
     Vector<SQL::Value> values;
-    values.append(SQL::Value("Test"));
+    values.empend("Test");
-    values.append(SQL::Value(42));
+    values.empend(42);
-    v = values;
+
-    EXPECT(v.is_null());
+    auto result = v.assign_tuple(move(values));
+    EXPECT(result.is_error());
 }
 
 TEST_CASE(tuple_value_not_enough_values)
@@ -353,18 +425,20 @@ TEST_CASE(tuple_value_not_enough_values)
     NonnullRefPtr<SQL::TupleDescriptor> descriptor = adopt_ref(*new SQL::TupleDescriptor);
     descriptor->append({ "schema", "table", "col1", SQL::SQLType::Text, SQL::Order::Ascending });
     descriptor->append({ "schema", "table", "col2", SQL::SQLType::Integer, SQL::Order::Ascending });
+    auto v = MUST(SQL::Value::create_tuple(move(descriptor)));
 
-    auto v = SQL::Value::create_tuple(descriptor);
     Vector<SQL::Value> values;
-    values.append(SQL::Value("Test"));
+    values.empend("Test");
-    v = values;
+    MUST(v.assign_tuple(values));
-    EXPECT(!v.is_null());
+
+    EXPECT_EQ(v.type(), SQL::SQLType::Tuple);
+
     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)
@@ -372,89 +446,22 @@ TEST_CASE(serialize_tuple_value)
     NonnullRefPtr<SQL::TupleDescriptor> descriptor = adopt_ref(*new SQL::TupleDescriptor);
     descriptor->append({ "schema", "table", "col1", SQL::SQLType::Text, SQL::Order::Ascending });
     descriptor->append({ "schema", "table", "col2", SQL::SQLType::Integer, SQL::Order::Descending });
+    auto v = MUST(SQL::Value::create_tuple(move(descriptor)));
 
-    auto v = SQL::Value::create_tuple(descriptor);
     Vector<SQL::Value> values;
-    values.append(SQL::Value("Test"));
+    values.empend("Test");
-    values.append(SQL::Value(42));
+    values.empend(42);
-    v = values;
+    MUST(v.assign_tuple(values));
 
     SQL::Serializer serializer;
     serializer.serialize<SQL::Value>(v);
 
     serializer.rewind();
     auto v2 = serializer.deserialize<SQL::Value>();
-    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;
-
-    SQL::Serializer serializer;
-    serializer.serialize<SQL::Value>(v);
-
-    serializer.rewind();
-    auto v2 = serializer.deserialize<SQL::Value>();
-    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);
@@ -488,8 +495,8 @@ TEST_CASE(tuple)
 
     tuple["col1"] = "Test";
     tuple["col2"] = 42;
-    EXPECT(tuple[0] == "Test");
+    EXPECT_EQ(tuple[0], "Test"sv);
-    EXPECT(tuple[1] == 42);
+    EXPECT_EQ(tuple[1], 42);
 }
 
 TEST_CASE(serialize_tuple)
@@ -502,7 +509,7 @@ TEST_CASE(serialize_tuple)
     tuple["col1"] = "Test";
     tuple["col2"] = 42;
 
-    EXPECT_EQ(tuple[0], "Test");
+    EXPECT_EQ(tuple[0], "Test"sv);
     EXPECT_EQ(tuple[1], 42);
 
     SQL::Serializer serializer;
@@ -510,8 +517,8 @@ TEST_CASE(serialize_tuple)
 
     serializer.rewind();
     auto tuple2 = serializer.deserialize<SQL::Tuple>();
-    EXPECT(tuple2[0] == "Test");
+    EXPECT_EQ(tuple2[0], "Test"sv);
-    EXPECT(tuple2[1] == 42);
+    EXPECT_EQ(tuple2[1], 42);
 }
 
 TEST_CASE(copy_tuple)
@@ -526,10 +533,10 @@ TEST_CASE(copy_tuple)
 
     SQL::Tuple copy;
     copy = tuple;
-    EXPECT(tuple == copy);
+    EXPECT_EQ(tuple, copy);
 
     SQL::Tuple copy_2(copy);
-    EXPECT(tuple == copy_2);
+    EXPECT_EQ(tuple, copy_2);
 }
 
 TEST_CASE(compare_tuples)

Userland/Libraries/LibSQL/AST/Expression.cpp

@@ -16,21 +16,17 @@ static constexpr auto s_posix_basic_metacharacters = ".^$*[]+\\"sv;
 
 ResultOr<Value> NumericLiteral::evaluate(ExecutionContext&) const
 {
-    Value ret(SQLType::Float);
+    return Value { value() };
-    ret = value();
-    return ret;
 }
 
 ResultOr<Value> StringLiteral::evaluate(ExecutionContext&) const
 {
-    Value ret(SQLType::Text);
+    return Value { value() };
-    ret = value();
-    return ret;
 }
 
 ResultOr<Value> NullLiteral::evaluate(ExecutionContext&) const
 {
-    return Value::null();
+    return Value {};
 }
 
 ResultOr<Value> NestedExpression::evaluate(ExecutionContext& context) const
@@ -46,7 +42,7 @@ ResultOr<Value> ChainedExpression::evaluate(ExecutionContext& context) const
     for (auto& expression : expressions())
         values.unchecked_append(TRY(expression.evaluate(context)));
 
-    return Value { move(values) };
+    return Value::create_tuple(move(values));
 }
 
 ResultOr<Value> BinaryOperatorExpression::evaluate(ExecutionContext& context) const

Userland/Libraries/LibSQL/AST/Select.cpp

@@ -54,7 +54,7 @@ ResultOr<ResultSet> Select::execute(ExecutionContext& context) const
     Tuple tuple(descriptor);
     Vector<Tuple> rows;
     descriptor->empend("__unity__"sv);
-    tuple.append(Value(SQLType::Boolean, true));
+    tuple.append(Value { true });
     rows.append(tuple);
 
     for (auto& table_descriptor : table_or_subquery_list()) {

Userland/Libraries/LibSQL/Type.h

@@ -18,8 +18,7 @@ namespace SQL {
     S("int", 4, Integer, int, sizeof(int))       \
     S("float", 8, Float, double, sizeof(double)) \
     S("bool", 16, Boolean, bool, sizeof(bool))   \
-    S("tuple", 32, Tuple, int, sizeof(int))      \
+    S("tuple", 32, Tuple, int, sizeof(int))
-    S("array", 64, Array, int, sizeof(int))
 
 enum class SQLType {
 #undef __ENUMERATE_SQL_TYPE

Userland/Libraries/LibSQL/Value.h

@@ -1,22 +1,21 @@
 /*
  * Copyright (c) 2021, Jan de Visser <jan@de-visser.net>
+ * Copyright (c) 2022, Tim Flynn <trflynn89@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
 #pragma once
 
-#include <AK/Badge.h>
+#include <AK/Format.h>
-#include <AK/ByteBuffer.h>
+#include <AK/Optional.h>
-#include <AK/ScopeGuard.h>
 #include <AK/String.h>
+#include <AK/StringView.h>
 #include <AK/Variant.h>
+#include <AK/Vector.h>
 #include <LibSQL/Forward.h>
 #include <LibSQL/Result.h>
-#include <LibSQL/TupleDescriptor.h>
 #include <LibSQL/Type.h>
-#include <LibSQL/ValueImpl.h>
-#include <string.h>
 
 namespace SQL {
 
@@ -27,49 +26,28 @@ namespace SQL {
  */
 class Value {
 public:
-    Value(Value&) = default;
-    Value(Value const&) = default;
-
     explicit Value(SQLType sql_type = SQLType::Null);
-
+    explicit Value(String);
-    template<typename... Ts>
-    explicit Value(Variant<Ts...> impl)
-        : m_impl(impl)
-    {
-    }
-
-    enum SetImplementation {
-        SetImplementationSingleton
-    };
-
-    template<typename I>
-    Value(SetImplementation, I&& impl)
-    {
-        m_impl.set<I>(forward<I>(impl));
-    }
-
-    Value(SQLType, Value const&);
-    Value(SQLType, String const&);
-    Value(SQLType, char const*);
-    Value(SQLType, int);
-    Value(SQLType, double);
-    Value(SQLType, bool);
-    explicit Value(String const&);
-    explicit Value(char const*);
     explicit Value(int);
     explicit Value(u32);
     explicit Value(double);
-    explicit Value(bool);
+    Value(Value const&);
+    Value(Value&&);
+    ~Value();
 
-    ~Value() = default;
+    static ResultOr<Value> create_tuple(NonnullRefPtr<TupleDescriptor>);
+    static ResultOr<Value> create_tuple(Vector<Value>);
+
+    template<typename T>
+    requires(SameAs<RemoveCVReference<T>, bool>) explicit Value(T value)
+        : m_type(SQLType::Boolean)
+        , m_value(value)
+    {
+    }
 
-    [[nodiscard]] bool is_null() const;
     [[nodiscard]] SQLType type() const;
-    [[nodiscard]] String type_name() const;
+    [[nodiscard]] StringView type_name() const;
-    [[nodiscard]] BaseTypeImpl downcast_to_basetype() const;
+    [[nodiscard]] bool is_null() const;
-
-    template<typename Impl>
-    Impl const& get_impl(Badge<Impl>) const { return m_impl.get<Impl>(); }
 
     [[nodiscard]] String to_string() const;
     [[nodiscard]] Optional<int> to_int() const;
@@ -78,34 +56,33 @@ public:
     [[nodiscard]] Optional<bool> to_bool() const;
     [[nodiscard]] Optional<Vector<Value>> to_vector() const;
 
-    void assign(Value const& other_value);
+    Value& operator=(Value);
-    void assign(String const& string_value);
+    Value& operator=(String);
-    void assign(int int_value);
-    void assign(u32 unsigned_int_value);
-    void assign(double double_value);
-    void assign(bool bool_value);
-    void assign(Vector<Value> const& values);
-
-    Value& operator=(Value const& other);
-
-    Value& operator=(String const&);
-    Value& operator=(char const*);
     Value& operator=(int);
     Value& operator=(u32);
     Value& operator=(double);
-    Value& operator=(bool);
+
-    Value& operator=(Vector<Value> const&);
+    ResultOr<void> assign_tuple(NonnullRefPtr<TupleDescriptor>);
+    ResultOr<void> assign_tuple(Vector<Value>);
+
+    template<typename T>
+    requires(SameAs<RemoveCVReference<T>, bool>) Value& operator=(T value)
+    {
+        m_type = SQLType::Boolean;
+        m_value = value;
+        return *this;
+    }
 
     [[nodiscard]] size_t length() const;
     [[nodiscard]] u32 hash() const;
-    [[nodiscard]] bool can_cast(Value const&) const;
     void serialize(Serializer&) const;
     void deserialize(Serializer&);
 
     [[nodiscard]] int compare(Value const&) const;
     bool operator==(Value const&) const;
-    bool operator==(String const&) const;
+    bool operator==(StringView) const;
     bool operator==(int) const;
+    bool operator==(u32) const;
     bool operator==(double) const;
     bool operator!=(Value const&) const;
     bool operator<(Value const&) const;
@@ -123,20 +100,31 @@ public:
     ResultOr<Value> bitwise_or(Value const&) const;
     ResultOr<Value> bitwise_and(Value const&) const;
 
-    [[nodiscard]] TupleElementDescriptor descriptor() const
+    [[nodiscard]] TupleElementDescriptor descriptor() const;
-    {
-        return { "", "", "", type(), Order::Ascending };
-    }
-
-    static Value const& null();
-    static Value create_tuple(NonnullRefPtr<TupleDescriptor> const&);
-    static Value create_array(SQLType element_type, Optional<size_t> const& max_size = {});
 
 private:
-    void setup(SQLType type);
-
-    ValueTypeImpl m_impl { NullImpl() };
     friend Serializer;
+
+    struct TupleValue {
+        NonnullRefPtr<TupleDescriptor> descriptor;
+        Vector<Value> values;
+    };
+
+    using ValueType = Variant<String, int, double, bool, TupleValue>;
+
+    static ResultOr<NonnullRefPtr<TupleDescriptor>> infer_tuple_descriptor(Vector<Value> const& values);
+    Value(NonnullRefPtr<TupleDescriptor> descriptor, Vector<Value> values);
+
+    SQLType m_type { SQLType::Null };
+    Optional<ValueType> m_value;
 };
 
 }
+
+template<>
+struct AK::Formatter<SQL::Value> : Formatter<StringView> {
+    ErrorOr<void> format(FormatBuilder& builder, SQL::Value const& value)
+    {
+        return Formatter<StringView>::format(builder, value.to_string());
+    }
+};

Userland/Libraries/LibSQL/ValueImpl.h

@@ -1,298 +0,0 @@
-/*
- * Copyright (c) 2021, Jan de Visser <jan@de-visser.net>
- *
- * SPDX-License-Identifier: BSD-2-Clause
- */
-
-#pragma once
-
-#include <AK/Badge.h>
-#include <AK/ByteBuffer.h>
-#include <AK/ScopeGuard.h>
-#include <AK/String.h>
-#include <AK/Variant.h>
-#include <LibSQL/Forward.h>
-#include <LibSQL/Serializer.h>
-#include <LibSQL/TupleDescriptor.h>
-#include <LibSQL/Type.h>
-#include <string.h>
-
-namespace SQL {
-
-class Value;
-
-class BaseImpl {
-public:
-    explicit BaseImpl(SQLType type = SQLType::Null)
-        : m_type(type)
-    {
-    }
-
-    [[nodiscard]] SQLType type() const { return m_type; }
-    [[nodiscard]] String type_name() const { return SQLType_name(type()); }
-
-private:
-    SQLType m_type { SQLType::Null };
-};
-
-class NullImpl : public BaseImpl {
-public:
-    explicit NullImpl()
-        : BaseImpl(SQLType::Null)
-    {
-    }
-
-    [[nodiscard]] static bool is_null() { return true; }
-    [[nodiscard]] static String to_string() { return "(null)"; }
-    [[nodiscard]] static Optional<int> to_int() { return {}; }
-    [[nodiscard]] static Optional<double> to_double() { return {}; }
-    [[nodiscard]] static Optional<bool> to_bool() { return {}; }
-    [[nodiscard]] static bool to_vector(Vector<Value>&) { return false; }
-    static void assign(Value const&) { }
-    static void assign_string(String const&) { }
-    static void assign_int(int) { }
-    static void assign_double(double) { }
-    static void assign_bool(bool) { }
-    static void assign_vector(Vector<Value> const&) { }
-    [[nodiscard]] static size_t length() { return 0; }
-    [[nodiscard]] static bool can_cast(Value const&);
-    [[nodiscard]] static int compare(Value const&);
-    static void serialize(Serializer&) { }
-    static void deserialize(Serializer&) { }
-    [[nodiscard]] static u32 hash() { return 0; }
-};
-
-template<typename T>
-class Impl : public BaseImpl {
-public:
-    [[nodiscard]] bool is_null() const
-    {
-        return !m_value.has_value();
-    }
-
-    [[nodiscard]] T const& value() const
-    {
-        VERIFY(m_value.has_value());
-        return m_value.value();
-    }
-
-    [[nodiscard]] size_t length() const
-    {
-        return sizeof(T);
-    }
-
-    void serialize(Serializer& serializer) const
-    {
-        serializer.serialize(value());
-    }
-
-    void deserialize(Serializer& serializer)
-    {
-        T value;
-        serializer.deserialize_to(value);
-        m_value = value;
-    }
-
-protected:
-    explicit Impl(SQLType sql_type)
-        : BaseImpl(sql_type)
-    {
-    }
-
-    Optional<T> m_value {};
-};
-
-class TextImpl : public Impl<String> {
-public:
-    explicit TextImpl()
-        : Impl(SQLType::Text)
-    {
-    }
-
-    [[nodiscard]] String to_string() const;
-    [[nodiscard]] Optional<int> to_int() const;
-    [[nodiscard]] Optional<double> to_double() const;
-    [[nodiscard]] Optional<bool> to_bool() const;
-    [[nodiscard]] static bool to_vector(Vector<Value>&) { return false; }
-    void assign(Value const&);
-    void assign_string(String const&);
-    void assign_int(int);
-    void assign_double(double);
-    void assign_bool(bool);
-    void assign_vector(Vector<Value> const&) { m_value = {}; }
-    [[nodiscard]] size_t length() const;
-    [[nodiscard]] static bool can_cast(Value const&) { return true; }
-    [[nodiscard]] int compare(Value const& other) const;
-    [[nodiscard]] u32 hash() const;
-};
-
-class IntegerImpl : public Impl<int> {
-public:
-    IntegerImpl()
-        : Impl(SQLType::Integer)
-    {
-    }
-
-    [[nodiscard]] String to_string() const;
-    [[nodiscard]] Optional<int> to_int() const;
-    [[nodiscard]] Optional<double> to_double() const;
-    [[nodiscard]] Optional<bool> to_bool() const;
-    [[nodiscard]] static bool to_vector(Vector<Value>&) { return false; }
-    void assign(Value const&);
-    void assign_string(String const&);
-    void assign_int(int);
-    void assign_double(double);
-    void assign_bool(bool);
-    void assign_vector(Vector<Value> const&) { m_value = {}; }
-    [[nodiscard]] static bool can_cast(Value const&);
-    [[nodiscard]] int compare(Value const& other) const;
-    [[nodiscard]] u32 hash() const;
-};
-
-class FloatImpl : public Impl<double> {
-public:
-    explicit FloatImpl()
-        : Impl(SQLType::Float)
-    {
-    }
-
-    [[nodiscard]] String to_string() const;
-    [[nodiscard]] Optional<int> to_int() const;
-    [[nodiscard]] Optional<double> to_double() const;
-    [[nodiscard]] Optional<bool> to_bool() const;
-    [[nodiscard]] static bool to_vector(Vector<Value>&) { return false; }
-    void assign(Value const&);
-    void assign_string(String const&);
-    void assign_int(int);
-    void assign_double(double);
-    void assign_bool(bool) { m_value = {}; }
-    void assign_vector(Vector<Value> const&) { m_value = {}; }
-    [[nodiscard]] static bool can_cast(Value const&);
-    [[nodiscard]] int compare(Value const& other) const;
-
-    // Using floats in hash functions is a bad idea. Let's disable that for now.
-    [[nodiscard]] static u32 hash() { VERIFY_NOT_REACHED(); }
-};
-
-class BooleanImpl : public Impl<bool> {
-public:
-    explicit BooleanImpl()
-        : Impl(SQLType::Boolean)
-    {
-    }
-
-    [[nodiscard]] String to_string() const;
-    [[nodiscard]] Optional<int> to_int() const;
-    [[nodiscard]] static Optional<double> to_double();
-    [[nodiscard]] Optional<bool> to_bool() const;
-    [[nodiscard]] static bool to_vector(Vector<Value>&) { return false; }
-    void assign(Value const&);
-    void assign_string(String const&);
-    void assign_int(int);
-    void assign_double(double);
-    void assign_bool(bool);
-    void assign_vector(Vector<Value> const&) { m_value = {}; }
-    [[nodiscard]] static bool can_cast(Value const&);
-    [[nodiscard]] int compare(Value const& other) const;
-    [[nodiscard]] u32 hash() const;
-};
-
-using BaseTypeImpl = Variant<NullImpl, TextImpl, IntegerImpl, FloatImpl, BooleanImpl>;
-
-class ContainerValueImpl : public Impl<Vector<BaseTypeImpl>> {
-public:
-    virtual ~ContainerValueImpl() = default;
-
-    [[nodiscard]] String to_string() const;
-    [[nodiscard]] static Optional<int> to_int() { return {}; }
-    [[nodiscard]] static Optional<double> to_double() { return {}; }
-    [[nodiscard]] static Optional<bool> to_bool() { return {}; }
-    [[nodiscard]] bool to_vector(Vector<Value>&) const;
-    void assign_string(String const&) { m_value = {}; }
-    void assign_int(int) { m_value = {}; }
-    void assign_double(double) { m_value = {}; }
-    void assign_bool(bool) { m_value = {}; }
-    void assign_vector(Vector<Value> const&);
-    [[nodiscard]] u32 hash() const;
-
-    virtual bool validate_before_assignment(Vector<Value> const&) { return true; }
-    virtual bool validate(BaseTypeImpl const&) { return true; }
-    virtual bool validate_after_assignment() { return true; }
-    [[nodiscard]] Vector<String> to_string_vector() const;
-    [[nodiscard]] size_t length() const;
-    [[nodiscard]] size_t size() const { return is_null() ? 0 : value().size(); }
-    bool append(Value const&);
-    bool append(BaseTypeImpl const& value);
-    void serialize_values(Serializer&) const;
-    void deserialize_values(Serializer&);
-
-protected:
-    explicit ContainerValueImpl(SQLType sql_type)
-        : Impl(sql_type)
-    {
-    }
-};
-
-class TupleImpl : public ContainerValueImpl {
-public:
-    explicit TupleImpl(NonnullRefPtr<TupleDescriptor> const& descriptor)
-        : ContainerValueImpl(SQLType::Tuple)
-        , m_descriptor(descriptor)
-    {
-    }
-
-    explicit TupleImpl()
-        : ContainerValueImpl(SQLType::Tuple)
-    {
-    }
-
-    void assign(Value const&);
-    [[nodiscard]] size_t length() const;
-    [[nodiscard]] bool can_cast(Value const&) const;
-    [[nodiscard]] int compare(Value const& other) const;
-    [[nodiscard]] Optional<bool> to_bool() const;
-
-    virtual bool validate_before_assignment(Vector<Value> const&) override;
-    virtual bool validate(BaseTypeImpl const&) override;
-    virtual bool validate_after_assignment() override;
-    void serialize(Serializer&) const;
-    void deserialize(Serializer&);
-
-private:
-    void infer_descriptor();
-    void extend_descriptor(Value const&);
-    RefPtr<TupleDescriptor> m_descriptor;
-    bool m_descriptor_inferred { false };
-};
-
-class ArrayImpl : public ContainerValueImpl {
-public:
-    explicit ArrayImpl(SQLType element_type, Optional<size_t> const& max_size = {})
-        : ContainerValueImpl(SQLType::Array)
-        , m_element_type(element_type)
-        , m_max_size(max_size)
-    {
-    }
-
-    explicit ArrayImpl()
-        : ContainerValueImpl(SQLType::Array)
-        , m_element_type(SQLType::Null)
-    {
-    }
-
-    void assign(Value const&);
-    [[nodiscard]] size_t length() const;
-    [[nodiscard]] bool can_cast(Value const&) const;
-    [[nodiscard]] int compare(Value const& other) const;
-    void serialize(Serializer&) const;
-    void deserialize(Serializer&);
-    virtual bool validate(BaseTypeImpl const&) override;
-
-private:
-    SQLType m_element_type { SQLType::Text };
-    Optional<size_t> m_max_size {};
-};
-
-using ValueTypeImpl = Variant<NullImpl, TextImpl, IntegerImpl, FloatImpl, BooleanImpl, TupleImpl, ArrayImpl>;
-
-}