From 001949d77a3e78ccd54a28f075d5e67619ed79bd Mon Sep 17 00:00:00 2001 From: Jan de Visser Date: Fri, 5 Nov 2021 19:05:59 -0400 Subject: [PATCH] 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 template got deprecated in favour of ErrorOr. This resulted in more busywork. --- Tests/LibSQL/TestSqlBtreeIndex.cpp | 4 + Tests/LibSQL/TestSqlDatabase.cpp | 85 ++++++++--- Tests/LibSQL/TestSqlHashIndex.cpp | 4 + Tests/LibSQL/TestSqlStatementExecution.cpp | 38 +++-- .../Libraries/LibSQL/AST/CreateSchema.cpp | 8 +- Userland/Libraries/LibSQL/AST/CreateTable.cpp | 13 +- Userland/Libraries/LibSQL/AST/Expression.cpp | 7 +- Userland/Libraries/LibSQL/AST/Insert.cpp | 9 +- Userland/Libraries/LibSQL/AST/Select.cpp | 24 +++- Userland/Libraries/LibSQL/Database.cpp | 110 ++++++++++---- Userland/Libraries/LibSQL/Database.h | 23 +-- Userland/Libraries/LibSQL/Heap.cpp | 134 ++++++++++++------ Userland/Libraries/LibSQL/Heap.h | 19 +-- Userland/Libraries/LibSQL/SQLResult.h | 8 ++ .../Services/SQLServer/DatabaseConnection.cpp | 9 +- 15 files changed, 355 insertions(+), 140 deletions(-) diff --git a/Tests/LibSQL/TestSqlBtreeIndex.cpp b/Tests/LibSQL/TestSqlBtreeIndex.cpp index 92d63cf6e7..d38547762d 100644 --- a/Tests/LibSQL/TestSqlBtreeIndex.cpp +++ b/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); diff --git a/Tests/LibSQL/TestSqlDatabase.cpp b/Tests/LibSQL/TestSqlDatabase.cpp index e57b7bd8a5..4e6b0df26a 100644 --- a/Tests/LibSQL/TestSqlDatabase.cpp +++ b/Tests/LibSQL/TestSqlDatabase.cpp @@ -20,11 +20,13 @@ NonnullRefPtr 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 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 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); diff --git a/Tests/LibSQL/TestSqlHashIndex.cpp b/Tests/LibSQL/TestSqlHashIndex.cpp index 57f862a1d3..7b47ad9d00 100644 --- a/Tests/LibSQL/TestSqlHashIndex.cpp +++ b/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 found; diff --git a/Tests/LibSQL/TestSqlStatementExecution.cpp b/Tests/LibSQL/TestSqlStatementExecution.cpp index e46056e427..a96632c257 100644 --- a/Tests/LibSQL/TestSqlStatementExecution.cpp +++ b/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 " diff --git a/Userland/Libraries/LibSQL/AST/CreateSchema.cpp b/Userland/Libraries/LibSQL/AST/CreateSchema.cpp index e387279506..319f2f4350 100644 --- a/Userland/Libraries/LibSQL/AST/CreateSchema.cpp +++ b/Userland/Libraries/LibSQL/AST/CreateSchema.cpp @@ -12,7 +12,10 @@ namespace SQL::AST { RefPtr 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 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); } diff --git a/Userland/Libraries/LibSQL/AST/CreateTable.cpp b/Userland/Libraries/LibSQL/AST/CreateTable.cpp index 090cb7036d..9a4ba58764 100644 --- a/Userland/Libraries/LibSQL/AST/CreateTable.cpp +++ b/Userland/Libraries/LibSQL/AST/CreateTable.cpp @@ -12,10 +12,16 @@ namespace SQL::AST { RefPtr 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 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); } diff --git a/Userland/Libraries/LibSQL/AST/Expression.cpp b/Userland/Libraries/LibSQL/AST/Expression.cpp index a7b793e602..d4030b3532 100644 --- a/Userland/Libraries/LibSQL/AST/Expression.cpp +++ b/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(); diff --git a/Userland/Libraries/LibSQL/AST/Insert.cpp b/Userland/Libraries/LibSQL/AST/Insert.cpp index 5e1c39a91b..a2eb08451b 100644 --- a/Userland/Libraries/LibSQL/AST/Insert.cpp +++ b/Userland/Libraries/LibSQL/AST/Insert.cpp @@ -27,7 +27,10 @@ static bool does_value_data_type_match(SQLType expected, SQLType actual) RefPtr 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 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); diff --git a/Userland/Libraries/LibSQL/AST/Select.cpp b/Userland/Libraries/LibSQL/AST/Select.cpp index 81ef322a57..d890f1c7e9 100644 --- a/Userland/Libraries/LibSQL/AST/Select.cpp +++ b/Userland/Libraries/LibSQL/AST/Select.cpp @@ -16,11 +16,15 @@ RefPtr Select::execute(ExecutionContext& context) const NonnullRefPtrVector 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 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 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); diff --git a/Userland/Libraries/LibSQL/Database.cpp b/Userland/Libraries/LibSQL/Database.cpp index 23f7774b42..e0574f1d59 100644 --- a/Userland/Libraries/LibSQL/Database.cpp +++ b/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 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 {}; } -void Database::add_schema(SchemaDef const& schema) +Database::~Database() { - m_schemas->insert(schema.key()); + // 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 Database::commit() +{ + VERIFY(is_open()); + TRY(m_heap->flush()); + return {}; +} + +ErrorOr Database::add_schema(SchemaDef const& schema) +{ + 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 Database::get_schema(String const& schema) +ErrorOr> 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(schema_def_opt.value()); + } auto schema_iterator = m_schemas->find(key); if (schema_iterator.is_end() || (*schema_iterator != key)) { - return nullptr; + return RefPtr(nullptr); } auto ret = SchemaDef::construct(*schema_iterator); m_schema_cache.set(key.hash(), ret); - return ret; + return RefPtr(ret); } -void Database::add_table(TableDef& table) +ErrorOr Database::add_table(TableDef& table) { - m_tables->insert(table.key()); - for (auto& column : table.columns()) { - m_table_columns->insert(column.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()) { + 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 Database::get_table(String const& schema, String const& name) +ErrorOr> 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(table_def_opt.value()); auto table_iterator = m_tables->find(key); if (table_iterator.is_end() || (*table_iterator != key)) { - return nullptr; + return RefPtr(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(ret); } -Vector Database::select_all(TableDef const& table) +ErrorOr> Database::select_all(TableDef const& table) { VERIFY(m_table_cache.get(table.key().hash()).has_value()); Vector ret; @@ -124,7 +168,7 @@ Vector Database::select_all(TableDef const& table) return ret; } -Vector Database::match(TableDef const& table, Key const& key) +ErrorOr> Database::match(TableDef const& table, Key const& key) { VERIFY(m_table_cache.get(table.key().hash()).has_value()); Vector ret; @@ -140,12 +184,14 @@ Vector Database::match(TableDef const& table, Key const& key) return ret; } -bool Database::insert(Row& row) +ErrorOr 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 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.pointer()); + m_serializer.serialize_and_write(tuple, tuple.pointer()); // TODO update indexes defined on table. + return {}; } } diff --git a/Userland/Libraries/LibSQL/Database.h b/Userland/Libraries/LibSQL/Database.h index ae562941bc..662036c994 100644 --- a/Userland/Libraries/LibSQL/Database.h +++ b/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 open(); + bool is_open() const { return m_open; } + ErrorOr commit(); - void add_schema(SchemaDef const&); + ErrorOr add_schema(SchemaDef const&); static Key get_schema_key(String const&); - RefPtr get_schema(String const&); + ErrorOr> get_schema(String const&); - void add_table(TableDef& table); + ErrorOr add_table(TableDef& table); static Key get_table_key(String const&, String const&); - RefPtr get_table(String const&, String const&); + ErrorOr> get_table(String const&, String const&); - Vector select_all(TableDef const&); - Vector match(TableDef const&, Key const&); - bool insert(Row&); - bool update(Row&); + ErrorOr> select_all(TableDef const&); + ErrorOr> match(TableDef const&, Key const&); + ErrorOr insert(Row&); + ErrorOr update(Row&); private: explicit Database(String); + bool m_open { false }; NonnullRefPtr m_heap; Serializer m_serializer; RefPtr m_schemas; diff --git a/Userland/Libraries/LibSQL/Heap.cpp b/Userland/Libraries/LibSQL/Heap.cpp index fb99fa5fa4..0929d4d05b 100644 --- a/Userland/Libraries/LibSQL/Heap.cpp +++ b/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 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 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 Heap::read_block(u32 block) return ret; } -bool Heap::write_block(u32 block, ByteBuffer& buffer) +ErrorOr 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 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 Heap::flush() { + VERIFY(!m_file.is_null()); Vector 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 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)); diff --git a/Userland/Libraries/LibSQL/Heap.h b/Userland/Libraries/LibSQL/Heap.h index 51c21a224a..da0285a0e0 100644 --- a/Userland/Libraries/LibSQL/Heap.h +++ b/Userland/Libraries/LibSQL/Heap.h @@ -6,6 +6,7 @@ #pragma once +#include #include #include #include @@ -32,13 +33,14 @@ class Heap : public Core::Object { C_OBJECT(Heap); public: - virtual ~Heap() override { flush(); } + virtual ~Heap() override; + ErrorOr open(); u32 size() const { return m_end_of_file; } ErrorOr 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 flush(); private: explicit Heap(String); - bool seek_block(u32); - void read_zero_block(); + ErrorOr write_block(u32, ByteBuffer&); + ErrorOr seek_block(u32); + ErrorOr read_zero_block(); void initialize_zero_block(); void update_zero_block(); - RefPtr m_file; + RefPtr 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 m_user_values; + Array m_user_values { 0 }; HashMap m_write_ahead_log; }; diff --git a/Userland/Libraries/LibSQL/SQLResult.h b/Userland/Libraries/LibSQL/SQLResult.h index 6f14873655..8072f67b0a 100644 --- a/Userland/Libraries/LibSQL/SQLResult.h +++ b/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 }; diff --git a/Userland/Services/SQLServer/DatabaseConnection.cpp b/Userland/Services/SQLServer/DatabaseConnection.cpp index 97c50a32d5..f02d2cd415 100644 --- a/Userland/Services/SQLServer/DatabaseConnection.cpp +++ b/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