LibSQL: Improve error handling

The handling of filesystem level errors was basically non-existing or
consisting of `VERIFY_NOT_REACHED` assertions. Addressed this by
* Adding `open` methods to `Heap` and `Database` which return errors.
* Changing the interface of methods of these classes and clients
downstream to propagate these errors.

The constructors of `Heap` and `Database` don't open the underlying
filesystem file anymore.

The SQL statement handlers return an `SQLErrorCode::InternalError`
error code if an error comes back from the lower levels. Note that some
of these errors are things like duplicate index entry errors that should
be caught before the SQL layer attempts to actually update the database.

Added tests to catch attempts to open weird or non-existent files as
databases.

Finally, in between me writing this patch and submitting the PR the
AK::Result<Foo, Bar> template got deprecated in favour of ErrorOr<Foo>.
This resulted in more busywork.

Tests/LibSQL/TestSqlBtreeIndex.cpp

@@ -146,6 +146,7 @@ void insert_and_get_to_and_from_btree(int num_keys)
     ScopeGuard guard([]() { unlink("/tmp/test.db"); });
     {
         auto heap = SQL::Heap::construct("/tmp/test.db");
+        EXPECT(!heap->open().is_error());
         SQL::Serializer serializer(heap);
         auto btree = setup_btree(serializer);
 
@@ -162,6 +163,7 @@ void insert_and_get_to_and_from_btree(int num_keys)
 
     {
         auto heap = SQL::Heap::construct("/tmp/test.db");
+        EXPECT(!heap->open().is_error());
         SQL::Serializer serializer(heap);
         auto btree = setup_btree(serializer);
 
@@ -180,6 +182,7 @@ void insert_into_and_scan_btree(int num_keys)
     ScopeGuard guard([]() { unlink("/tmp/test.db"); });
     {
         auto heap = SQL::Heap::construct("/tmp/test.db");
+        EXPECT(!heap->open().is_error());
         SQL::Serializer serializer(heap);
         auto btree = setup_btree(serializer);
 
@@ -197,6 +200,7 @@ void insert_into_and_scan_btree(int num_keys)
 
     {
         auto heap = SQL::Heap::construct("/tmp/test.db");
+        EXPECT(!heap->open().is_error());
         SQL::Serializer serializer(heap);
         auto btree = setup_btree(serializer);
 

Tests/LibSQL/TestSqlDatabase.cpp

@@ -20,11 +20,13 @@ NonnullRefPtr<SQL::TableDef> setup_table(SQL::Database&);
 void insert_into_table(SQL::Database&, int);
 void verify_table_contents(SQL::Database&, int);
 void insert_and_verify(int);
+void commit(SQL::Database&);
 
 NonnullRefPtr<SQL::SchemaDef> setup_schema(SQL::Database& db)
 {
     auto schema = SQL::SchemaDef::construct("TestSchema");
-    db.add_schema(schema);
+    auto maybe_error = db.add_schema(schema);
+    EXPECT(!maybe_error.is_error());
     return schema;
 }
 
@@ -32,17 +34,19 @@ NonnullRefPtr<SQL::TableDef> setup_table(SQL::Database& db)
 {
     auto schema = setup_schema(db);
     auto table = SQL::TableDef::construct(schema, "TestTable");
-    db.add_table(table);
     table->append_column("TextColumn", SQL::SQLType::Text);
     table->append_column("IntColumn", SQL::SQLType::Integer);
     EXPECT_EQ(table->num_columns(), 2u);
-    db.add_table(table);
+    auto maybe_error = db.add_table(table);
+    EXPECT(!maybe_error.is_error());
     return table;
 }
 
 void insert_into_table(SQL::Database& db, int count)
 {
-    auto table = db.get_table("TestSchema", "TestTable");
+    auto table_or_error = db.get_table("TestSchema", "TestTable");
+    EXPECT(!table_or_error.is_error());
+    auto table = table_or_error.value();
     EXPECT(table);
 
     for (int ix = 0; ix < count; ix++) {
@@ -52,18 +56,23 @@ void insert_into_table(SQL::Database& db, int count)
 
         row["TextColumn"] = builder.build();
         row["IntColumn"] = ix;
-        EXPECT(db.insert(row));
+        auto maybe_error = db.insert(row);
+        EXPECT(!maybe_error.is_error());
     }
 }
 
 void verify_table_contents(SQL::Database& db, int expected_count)
 {
-    auto table = db.get_table("TestSchema", "TestTable");
+    auto table_or_error = db.get_table("TestSchema", "TestTable");
+    EXPECT(!table_or_error.is_error());
+    auto table = table_or_error.value();
     EXPECT(table);
 
     int sum = 0;
     int count = 0;
-    for (auto& row : db.select_all(*table)) {
+    auto rows_or_error = db.select_all(*table);
+    EXPECT(!rows_or_error.is_error());
+    for (auto& row : rows_or_error.value()) {
         StringBuilder builder;
         builder.appendff("Test{}", row["IntColumn"].to_int().value());
         EXPECT_EQ(row["TextColumn"].to_string(), builder.build());
@@ -74,21 +83,30 @@ void verify_table_contents(SQL::Database& db, int expected_count)
     EXPECT_EQ(sum, (expected_count * (expected_count - 1)) / 2);
 }
 
+void commit(SQL::Database& db)
+{
+    auto maybe_error = db.commit();
+    EXPECT(!maybe_error.is_error());
+}
+
 void insert_and_verify(int count)
 {
     ScopeGuard guard([]() { unlink("/tmp/test.db"); });
     {
         auto db = SQL::Database::construct("/tmp/test.db");
+        EXPECT(!db->open().is_error());
         setup_table(db);
-        db->commit();
+        commit(db);
     }
     {
         auto db = SQL::Database::construct("/tmp/test.db");
+        EXPECT(!db->open().is_error());
         insert_into_table(db, count);
-        db->commit();
+        commit(db);
     }
     {
         auto db = SQL::Database::construct("/tmp/test.db");
+        EXPECT(!db->open().is_error());
         verify_table_contents(db, count);
     }
 }
@@ -97,22 +115,47 @@ TEST_CASE(create_heap)
 {
     ScopeGuard guard([]() { unlink("/tmp/test.db"); });
     auto heap = SQL::Heap::construct("/tmp/test.db");
+    EXPECT(!heap->open().is_error());
     EXPECT_EQ(heap->version(), 0x00000001u);
 }
 
+TEST_CASE(create_from_dev_random)
+{
+    auto heap = SQL::Heap::construct("/dev/random");
+    auto should_be_error = heap->open();
+    EXPECT(should_be_error.is_error());
+}
+
+TEST_CASE(create_from_unreadable_file)
+{
+    auto heap = SQL::Heap::construct("/etc/shadow");
+    auto should_be_error = heap->open();
+    EXPECT(should_be_error.is_error());
+}
+
+TEST_CASE(create_in_non_existing_dir)
+{
+    auto heap = SQL::Heap::construct("/tmp/bogus/test.db");
+    auto should_be_error = heap->open();
+    EXPECT(should_be_error.is_error());
+}
+
 TEST_CASE(create_database)
 {
     ScopeGuard guard([]() { unlink("/tmp/test.db"); });
     auto db = SQL::Database::construct("/tmp/test.db");
-    db->commit();
+    auto should_not_be_error = db->open();
+    EXPECT(!should_not_be_error.is_error());
+    commit(db);
 }
 
 TEST_CASE(add_schema_to_database)
 {
     ScopeGuard guard([]() { unlink("/tmp/test.db"); });
     auto db = SQL::Database::construct("/tmp/test.db");
+    EXPECT(!db->open().is_error());
     setup_schema(db);
-    db->commit();
+    commit(db);
 }
 
 TEST_CASE(get_schema_from_database)
@@ -120,13 +163,16 @@ TEST_CASE(get_schema_from_database)
     ScopeGuard guard([]() { unlink("/tmp/test.db"); });
     {
         auto db = SQL::Database::construct("/tmp/test.db");
+        EXPECT(!db->open().is_error());
         setup_schema(db);
-        db->commit();
+        commit(db);
     }
     {
         auto db = SQL::Database::construct("/tmp/test.db");
-        auto schema = db->get_schema("TestSchema");
-        EXPECT(schema);
+        EXPECT(!db->open().is_error());
+        auto schema_or_error = db->get_schema("TestSchema");
+        EXPECT(!schema_or_error.is_error());
+        EXPECT(schema_or_error.value());
     }
 }
 
@@ -134,8 +180,9 @@ TEST_CASE(add_table_to_database)
 {
     ScopeGuard guard([]() { unlink("/tmp/test.db"); });
     auto db = SQL::Database::construct("/tmp/test.db");
+    EXPECT(!db->open().is_error());
     setup_table(db);
-    db->commit();
+    commit(db);
 }
 
 TEST_CASE(get_table_from_database)
@@ -143,12 +190,16 @@ TEST_CASE(get_table_from_database)
     ScopeGuard guard([]() { unlink("/tmp/test.db"); });
     {
         auto db = SQL::Database::construct("/tmp/test.db");
+        EXPECT(!db->open().is_error());
         setup_table(db);
-        db->commit();
+        commit(db);
     }
     {
         auto db = SQL::Database::construct("/tmp/test.db");
-        auto table = db->get_table("TestSchema", "TestTable");
+        EXPECT(!db->open().is_error());
+        auto table_or_error = db->get_table("TestSchema", "TestTable");
+        EXPECT(!table_or_error.is_error());
+        auto table = table_or_error.value();
         EXPECT(table);
         EXPECT_EQ(table->name(), "TestTable");
         EXPECT_EQ(table->num_columns(), 2u);

Tests/LibSQL/TestSqlHashIndex.cpp

@@ -141,6 +141,7 @@ void insert_and_get_to_and_from_hash_index(int num_keys)
     ScopeGuard guard([]() { unlink("/tmp/test.db"); });
     {
         auto heap = SQL::Heap::construct("/tmp/test.db");
+        EXPECT(!heap->open().is_error());
         SQL::Serializer serializer(heap);
         auto hash_index = setup_hash_index(serializer);
 
@@ -158,6 +159,7 @@ void insert_and_get_to_and_from_hash_index(int num_keys)
 
     {
         auto heap = SQL::Heap::construct("/tmp/test.db");
+        EXPECT(!heap->open().is_error());
         SQL::Serializer serializer(heap);
         auto hash_index = setup_hash_index(serializer);
 
@@ -237,6 +239,7 @@ void insert_into_and_scan_hash_index(int num_keys)
     ScopeGuard guard([]() { unlink("/tmp/test.db"); });
     {
         auto heap = SQL::Heap::construct("/tmp/test.db");
+        EXPECT(!heap->open().is_error());
         SQL::Serializer serializer(heap);
         auto hash_index = setup_hash_index(serializer);
 
@@ -254,6 +257,7 @@ void insert_into_and_scan_hash_index(int num_keys)
 
     {
         auto heap = SQL::Heap::construct("/tmp/test.db");
+        EXPECT(!heap->open().is_error());
         SQL::Serializer serializer(heap);
         auto hash_index = setup_hash_index(serializer);
         Vector<bool> found;

Tests/LibSQL/TestSqlStatementExecution.cpp

@@ -61,33 +61,42 @@ TEST_CASE(create_schema)
 {
     ScopeGuard guard([]() { unlink(db_name); });
     auto database = SQL::Database::construct(db_name);
+    EXPECT(!database->open().is_error());
     create_schema(database);
-    auto schema = database->get_schema("TESTSCHEMA");
-    EXPECT(schema);
+    auto schema_or_error = database->get_schema("TESTSCHEMA");
+    EXPECT(!schema_or_error.is_error());
+    EXPECT(schema_or_error.value());
 }
 
 TEST_CASE(create_table)
 {
     ScopeGuard guard([]() { unlink(db_name); });
     auto database = SQL::Database::construct(db_name);
+    EXPECT(!database->open().is_error());
     create_table(database);
-    auto table = database->get_table("TESTSCHEMA", "TESTTABLE");
-    EXPECT(table);
+    auto table_or_error = database->get_table("TESTSCHEMA", "TESTTABLE");
+    EXPECT(!table_or_error.is_error());
+    EXPECT(table_or_error.value());
 }
 
 TEST_CASE(insert_into_table)
 {
     ScopeGuard guard([]() { unlink(db_name); });
     auto database = SQL::Database::construct(db_name);
+    EXPECT(!database->open().is_error());
     create_table(database);
     auto result = execute(database, "INSERT INTO TestSchema.TestTable ( TextColumn, IntColumn ) VALUES ( 'Test', 42 );");
     EXPECT(result->error().code == SQL::SQLErrorCode::NoError);
     EXPECT(result->inserted() == 1);
 
-    auto table = database->get_table("TESTSCHEMA", "TESTTABLE");
+    auto table_or_error = database->get_table("TESTSCHEMA", "TESTTABLE");
+    EXPECT(!table_or_error.is_error());
+    auto table = table_or_error.value();
 
     int count = 0;
-    for (auto& row : database->select_all(*table)) {
+    auto rows_or_error = database->select_all(*table);
+    EXPECT(!rows_or_error.is_error());
+    for (auto& row : rows_or_error.value()) {
         EXPECT_EQ(row["TEXTCOLUMN"].to_string(), "Test");
         EXPECT_EQ(row["INTCOLUMN"].to_int().value(), 42);
         count++;
@@ -99,6 +108,7 @@ TEST_CASE(insert_into_table_wrong_data_types)
 {
     ScopeGuard guard([]() { unlink(db_name); });
     auto database = SQL::Database::construct(db_name);
+    EXPECT(!database->open().is_error());
     create_table(database);
     auto result = execute(database, "INSERT INTO TestSchema.TestTable ( TextColumn, IntColumn ) VALUES (43, 'Test_2');");
     EXPECT(result->inserted() == 0);
@@ -109,6 +119,7 @@ TEST_CASE(insert_into_table_multiple_tuples_wrong_data_types)
 {
     ScopeGuard guard([]() { unlink(db_name); });
     auto database = SQL::Database::construct(db_name);
+    EXPECT(!database->open().is_error());
     create_table(database);
     auto result = execute(database, "INSERT INTO TestSchema.TestTable ( TextColumn, IntColumn ) VALUES ('Test_1', 42), (43, 'Test_2');");
     EXPECT(result->inserted() == 0);
@@ -119,6 +130,7 @@ TEST_CASE(insert_wrong_number_of_values)
 {
     ScopeGuard guard([]() { unlink(db_name); });
     auto database = SQL::Database::construct(db_name);
+    EXPECT(!database->open().is_error());
     create_table(database);
     auto result = execute(database, "INSERT INTO TestSchema.TestTable VALUES ( 42 );");
     EXPECT(result->error().code == SQL::SQLErrorCode::InvalidNumberOfValues);
@@ -129,19 +141,24 @@ TEST_CASE(insert_without_column_names)
 {
     ScopeGuard guard([]() { unlink(db_name); });
     auto database = SQL::Database::construct(db_name);
+    EXPECT(!database->open().is_error());
     create_table(database);
     auto result = execute(database, "INSERT INTO TestSchema.TestTable VALUES ('Test_1', 42), ('Test_2', 43);");
     EXPECT(result->error().code == SQL::SQLErrorCode::NoError);
     EXPECT(result->inserted() == 2);
 
-    auto table = database->get_table("TESTSCHEMA", "TESTTABLE");
-    EXPECT_EQ(database->select_all(*table).size(), 2u);
+    auto table_or_error = database->get_table("TESTSCHEMA", "TESTTABLE");
+    EXPECT(!table_or_error.is_error());
+    auto rows_or_error = database->select_all(*(table_or_error.value()));
+    EXPECT(!rows_or_error.is_error());
+    EXPECT_EQ(rows_or_error.value().size(), 2u);
 }
 
 TEST_CASE(select_from_table)
 {
     ScopeGuard guard([]() { unlink(db_name); });
     auto database = SQL::Database::construct(db_name);
+    EXPECT(!database->open().is_error());
     create_table(database);
     auto result = execute(database,
         "INSERT INTO TestSchema.TestTable ( TextColumn, IntColumn ) VALUES "
@@ -162,6 +179,7 @@ TEST_CASE(select_with_column_names)
 {
     ScopeGuard guard([]() { unlink(db_name); });
     auto database = SQL::Database::construct(db_name);
+    EXPECT(!database->open().is_error());
     create_table(database);
     auto result = execute(database,
         "INSERT INTO TestSchema.TestTable ( TextColumn, IntColumn ) VALUES "
@@ -183,6 +201,7 @@ TEST_CASE(select_with_nonexisting_column_name)
 {
     ScopeGuard guard([]() { unlink(db_name); });
     auto database = SQL::Database::construct(db_name);
+    EXPECT(!database->open().is_error());
     create_table(database);
     auto result = execute(database,
         "INSERT INTO TestSchema.TestTable ( TextColumn, IntColumn ) VALUES "
@@ -201,6 +220,7 @@ TEST_CASE(select_with_where)
 {
     ScopeGuard guard([]() { unlink(db_name); });
     auto database = SQL::Database::construct(db_name);
+    EXPECT(!database->open().is_error());
     create_table(database);
     auto result = execute(database,
         "INSERT INTO TestSchema.TestTable ( TextColumn, IntColumn ) VALUES "
@@ -224,6 +244,7 @@ TEST_CASE(select_cross_join)
 {
     ScopeGuard guard([]() { unlink(db_name); });
     auto database = SQL::Database::construct(db_name);
+    EXPECT(!database->open().is_error());
     create_two_tables(database);
     auto result = execute(database,
         "INSERT INTO TestSchema.TestTable1 ( TextColumn1, IntColumn ) VALUES "
@@ -260,6 +281,7 @@ TEST_CASE(select_inner_join)
 {
     ScopeGuard guard([]() { unlink(db_name); });
     auto database = SQL::Database::construct(db_name);
+    EXPECT(!database->open().is_error());
     create_two_tables(database);
     auto result = execute(database,
         "INSERT INTO TestSchema.TestTable1 ( TextColumn1, IntColumn ) VALUES "

Userland/Libraries/LibSQL/AST/CreateSchema.cpp

@@ -12,7 +12,10 @@ namespace SQL::AST {
 
 RefPtr<SQLResult> CreateSchema::execute(ExecutionContext& context) const
 {
-    auto schema_def = context.database->get_schema(m_schema_name);
+    auto schema_def_or_error = context.database->get_schema(m_schema_name);
+    if (schema_def_or_error.is_error())
+        return SQLResult::construct(SQLCommand::Create, SQLErrorCode::InternalError, schema_def_or_error.error());
+    auto schema_def = schema_def_or_error.release_value();
     if (schema_def) {
         if (m_is_error_if_schema_exists) {
             return SQLResult::construct(SQLCommand::Create, SQLErrorCode::SchemaExists, m_schema_name);
@@ -21,7 +24,8 @@ RefPtr<SQLResult> CreateSchema::execute(ExecutionContext& context) const
     }
 
     schema_def = SchemaDef::construct(m_schema_name);
-    context.database->add_schema(*schema_def);
+    if (auto maybe_error = context.database->add_schema(*schema_def); maybe_error.is_error())
+        return SQLResult::construct(SQLCommand::Create, SQLErrorCode::InternalError, maybe_error.error());
     return SQLResult::construct(SQLCommand::Create, 0, 1);
 }
 

Userland/Libraries/LibSQL/AST/CreateTable.cpp

@@ -12,10 +12,16 @@ namespace SQL::AST {
 RefPtr<SQLResult> CreateTable::execute(ExecutionContext& context) const
 {
     auto schema_name = (!m_schema_name.is_null() && !m_schema_name.is_empty()) ? m_schema_name : "default";
-    auto schema_def = context.database->get_schema(schema_name);
+    auto schema_def_or_error = context.database->get_schema(schema_name);
+    if (schema_def_or_error.is_error())
+        return SQLResult::construct(SQLCommand::Create, SQLErrorCode::InternalError, schema_def_or_error.error());
+    auto schema_def = schema_def_or_error.release_value();
     if (!schema_def)
         return SQLResult::construct(SQLCommand::Create, SQLErrorCode::SchemaDoesNotExist, m_schema_name);
-    auto table_def = context.database->get_table(schema_name, m_table_name);
+    auto table_def_or_error = context.database->get_table(schema_name, m_table_name);
+    if (table_def_or_error.is_error())
+        return SQLResult::construct(SQLCommand::Create, SQLErrorCode::InternalError, table_def_or_error.error());
+    auto table_def = table_def_or_error.release_value();
     if (table_def) {
         if (m_is_error_if_table_exists) {
             return SQLResult::construct(SQLCommand::Create, SQLErrorCode::TableExists, m_table_name);
@@ -37,7 +43,8 @@ RefPtr<SQLResult> CreateTable::execute(ExecutionContext& context) const
         }
         table_def->append_column(column.name(), type);
     }
-    context.database->add_table(*table_def);
+    if (auto maybe_error = context.database->add_table(*table_def); maybe_error.is_error())
+        return SQLResult::construct(SQLCommand::Create, SQLErrorCode::InternalError, maybe_error.release_error());
     return SQLResult::construct(SQLCommand::Create, 0, 1);
 }
 

Userland/Libraries/LibSQL/AST/Expression.cpp

@@ -66,7 +66,8 @@ Value BinaryOperatorExpression::evaluate(ExecutionContext& context) const
     switch (type()) {
     case BinaryOperator::Concatenate: {
         if (lhs_value.type() != SQLType::Text) {
-            VERIFY_NOT_REACHED();
+            context.result->set_error(SQLErrorCode::BooleanOperatorTypeMismatch, BinaryOperator_name(type()));
+            return Value::null();
         }
         AK::StringBuilder builder;
         builder.append(lhs_value.to_string());
@@ -110,7 +111,7 @@ Value BinaryOperatorExpression::evaluate(ExecutionContext& context) const
             context.result->set_error(SQLErrorCode::BooleanOperatorTypeMismatch, BinaryOperator_name(type()));
             return Value::null();
         }
-        return Value(lhs_bool_maybe.value() && rhs_bool_maybe.value());
+        return Value(lhs_bool_maybe.release_value() && rhs_bool_maybe.release_value());
     }
     case BinaryOperator::Or: {
         auto lhs_bool_maybe = lhs_value.to_bool();
@@ -119,7 +120,7 @@ Value BinaryOperatorExpression::evaluate(ExecutionContext& context) const
             context.result->set_error(SQLErrorCode::BooleanOperatorTypeMismatch, BinaryOperator_name(type()));
             return Value::null();
         }
-        return Value(lhs_bool_maybe.value() || rhs_bool_maybe.value());
+        return Value(lhs_bool_maybe.release_value() || rhs_bool_maybe.release_value());
     }
     default:
         VERIFY_NOT_REACHED();

Userland/Libraries/LibSQL/AST/Insert.cpp

@@ -27,7 +27,10 @@ static bool does_value_data_type_match(SQLType expected, SQLType actual)
 
 RefPtr<SQLResult> Insert::execute(ExecutionContext& context) const
 {
-    auto table_def = context.database->get_table(m_schema_name, m_table_name);
+    auto table_def_or_error = context.database->get_table(m_schema_name, m_table_name);
+    if (table_def_or_error.is_error())
+        return SQLResult::construct(SQLCommand::Insert, SQLErrorCode::InternalError, table_def_or_error.release_error());
+    auto table_def = table_def_or_error.release_value();
     if (!table_def) {
         auto schema_name = m_schema_name;
         if (schema_name.is_null() || schema_name.is_empty())
@@ -78,8 +81,8 @@ RefPtr<SQLResult> Insert::execute(ExecutionContext& context) const
     }
 
     for (auto& inserted_row : inserted_rows) {
-        context.database->insert(inserted_row);
-        // FIXME Error handling
+        if (auto maybe_error = context.database->insert(inserted_row); maybe_error.is_error())
+            return SQLResult::construct(SQLCommand::Insert, SQLErrorCode::InternalError, maybe_error.release_error());
     }
 
     return SQLResult::construct(SQLCommand::Insert, 0, m_chained_expressions.size(), 0);

Userland/Libraries/LibSQL/AST/Select.cpp

@@ -16,11 +16,15 @@ RefPtr<SQLResult> Select::execute(ExecutionContext& context) const
     NonnullRefPtrVector<ResultColumn> columns;
     for (auto& table_descriptor : table_or_subquery_list()) {
         if (!table_descriptor.is_table())
-            TODO();
-        auto table = context.database->get_table(table_descriptor.schema_name(), table_descriptor.table_name());
+            return SQLResult::construct(SQLCommand::Select, SQLErrorCode::NotYetImplemented, "Sub-selects are not yet implemented");
+        auto table_def_or_error = context.database->get_table(table_descriptor.schema_name(), table_descriptor.table_name());
+        if (table_def_or_error.is_error())
+            return SQLResult::construct(SQLCommand::Select, SQLErrorCode::InternalError, table_def_or_error.error());
+        auto table = table_def_or_error.value();
         if (!table) {
-            return SQLResult::construct(SQL::SQLCommand::Select, SQL::SQLErrorCode::TableDoesNotExist, table_descriptor.table_name());
+            return SQLResult::construct(SQLCommand::Select, SQLErrorCode::TableDoesNotExist, table_descriptor.table_name());
         }
+
         if (result_column_list().size() == 1 && result_column_list()[0].type() == ResultType::All) {
             for (auto& col : table->columns()) {
                 columns.append(
@@ -36,7 +40,7 @@ RefPtr<SQLResult> Select::execute(ExecutionContext& context) const
         for (auto& col : result_column_list()) {
             if (col.type() == ResultType::All)
                 // FIXME can have '*' for example in conjunction with computed columns
-                return SQLResult::construct(SQL::SQLCommand::Select, SQL::SQLErrorCode::SyntaxError, "*");
+                return SQLResult::construct(SQL::SQLCommand::Select, SQLErrorCode::SyntaxError, "*");
             columns.append(col);
         }
     }
@@ -51,15 +55,21 @@ RefPtr<SQLResult> Select::execute(ExecutionContext& context) const
 
     for (auto& table_descriptor : table_or_subquery_list()) {
         if (!table_descriptor.is_table())
-            TODO();
-        auto table = context.database->get_table(table_descriptor.schema_name(), table_descriptor.table_name());
+            return SQLResult::construct(SQLCommand::Select, SQLErrorCode::NotYetImplemented, "Sub-selects are not yet implemented");
+        auto table_def_or_error = context.database->get_table(table_descriptor.schema_name(), table_descriptor.table_name());
+        if (table_def_or_error.is_error())
+            return SQLResult::construct(SQLCommand::Select, SQLErrorCode::InternalError, table_def_or_error.error());
+        auto table = table_def_or_error.value();
         if (table->num_columns() == 0)
             continue;
         auto old_descriptor_size = descriptor->size();
         descriptor->extend(table->to_tuple_descriptor());
         for (auto cartesian_row = rows.first(); cartesian_row.size() == old_descriptor_size; cartesian_row = rows.first()) {
             rows.remove(0);
-            for (auto& table_row : context.database->select_all(*table)) {
+            auto table_rows_or_error = context.database->select_all(*table);
+            if (table_rows_or_error.is_error())
+                return SQLResult::construct(SQLCommand::Create, SQLErrorCode::InternalError, table_rows_or_error.error());
+            for (auto& table_row : table_rows_or_error.value()) {
                 auto new_row = cartesian_row;
                 new_row.extend(table_row);
                 rows.append(new_row);

Userland/Libraries/LibSQL/Database.cpp

@@ -18,31 +18,65 @@
 namespace SQL {
 
 Database::Database(String name)
-    : m_heap(Heap::construct(name))
+    : m_heap(Heap::construct(move(name)))
     , m_serializer(m_heap)
-    , m_schemas(BTree::construct(m_serializer, SchemaDef::index_def()->to_tuple_descriptor(), m_heap->schemas_root()))
-    , m_tables(BTree::construct(m_serializer, TableDef::index_def()->to_tuple_descriptor(), m_heap->tables_root()))
-    , m_table_columns(BTree::construct(m_serializer, ColumnDef::index_def()->to_tuple_descriptor(), m_heap->table_columns_root()))
 {
+}
+
+ErrorOr<void> Database::open()
+{
+    TRY(m_heap->open());
+    m_schemas = BTree::construct(m_serializer, SchemaDef::index_def()->to_tuple_descriptor(), m_heap->schemas_root());
     m_schemas->on_new_root = [&]() {
         m_heap->set_schemas_root(m_schemas->root());
     };
+
+    m_tables = BTree::construct(m_serializer, TableDef::index_def()->to_tuple_descriptor(), m_heap->tables_root());
     m_tables->on_new_root = [&]() {
         m_heap->set_tables_root(m_tables->root());
     };
+
+    m_table_columns = BTree::construct(m_serializer, ColumnDef::index_def()->to_tuple_descriptor(), m_heap->table_columns_root());
     m_table_columns->on_new_root = [&]() {
         m_heap->set_table_columns_root(m_table_columns->root());
     };
-    auto default_schema = get_schema("default");
+
+    m_open = true;
+    auto default_schema = TRY(get_schema("default"));
     if (!default_schema) {
         default_schema = SchemaDef::construct("default");
-        add_schema(*default_schema);
+        TRY(add_schema(*default_schema));
     }
+    return {};
+}
+
+Database::~Database()
+{
+    // This crashes if the database can't commit. It's recommended to commit
+    // before the Database goes out of scope so the application can handle
+    // errors.
+    // Maybe we should enforce that by having a VERIFY here that there are no
+    // pending writes. But that's a new API on Heap so let's not do that right
+    // now.
+    if (is_open())
+        MUST(commit());
+}
+
+ErrorOr<void> Database::commit()
+{
+    VERIFY(is_open());
+    TRY(m_heap->flush());
+    return {};
 }
 
-void Database::add_schema(SchemaDef const& schema)
+ErrorOr<void> Database::add_schema(SchemaDef const& schema)
 {
-    m_schemas->insert(schema.key());
+    VERIFY(is_open());
+    if (!m_schemas->insert(schema.key())) {
+        warnln("Duplicate schema name {}"sv, schema.name());
+        return Error::from_string_literal("Duplicate schema name"sv);
+    }
+    return {};
 }
 
 Key Database::get_schema_key(String const& schema_name)
@@ -52,30 +86,37 @@ Key Database::get_schema_key(String const& schema_name)
     return key;
 }
 
-RefPtr<SchemaDef> Database::get_schema(String const& schema)
+ErrorOr<RefPtr<SchemaDef>> Database::get_schema(String const& schema)
 {
+    VERIFY(is_open());
     auto schema_name = schema;
     if (schema.is_null() || schema.is_empty())
         schema_name = "default";
     Key key = get_schema_key(schema_name);
     auto schema_def_opt = m_schema_cache.get(key.hash());
-    if (schema_def_opt.has_value())
-        return schema_def_opt.value();
+    if (schema_def_opt.has_value()) {
+        return RefPtr<SchemaDef>(schema_def_opt.value());
+    }
     auto schema_iterator = m_schemas->find(key);
     if (schema_iterator.is_end() || (*schema_iterator != key)) {
-        return nullptr;
+        return RefPtr<SchemaDef>(nullptr);
     }
     auto ret = SchemaDef::construct(*schema_iterator);
     m_schema_cache.set(key.hash(), ret);
-    return ret;
+    return RefPtr<SchemaDef>(ret);
 }
 
-void Database::add_table(TableDef& table)
+ErrorOr<void> Database::add_table(TableDef& table)
 {
-    m_tables->insert(table.key());
+    VERIFY(is_open());
+    if (!m_tables->insert(table.key())) {
+        warnln("Duplicate table name '{}'.'{}'"sv, table.parent()->name(), table.name());
+        return Error::from_string_literal("Duplicate table name"sv);
+    }
     for (auto& column : table.columns()) {
-        m_table_columns->insert(column.key());
+        VERIFY(m_table_columns->insert(column.key()));
     }
+    return {};
 }
 
 Key Database::get_table_key(String const& schema_name, String const& table_name)
@@ -85,36 +126,39 @@ Key Database::get_table_key(String const& schema_name, String const& table_name)
     return key;
 }
 
-RefPtr<TableDef> Database::get_table(String const& schema, String const& name)
+ErrorOr<RefPtr<TableDef>> Database::get_table(String const& schema, String const& name)
 {
+    VERIFY(is_open());
     auto schema_name = schema;
     if (schema.is_null() || schema.is_empty())
         schema_name = "default";
     Key key = get_table_key(schema_name, name);
     auto table_def_opt = m_table_cache.get(key.hash());
     if (table_def_opt.has_value())
-        return table_def_opt.value();
+        return RefPtr<TableDef>(table_def_opt.value());
     auto table_iterator = m_tables->find(key);
     if (table_iterator.is_end() || (*table_iterator != key)) {
-        return nullptr;
+        return RefPtr<TableDef>(nullptr);
+    }
+    auto schema_def = TRY(get_schema(schema));
+    if (!schema_def) {
+        warnln("Schema '{}' does not exist"sv, schema);
+        return Error::from_string_literal("Schema does not exist"sv);
     }
-    auto schema_def = get_schema(schema);
-    VERIFY(schema_def);
     auto ret = TableDef::construct(schema_def, name);
     ret->set_pointer((*table_iterator).pointer());
     m_table_cache.set(key.hash(), ret);
     auto hash = ret->hash();
     auto column_key = ColumnDef::make_key(ret);
-
     for (auto column_iterator = m_table_columns->find(column_key);
          !column_iterator.is_end() && ((*column_iterator)["table_hash"].to_u32().value() == hash);
          column_iterator++) {
         ret->append_column(*column_iterator);
     }
-    return ret;
+    return RefPtr<TableDef>(ret);
 }
 
-Vector<Row> Database::select_all(TableDef const& table)
+ErrorOr<Vector<Row>> Database::select_all(TableDef const& table)
 {
     VERIFY(m_table_cache.get(table.key().hash()).has_value());
     Vector<Row> ret;
@@ -124,7 +168,7 @@ Vector<Row> Database::select_all(TableDef const& table)
     return ret;
 }
 
-Vector<Row> Database::match(TableDef const& table, Key const& key)
+ErrorOr<Vector<Row>> Database::match(TableDef const& table, Key const& key)
 {
     VERIFY(m_table_cache.get(table.key().hash()).has_value());
     Vector<Row> ret;
@@ -140,12 +184,14 @@ Vector<Row> Database::match(TableDef const& table, Key const& key)
     return ret;
 }
 
-bool Database::insert(Row& row)
+ErrorOr<void> Database::insert(Row& row)
 {
     VERIFY(m_table_cache.get(row.table()->key().hash()).has_value());
+    // TODO Check constraints
+
     row.set_pointer(m_heap->new_record_pointer());
     row.next_pointer(row.table()->pointer());
-    update(row);
+    TRY(update(row));
 
     // TODO update indexes defined on table.
 
@@ -153,16 +199,18 @@ bool Database::insert(Row& row)
     table_key.set_pointer(row.pointer());
     VERIFY(m_tables->update_key_pointer(table_key));
     row.table()->set_pointer(row.pointer());
-    return true;
+    return {};
 }
 
-bool Database::update(Row& tuple)
+ErrorOr<void> Database::update(Row& tuple)
 {
     VERIFY(m_table_cache.get(tuple.table()->key().hash()).has_value());
+    // TODO Check constraints
     m_serializer.reset();
-    return m_serializer.serialize_and_write<Tuple>(tuple, tuple.pointer());
+    m_serializer.serialize_and_write<Tuple>(tuple, tuple.pointer());
 
     // TODO update indexes defined on table.
+    return {};
 }
 
 }

Userland/Libraries/LibSQL/Database.h

@@ -24,26 +24,29 @@ class Database : public Core::Object {
     C_OBJECT(Database);
 
 public:
-    ~Database() override = default;
+    ~Database() override;
 
-    void commit() { m_heap->flush(); }
+    ErrorOr<void> open();
+    bool is_open() const { return m_open; }
+    ErrorOr<void> commit();
 
-    void add_schema(SchemaDef const&);
+    ErrorOr<void> add_schema(SchemaDef const&);
     static Key get_schema_key(String const&);
-    RefPtr<SchemaDef> get_schema(String const&);
+    ErrorOr<RefPtr<SchemaDef>> get_schema(String const&);
 
-    void add_table(TableDef& table);
+    ErrorOr<void> add_table(TableDef& table);
     static Key get_table_key(String const&, String const&);
-    RefPtr<TableDef> get_table(String const&, String const&);
+    ErrorOr<RefPtr<TableDef>> get_table(String const&, String const&);
 
-    Vector<Row> select_all(TableDef const&);
-    Vector<Row> match(TableDef const&, Key const&);
-    bool insert(Row&);
-    bool update(Row&);
+    ErrorOr<Vector<Row>> select_all(TableDef const&);
+    ErrorOr<Vector<Row>> match(TableDef const&, Key const&);
+    ErrorOr<void> insert(Row&);
+    ErrorOr<void> update(Row&);
 
 private:
     explicit Database(String);
 
+    bool m_open { false };
     NonnullRefPtr<Heap> m_heap;
     Serializer m_serializer;
     RefPtr<BTree> m_schemas;

Userland/Libraries/LibSQL/Heap.cpp

@@ -18,13 +18,28 @@ namespace SQL {
 Heap::Heap(String file_name)
 {
     set_name(move(file_name));
+}
+
+Heap::~Heap()
+{
+    if (m_file && !m_write_ahead_log.is_empty()) {
+        if (auto maybe_error = flush(); maybe_error.is_error())
+            warnln("~Heap({}): {}", name(), maybe_error.error());
+    }
+}
+
+ErrorOr<void> Heap::open()
+{
     size_t file_size = 0;
     struct stat stat_buffer;
     if (stat(name().characters(), &stat_buffer) != 0) {
         if (errno != ENOENT) {
-            perror("stat");
-            VERIFY_NOT_REACHED();
+            warnln("Heap::open({}): could not stat: {}"sv, name(), strerror(errno));
+            return Error::from_string_literal("Heap::open(): could not stat file"sv);
         }
+    } else if (!S_ISREG(stat_buffer.st_mode)) {
+        warnln("Heap::open({}): can only use regular files"sv, name());
+        return Error::from_string_literal("Heap::open(): can only use regular files"sv);
     } else {
         file_size = stat_buffer.st_size;
     }
@@ -33,30 +48,43 @@ Heap::Heap(String file_name)
 
     auto file_or_error = Core::File::open(name(), Core::OpenMode::ReadWrite);
     if (file_or_error.is_error()) {
-        warnln("Couldn't open '{}': {}", name(), file_or_error.error());
-        VERIFY_NOT_REACHED();
+        warnln("Heap::open({}): could not open: {}"sv, name(), file_or_error.error());
+        return Error::from_string_literal("Heap::open(): could not open file"sv);
     }
     m_file = file_or_error.value();
-    if (file_size > 0)
-        read_zero_block();
-    else
+    if (file_size > 0) {
+        if (auto error_maybe = read_zero_block(); error_maybe.is_error()) {
+            m_file = nullptr;
+            return error_maybe.error();
+        }
+    } else {
         initialize_zero_block();
-    dbgln_if(SQL_DEBUG, "Heap file {} opened. Size = {}", file_name, size());
+    }
+    dbgln_if(SQL_DEBUG, "Heap file {} opened. Size = {}", name(), size());
+    return {};
 }
 
 ErrorOr<ByteBuffer> Heap::read_block(u32 block)
 {
+    if (m_file.is_null()) {
+        warnln("Heap({})::read_block({}): Heap file not opened"sv, name(), block);
+        return Error::from_string_literal("Heap()::read_block(): Heap file not opened"sv);
+    }
     auto buffer_or_empty = m_write_ahead_log.get(block);
     if (buffer_or_empty.has_value())
         return buffer_or_empty.value();
 
-    VERIFY(block < m_next_block);
+    if (block >= m_next_block) {
+        warnln("Heap({})::read_block({}): block # out of range (>= {})"sv, name(), block, m_next_block);
+        return Error::from_string_literal("Heap()::read_block(): block # out of range"sv);
+    }
     dbgln_if(SQL_DEBUG, "Read heap block {}", block);
-    if (!seek_block(block))
-        VERIFY_NOT_REACHED();
+    TRY(seek_block(block));
     auto ret = m_file->read(BLOCKSIZE);
-    if (ret.is_empty())
-        return Error::from_string_literal("Could not read block"sv);
+    if (ret.is_empty()) {
+        warnln("Heap({})::read_block({}): Could not read block"sv, name(), block);
+        return Error::from_string_literal("Heap()::read_block(): Could not read block"sv);
+    }
     dbgln_if(SQL_DEBUG, "{:02x} {:02x} {:02x} {:02x} {:02x} {:02x} {:02x} {:02x}",
         *ret.offset_pointer(0), *ret.offset_pointer(1),
         *ret.offset_pointer(2), *ret.offset_pointer(3),
@@ -65,18 +93,28 @@ ErrorOr<ByteBuffer> Heap::read_block(u32 block)
     return ret;
 }
 
-bool Heap::write_block(u32 block, ByteBuffer& buffer)
+ErrorOr<void> Heap::write_block(u32 block, ByteBuffer& buffer)
 {
-    dbgln_if(SQL_DEBUG, "write_block({}): m_next_block {}", block, m_next_block);
-    VERIFY(block <= m_next_block);
-    if (!seek_block(block))
-        VERIFY_NOT_REACHED();
+    if (m_file.is_null()) {
+        warnln("Heap({})::write_block({}): Heap file not opened"sv, name(), block);
+        return Error::from_string_literal("Heap()::write_block(): Heap file not opened"sv);
+    }
+    if (block > m_next_block) {
+        warnln("Heap({})::write_block({}): block # out of range (> {})"sv, name(), block, m_next_block);
+        return Error::from_string_literal("Heap()::write_block(): block # out of range"sv);
+    }
+    TRY(seek_block(block));
     dbgln_if(SQL_DEBUG, "Write heap block {} size {}", block, buffer.size());
-    VERIFY(buffer.size() <= BLOCKSIZE);
+    if (buffer.size() > BLOCKSIZE) {
+        warnln("Heap({})::write_block({}): Oversized block ({} > {})"sv, name(), block, buffer.size(), BLOCKSIZE);
+        return Error::from_string_literal("Heap()::write_block(): Oversized block"sv);
+    }
     auto sz = buffer.size();
     if (sz < BLOCKSIZE) {
-        if (buffer.try_resize(BLOCKSIZE).is_error())
-            return false;
+        if (buffer.try_resize(BLOCKSIZE).is_error()) {
+            warnln("Heap({})::write_block({}): Could not align block of size {} to {}"sv, name(), block, buffer.size(), BLOCKSIZE);
+            return Error::from_string_literal("Heap()::write_block(): Could not align block"sv);
+        }
         memset(buffer.offset_pointer((int)sz), 0, BLOCKSIZE - sz);
     }
     dbgln_if(SQL_DEBUG, "{:02x} {:02x} {:02x} {:02x} {:02x} {:02x} {:02x} {:02x}",
@@ -87,35 +125,39 @@ bool Heap::write_block(u32 block, ByteBuffer& buffer)
     if (m_file->write(buffer.data(), (int)buffer.size())) {
         if (block == m_end_of_file)
             m_end_of_file++;
-        return true;
+        return {};
     }
-    return false;
+    warnln("Heap({})::write_block({}): Could not full write block"sv, name(), block);
+    return Error::from_string_literal("Heap()::write_block(): Could not full write block"sv);
 }
 
-bool Heap::seek_block(u32 block)
+ErrorOr<void> Heap::seek_block(u32 block)
 {
+    if (m_file.is_null()) {
+        warnln("Heap({})::seek_block({}): Heap file not opened"sv, name(), block);
+        return Error::from_string_literal("Heap()::seek_block(): Heap file not opened"sv);
+    }
     if (block == m_end_of_file) {
         off_t pos;
         if (!m_file->seek(0, Core::SeekMode::FromEndPosition, &pos)) {
-            warnln("Could not seek block {} from file {}, which is at the end of the file", block, name());
-            warnln("FD: {} Position: {} error: {}", m_file->fd(), pos, m_file->error_string());
-            return false;
+            warnln("Heap({})::seek_block({}): Error seeking end of file: {}"sv, name(), block, m_file->error_string());
+            return Error::from_string_literal("Heap()::seek_block(): Error seeking end of file"sv);
         }
     } else if (block > m_end_of_file) {
-        warnln("Seeking block {} of file {} which is beyond the end of the file", block, name());
-        return false;
+        warnln("Heap({})::seek_block({}): Cannot seek beyond end of file at block {}"sv, name(), block, m_end_of_file);
+        return Error::from_string_literal("Heap()::seek_block(): Cannot seek beyond end of file"sv);
     } else {
         if (!m_file->seek(block * BLOCKSIZE)) {
-            warnln("Could not seek block {} of file {}. The current size is {} blocks",
-                block, name(), m_end_of_file);
-            return false;
+            warnln("Heap({})::seek_block({}): Error seeking: {}"sv, name(), block, m_file->error_string());
+            return Error::from_string_literal("Heap()::seek_block(): Error seeking: {}"sv);
         }
     }
-    return true;
+    return {};
 }
 
 u32 Heap::new_record_pointer()
 {
+    VERIFY(!m_file.is_null());
     if (m_free_list) {
         auto block_or_error = read_block(m_free_list);
         if (block_or_error.is_error()) {
@@ -130,8 +172,9 @@ u32 Heap::new_record_pointer()
     return m_next_block++;
 }
 
-void Heap::flush()
+ErrorOr<void> Heap::flush()
 {
+    VERIFY(!m_file.is_null());
     Vector<u32> blocks;
     for (auto& wal_entry : m_write_ahead_log) {
         blocks.append(wal_entry.key);
@@ -143,13 +186,14 @@ void Heap::flush()
             VERIFY_NOT_REACHED();
         }
         dbgln_if(SQL_DEBUG, "Flushing block {} to {}", block, name());
-        write_block(block, buffer_or_empty.value());
+        TRY(write_block(block, buffer_or_empty.value()));
     }
     m_write_ahead_log.clear();
     dbgln_if(SQL_DEBUG, "WAL flushed. Heap size = {}", size());
+    return {};
 }
 
-constexpr static const char* FILE_ID = "SerenitySQL ";
+constexpr static StringView FILE_ID = "SerenitySQL "sv;
 constexpr static int VERSION_OFFSET = 12;
 constexpr static int SCHEMAS_ROOT_OFFSET = 16;
 constexpr static int TABLES_ROOT_OFFSET = 20;
@@ -157,18 +201,17 @@ constexpr static int TABLE_COLUMNS_ROOT_OFFSET = 24;
 constexpr static int FREE_LIST_OFFSET = 28;
 constexpr static int USER_VALUES_OFFSET = 32;
 
-void Heap::read_zero_block()
+ErrorOr<void> Heap::read_zero_block()
 {
-    char file_id[256];
     auto bytes_or_error = read_block(0);
     if (bytes_or_error.is_error())
-        VERIFY_NOT_REACHED();
+        return bytes_or_error.error();
     auto buffer = bytes_or_error.value();
-    memcpy(file_id, buffer.offset_pointer(0), strlen(FILE_ID));
-    file_id[strlen(FILE_ID)] = 0;
-    if (strncmp(file_id, FILE_ID, strlen(FILE_ID)) != 0) {
-        warnln("Corrupt zero page in {}", name());
-        VERIFY_NOT_REACHED();
+    auto file_id_buffer = buffer.slice(0, FILE_ID.length());
+    auto file_id = StringView(file_id_buffer);
+    if (file_id != FILE_ID) {
+        warnln("{}: Zero page corrupt. This is probably not a {} heap file"sv, name(), FILE_ID);
+        return Error::from_string_literal("Heap()::read_zero_block(): Zero page corrupt. This is probably not a SerenitySQL heap file"sv);
     }
     dbgln_if(SQL_DEBUG, "Read zero block from {}", name());
     memcpy(&m_version, buffer.offset_pointer(VERSION_OFFSET), sizeof(u32));
@@ -187,6 +230,7 @@ void Heap::read_zero_block()
             dbgln_if(SQL_DEBUG, "User value {}: {}", ix, m_user_values[ix]);
         }
     }
+    return {};
 }
 
 void Heap::update_zero_block()
@@ -205,7 +249,7 @@ void Heap::update_zero_block()
 
     // FIXME: Handle an OOM failure here.
     auto buffer = ByteBuffer::create_zeroed(BLOCKSIZE).release_value();
-    buffer.overwrite(0, FILE_ID, strlen(FILE_ID));
+    buffer.overwrite(0, FILE_ID.characters_without_null_termination(), FILE_ID.length());
     buffer.overwrite(VERSION_OFFSET, &m_version, sizeof(u32));
     buffer.overwrite(SCHEMAS_ROOT_OFFSET, &m_schemas_root, sizeof(u32));
     buffer.overwrite(TABLES_ROOT_OFFSET, &m_tables_root, sizeof(u32));

Userland/Libraries/LibSQL/Heap.h

@@ -6,6 +6,7 @@
 
 #pragma once
 
+#include <AK/Array.h>
 #include <AK/Debug.h>
 #include <AK/HashMap.h>
 #include <AK/String.h>
@@ -32,13 +33,14 @@ class Heap : public Core::Object {
     C_OBJECT(Heap);
 
 public:
-    virtual ~Heap() override { flush(); }
+    virtual ~Heap() override;
 
+    ErrorOr<void> open();
     u32 size() const { return m_end_of_file; }
     ErrorOr<ByteBuffer> read_block(u32);
-    bool write_block(u32, ByteBuffer&);
-    u32 new_record_pointer();
+    [[nodiscard]] u32 new_record_pointer();
     [[nodiscard]] bool has_block(u32 block) const { return block < size(); }
+    [[nodiscard]] bool valid() const { return m_file != nullptr; }
 
     u32 schemas_root() const { return m_schemas_root; }
 
@@ -89,17 +91,18 @@ public:
         m_write_ahead_log.set(block, buffer);
     }
 
-    void flush();
+    ErrorOr<void> flush();
 
 private:
     explicit Heap(String);
 
-    bool seek_block(u32);
-    void read_zero_block();
+    ErrorOr<void> write_block(u32, ByteBuffer&);
+    ErrorOr<void> seek_block(u32);
+    ErrorOr<void> read_zero_block();
     void initialize_zero_block();
     void update_zero_block();
 
-    RefPtr<Core::File> m_file;
+    RefPtr<Core::File> m_file { nullptr };
     u32 m_free_list { 0 };
     u32 m_next_block { 1 };
     u32 m_end_of_file { 1 };
@@ -107,7 +110,7 @@ private:
     u32 m_tables_root { 0 };
     u32 m_table_columns_root { 0 };
     u32 m_version { 0x00000001 };
-    Array<u32, 16> m_user_values;
+    Array<u32, 16> m_user_values { 0 };
     HashMap<u32, ByteBuffer> m_write_ahead_log;
 };
 

Userland/Libraries/LibSQL/SQLResult.h

@@ -43,6 +43,8 @@ constexpr char const* command_tag(SQLCommand command)
 
 #define ENUMERATE_SQL_ERRORS(S)                                                          \
     S(NoError, "No error")                                                               \
+    S(InternalError, "{}")                                                               \
+    S(NotYetImplemented, "{}")                                                           \
     S(DatabaseUnavailable, "Database Unavailable")                                       \
     S(StatementUnavailable, "Statement with id '{}' Unavailable")                        \
     S(SyntaxError, "Syntax Error")                                                       \
@@ -147,6 +149,12 @@ private:
     {
     }
 
+    SQLResult(SQLCommand command, SQLErrorCode error_code, AK::Error error)
+        : m_command(command)
+        , m_error({ error_code, error.string_literal() })
+    {
+    }
+
     SQLCommand m_command { SQLCommand::Select };
     SQLError m_error { SQLErrorCode::NoError, "" };
     int m_update_count { 0 };

Userland/Services/SQLServer/DatabaseConnection.cpp

@@ -29,8 +29,7 @@ DatabaseConnection::DatabaseConnection(String database_name, int client_id)
     , m_connection_id(s_next_connection_id++)
     , m_client_id(client_id)
 {
-    LexicalPath path(database_name);
-    if (path.title() != database_name) {
+    if (LexicalPath path(m_database_name); (path.title() != m_database_name) || (path.dirname() != ".")) {
         auto client_connection = ClientConnection::client_connection_for(m_client_id);
         client_connection->async_connection_error(m_connection_id, (int)SQL::SQLErrorCode::InvalidDatabaseName, m_database_name);
         return;
@@ -40,8 +39,12 @@ DatabaseConnection::DatabaseConnection(String database_name, int client_id)
     s_connections.set(m_connection_id, *this);
     deferred_invoke([this]() {
         m_database = SQL::Database::construct(String::formatted("/home/anon/sql/{}.db", m_database_name));
-        m_accept_statements = true;
         auto client_connection = ClientConnection::client_connection_for(m_client_id);
+        if (auto maybe_error = m_database->open(); maybe_error.is_error()) {
+            client_connection->async_connection_error(m_connection_id, (int)SQL::SQLErrorCode::InternalError, maybe_error.error().string_literal());
+            return;
+        }
+        m_accept_statements = true;
         if (client_connection)
             client_connection->async_connected(m_connection_id, m_database_name);
         else