diff --git a/Tests/LibSQL/TestSqlDatabase.cpp b/Tests/LibSQL/TestSqlDatabase.cpp index 2e5c6b5ba7..021b43a74b 100644 --- a/Tests/LibSQL/TestSqlDatabase.cpp +++ b/Tests/LibSQL/TestSqlDatabase.cpp @@ -44,10 +44,7 @@ NonnullRefPtr setup_table(SQL::Database& db) void insert_into_table(SQL::Database& db, int count) { - auto table_or_error = db.get_table("TestSchema", "TestTable"); - EXPECT(!table_or_error.is_error()); - auto table = table_or_error.value(); - EXPECT(table); + auto table = MUST(db.get_table("TestSchema", "TestTable")); for (int ix = 0; ix < count; ix++) { SQL::Row row(*table); @@ -63,10 +60,7 @@ void insert_into_table(SQL::Database& db, int count) void verify_table_contents(SQL::Database& db, int expected_count) { - auto table_or_error = db.get_table("TestSchema", "TestTable"); - EXPECT(!table_or_error.is_error()); - auto table = table_or_error.value(); - EXPECT(table); + auto table = MUST(db.get_table("TestSchema", "TestTable")); int sum = 0; int count = 0; @@ -196,10 +190,8 @@ TEST_CASE(get_table_from_database) { auto db = SQL::Database::construct("/tmp/test.db"); 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); + + auto table = MUST(db->get_table("TestSchema", "TestTable")); EXPECT_EQ(table->name(), "TestTable"); EXPECT_EQ(table->num_columns(), 2u); } diff --git a/Tests/LibSQL/TestSqlStatementExecution.cpp b/Tests/LibSQL/TestSqlStatementExecution.cpp index 2e8e06b5ed..c7d293755a 100644 --- a/Tests/LibSQL/TestSqlStatementExecution.cpp +++ b/Tests/LibSQL/TestSqlStatementExecution.cpp @@ -81,7 +81,6 @@ TEST_CASE(create_table) create_table(database); 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) @@ -93,9 +92,7 @@ TEST_CASE(insert_into_table) auto result = execute(database, "INSERT INTO TestSchema.TestTable ( TextColumn, IntColumn ) VALUES ( 'Test', 42 );"); EXPECT(result.size() == 1); - auto table_or_error = database->get_table("TESTSCHEMA", "TESTTABLE"); - EXPECT(!table_or_error.is_error()); - auto table = table_or_error.value(); + auto table = MUST(database->get_table("TESTSCHEMA", "TESTTABLE")); int count = 0; auto rows_or_error = database->select_all(*table); @@ -172,9 +169,8 @@ TEST_CASE(insert_without_column_names) auto result = execute(database, "INSERT INTO TestSchema.TestTable VALUES ('Test_1', 42), ('Test_2', 43);"); EXPECT(result.size() == 2); - 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())); + auto table = MUST(database->get_table("TESTSCHEMA", "TESTTABLE")); + auto rows_or_error = database->select_all(*table); EXPECT(!rows_or_error.is_error()); EXPECT_EQ(rows_or_error.value().size(), 2u); } diff --git a/Userland/Libraries/LibSQL/AST/CreateTable.cpp b/Userland/Libraries/LibSQL/AST/CreateTable.cpp index 2312e0f23d..a40409ee53 100644 --- a/Userland/Libraries/LibSQL/AST/CreateTable.cpp +++ b/Userland/Libraries/LibSQL/AST/CreateTable.cpp @@ -12,16 +12,9 @@ namespace SQL::AST { ResultOr CreateTable::execute(ExecutionContext& context) const { auto schema_def = TRY(context.database->get_schema(m_schema_name)); - auto table_def = TRY(context.database->get_table(m_schema_name, m_table_name)); - if (table_def) { - if (m_is_error_if_table_exists) - return Result { SQLCommand::Create, SQLErrorCode::TableExists, m_table_name }; - return ResultSet { SQLCommand::Create }; - } + auto table_def = TableDef::construct(schema_def, m_table_name); - table_def = TableDef::construct(schema_def, m_table_name); - - for (auto& column : m_columns) { + for (auto const& column : m_columns) { SQLType type; if (column.type_name()->name().is_one_of("VARCHAR"sv, "TEXT"sv)) @@ -38,7 +31,11 @@ ResultOr CreateTable::execute(ExecutionContext& context) const table_def->append_column(column.name(), type); } - TRY(context.database->add_table(*table_def)); + if (auto result = context.database->add_table(*table_def); result.is_error()) { + if (result.error().error() != SQLErrorCode::TableExists || m_is_error_if_table_exists) + return result.release_error(); + } + return ResultSet { SQLCommand::Create }; } diff --git a/Userland/Libraries/LibSQL/AST/Describe.cpp b/Userland/Libraries/LibSQL/AST/Describe.cpp index 180243b782..8cc6dfcba8 100644 --- a/Userland/Libraries/LibSQL/AST/Describe.cpp +++ b/Userland/Libraries/LibSQL/AST/Describe.cpp @@ -14,15 +14,9 @@ namespace SQL::AST { ResultOr DescribeTable::execute(ExecutionContext& context) const { - auto schema_name = m_qualified_table_name->schema_name(); - auto table_name = m_qualified_table_name->table_name(); - + auto const& schema_name = m_qualified_table_name->schema_name(); + auto const& table_name = m_qualified_table_name->table_name(); auto table_def = TRY(context.database->get_table(schema_name, table_name)); - if (!table_def) { - if (schema_name.is_empty()) - schema_name = "default"sv; - return Result { SQLCommand::Describe, SQLErrorCode::TableDoesNotExist, String::formatted("{}.{}", schema_name, table_name) }; - } auto describe_table_def = MUST(context.database->get_table("master"sv, "internal_describe_table"sv)); auto descriptor = describe_table_def->to_tuple_descriptor(); diff --git a/Userland/Libraries/LibSQL/AST/Insert.cpp b/Userland/Libraries/LibSQL/AST/Insert.cpp index 0c9cfb4fa9..52fde78402 100644 --- a/Userland/Libraries/LibSQL/AST/Insert.cpp +++ b/Userland/Libraries/LibSQL/AST/Insert.cpp @@ -25,11 +25,6 @@ ResultOr Insert::execute(ExecutionContext& context) const { auto table_def = TRY(context.database->get_table(m_schema_name, m_table_name)); - if (!table_def) { - auto schema_name = m_schema_name.is_empty() ? String("default"sv) : m_schema_name; - return Result { SQLCommand::Insert, SQLErrorCode::TableDoesNotExist, String::formatted("{}.{}", schema_name, m_table_name) }; - } - Row row(table_def); for (auto& column : m_column_names) { if (!row.has(column)) diff --git a/Userland/Libraries/LibSQL/AST/Select.cpp b/Userland/Libraries/LibSQL/AST/Select.cpp index 663634c0a2..8dfa6a93cc 100644 --- a/Userland/Libraries/LibSQL/AST/Select.cpp +++ b/Userland/Libraries/LibSQL/AST/Select.cpp @@ -24,8 +24,6 @@ ResultOr Select::execute(ExecutionContext& context) const return Result { SQLCommand::Select, SQLErrorCode::NotYetImplemented, "Sub-selects are not yet implemented"sv }; auto table_def = TRY(context.database->get_table(table_descriptor.schema_name(), table_descriptor.table_name())); - if (!table_def) - return Result { 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_def->columns()) { diff --git a/Userland/Libraries/LibSQL/Database.cpp b/Userland/Libraries/LibSQL/Database.cpp index 401d18fb21..f2762607c5 100644 --- a/Userland/Libraries/LibSQL/Database.cpp +++ b/Userland/Libraries/LibSQL/Database.cpp @@ -61,12 +61,14 @@ ResultOr Database::open() (void)TRY(ensure_schema_exists("default"sv)); auto master_schema = TRY(ensure_schema_exists("master"sv)); - auto table_def = TRY(get_table("master", "internal_describe_table")); - if (!table_def) { - auto describe_internal_table = TableDef::construct(master_schema, "internal_describe_table"); - describe_internal_table->append_column("Name", SQLType::Text); - describe_internal_table->append_column("Type", SQLType::Text); - TRY(add_table(*describe_internal_table)); + if (auto result = get_table("master"sv, "internal_describe_table"sv); result.is_error()) { + if (result.error().error() != SQLErrorCode::TableDoesNotExist) + return result.release_error(); + + auto internal_describe_table = TableDef::construct(master_schema, "internal_describe_table"); + internal_describe_table->append_column("Name", SQLType::Text); + internal_describe_table->append_column("Type", SQLType::Text); + TRY(add_table(*internal_describe_table)); } return {}; @@ -118,16 +120,18 @@ ResultOr> Database::get_schema(String const& schema) return schema_def; } -ErrorOr Database::add_table(TableDef& table) +ResultOr Database::add_table(TableDef& table) { 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"); - } + + if (!m_tables->insert(table.key())) + return Result { SQLCommand::Unknown, SQLErrorCode::TableExists, table.name() }; + for (auto& column : table.columns()) { - VERIFY(m_table_columns->insert(column.key())); + if (!m_table_columns->insert(column.key())) + VERIFY_NOT_REACHED(); } + return {}; } @@ -138,32 +142,33 @@ Key Database::get_table_key(String const& schema_name, String const& table_name) return key; } -ResultOr> Database::get_table(String const& schema, String const& name) +ResultOr> 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"; + if (schema.is_empty()) + schema_name = "default"sv; + 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 RefPtr(table_def_opt.value()); + if (auto it = m_table_cache.find(key.hash()); it != m_table_cache.end()) + return it->value; + auto table_iterator = m_tables->find(key); - if (table_iterator.is_end() || (*table_iterator != key)) { - return RefPtr(nullptr); - } + if (table_iterator.is_end() || (*table_iterator != key)) + return Result { SQLCommand::Unknown, SQLErrorCode::TableDoesNotExist, String::formatted("{}.{}", schema_name, name) }; + auto schema_def = TRY(get_schema(schema)); - 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 RefPtr(ret); + auto table_def = TableDef::construct(schema_def, name); + table_def->set_pointer((*table_iterator).pointer()); + m_table_cache.set(key.hash(), table_def); + + auto table_hash = table_def->hash(); + auto column_key = ColumnDef::make_key(table_def); + for (auto it = m_table_columns->find(column_key); !it.is_end() && ((*it)["table_hash"].to_u32().value() == table_hash); ++it) + table_def->append_column(*it); + + return table_def; } ErrorOr> Database::select_all(TableDef const& table) diff --git a/Userland/Libraries/LibSQL/Database.h b/Userland/Libraries/LibSQL/Database.h index 29c97c089c..baf7ebb709 100644 --- a/Userland/Libraries/LibSQL/Database.h +++ b/Userland/Libraries/LibSQL/Database.h @@ -37,9 +37,9 @@ public: static Key get_schema_key(String const&); ResultOr> get_schema(String const&); - ErrorOr add_table(TableDef& table); + ResultOr add_table(TableDef& table); static Key get_table_key(String const&, String const&); - ResultOr> get_table(String const&, String const&); + ResultOr> get_table(String const&, String const&); ErrorOr> select_all(TableDef const&); ErrorOr> match(TableDef const&, Key const&); @@ -57,7 +57,7 @@ private: RefPtr m_table_columns; HashMap> m_schema_cache; - HashMap> m_table_cache; + HashMap> m_table_cache; }; }