LibSQL+SQLServer+SQLStudio+sql: Use proper types for SQL IPC and IDs

When storing IDs and sending values over IPC, this changes SQLServer to:

1. Stop using -1 as a nominal "bad" ID. Store the IDs as unsigned, and
   use Optional in the one place that the IPC needs to indicate an ID
   was not allocated.

2. Let LibIPC encode/decode enumerations (SQLErrorCode) on our behalf.

3. Use size_t for array sizes.

Userland/DevTools/SQLStudio/MainWidget.cpp

@@ -214,13 +214,13 @@ MainWidget::MainWidget()
     m_statusbar->segment(2).set_fixed_width(font().width("Ln 0000, Col 000"sv) + font().max_glyph_width());
 
     m_sql_client = SQL::SQLClient::try_create().release_value_but_fixme_should_propagate_errors();
-    m_sql_client->on_execution_success = [this](int, bool, int, int, int) {
+    m_sql_client->on_execution_success = [this](auto, auto, auto, auto, auto) {
         read_next_sql_statement_of_editor();
     };
-    m_sql_client->on_next_result = [this](int, Vector<DeprecatedString> const& row) {
+    m_sql_client->on_next_result = [this](auto, auto const& row) {
         m_results.append(row);
     };
-    m_sql_client->on_results_exhausted = [this](int, int) {
+    m_sql_client->on_results_exhausted = [this](auto, auto) {
         if (m_results.size() == 0)
             return;
         if (m_results[0].size() == 0)
@@ -478,8 +478,8 @@ DeprecatedString MainWidget::read_next_sql_statement_of_editor()
             m_editor_line_level = last_token_ended_statement ? 0 : (m_editor_line_level > 0 ? m_editor_line_level : 1);
     } while ((m_editor_line_level > 0) || piece.is_empty());
 
-    auto statement_id = m_sql_client->prepare_statement(m_connection_id, piece.to_deprecated_string());
+    if (auto statement_id = m_sql_client->prepare_statement(m_connection_id, piece.to_deprecated_string()); statement_id.has_value())
-    m_sql_client->async_execute_statement(statement_id, {});
+        m_sql_client->async_execute_statement(*statement_id, {});
 
     return piece.to_deprecated_string();
 }

Userland/DevTools/SQLStudio/MainWidget.h

@@ -65,7 +65,7 @@ private:
     Optional<DeprecatedString> read_next_line_of_editor();
     size_t m_current_line_for_parsing { 0 };
     int m_editor_line_level { 0 };
-    int m_connection_id { 0 };
+    u64 m_connection_id { 0 };
 };
 
 }

Userland/Libraries/LibSQL/SQLClient.cpp

@@ -9,35 +9,35 @@
 
 namespace SQL {
 
-void SQLClient::connected(int connection_id, DeprecatedString const& connected_to_database)
+void SQLClient::connected(u64 connection_id, DeprecatedString const& connected_to_database)
 {
     if (on_connected)
         on_connected(connection_id, connected_to_database);
 }
 
-void SQLClient::disconnected(int connection_id)
+void SQLClient::disconnected(u64 connection_id)
 {
     if (on_disconnected)
         on_disconnected(connection_id);
 }
 
-void SQLClient::connection_error(int connection_id, int code, DeprecatedString const& message)
+void SQLClient::connection_error(u64 connection_id, SQLErrorCode const& code, DeprecatedString const& message)
 {
     if (on_connection_error)
         on_connection_error(connection_id, code, message);
     else
-        warnln("Connection error for connection_id {}: {} ({})", connection_id, message, code);
+        warnln("Connection error for connection_id {}: {} ({})", connection_id, message, to_underlying(code));
 }
 
-void SQLClient::execution_error(int statement_id, int code, DeprecatedString const& message)
+void SQLClient::execution_error(u64 statement_id, SQLErrorCode const& code, DeprecatedString const& message)
 {
     if (on_execution_error)
         on_execution_error(statement_id, code, message);
     else
-        warnln("Execution error for statement_id {}: {} ({})", statement_id, message, code);
+        warnln("Execution error for statement_id {}: {} ({})", statement_id, message, to_underlying(code));
 }
 
-void SQLClient::execution_success(int statement_id, bool has_results, int created, int updated, int deleted)
+void SQLClient::execution_success(u64 statement_id, bool has_results, size_t created, size_t updated, size_t deleted)
 {
     if (on_execution_success)
         on_execution_success(statement_id, has_results, created, updated, deleted);
@@ -45,7 +45,7 @@ void SQLClient::execution_success(int statement_id, bool has_results, int create
         outln("{} row(s) created, {} updated, {} deleted", created, updated, deleted);
 }
 
-void SQLClient::next_result(int statement_id, Vector<DeprecatedString> const& row)
+void SQLClient::next_result(u64 statement_id, Vector<DeprecatedString> const& row)
 {
     if (on_next_result) {
         on_next_result(statement_id, row);
@@ -61,7 +61,7 @@ void SQLClient::next_result(int statement_id, Vector<DeprecatedString> const& ro
     outln();
 }
 
-void SQLClient::results_exhausted(int statement_id, int total_rows)
+void SQLClient::results_exhausted(u64 statement_id, size_t total_rows)
 {
     if (on_results_exhausted)
         on_results_exhausted(statement_id, total_rows);

Userland/Libraries/LibSQL/SQLClient.h

@@ -8,6 +8,7 @@
 #pragma once
 
 #include <LibIPC/ConnectionToServer.h>
+#include <LibSQL/Result.h>
 #include <SQLServer/SQLClientEndpoint.h>
 #include <SQLServer/SQLServerEndpoint.h>
 
@@ -19,13 +20,13 @@ class SQLClient
     IPC_CLIENT_CONNECTION(SQLClient, "/tmp/session/%sid/portal/sql"sv)
     virtual ~SQLClient() = default;
 
-    Function<void(int, DeprecatedString const&)> on_connected;
+    Function<void(u64, DeprecatedString const&)> on_connected;
-    Function<void(int)> on_disconnected;
+    Function<void(u64)> on_disconnected;
-    Function<void(int, int, DeprecatedString const&)> on_connection_error;
+    Function<void(u64, SQLErrorCode, DeprecatedString const&)> on_connection_error;
-    Function<void(int, int, DeprecatedString const&)> on_execution_error;
+    Function<void(u64, SQLErrorCode, DeprecatedString const&)> on_execution_error;
-    Function<void(int, bool, int, int, int)> on_execution_success;
+    Function<void(u64, bool, size_t, size_t, size_t)> on_execution_success;
-    Function<void(int, Vector<DeprecatedString> const&)> on_next_result;
+    Function<void(u64, Vector<DeprecatedString> const&)> on_next_result;
-    Function<void(int, int)> on_results_exhausted;
+    Function<void(u64, size_t)> on_results_exhausted;
 
 private:
     SQLClient(NonnullOwnPtr<Core::Stream::LocalSocket> socket)
@@ -33,13 +34,13 @@ private:
     {
     }
 
-    virtual void connected(int connection_id, DeprecatedString const& connected_to_database) override;
+    virtual void connected(u64 connection_id, DeprecatedString const& connected_to_database) override;
-    virtual void connection_error(int connection_id, int code, DeprecatedString const& message) override;
+    virtual void connection_error(u64 connection_id, SQLErrorCode const& code, DeprecatedString const& message) override;
-    virtual void execution_success(int statement_id, bool has_results, int created, int updated, int deleted) override;
+    virtual void execution_success(u64 statement_id, bool has_results, size_t created, size_t updated, size_t deleted) override;
-    virtual void next_result(int statement_id, Vector<DeprecatedString> const&) override;
+    virtual void next_result(u64 statement_id, Vector<DeprecatedString> const&) override;
-    virtual void results_exhausted(int statement_id, int total_rows) override;
+    virtual void results_exhausted(u64 statement_id, size_t total_rows) override;
-    virtual void execution_error(int statement_id, int code, DeprecatedString const& message) override;
+    virtual void execution_error(u64 statement_id, SQLErrorCode const& code, DeprecatedString const& message) override;
-    virtual void disconnected(int connection_id) override;
+    virtual void disconnected(u64 connection_id) override;
 };
 
 }

Userland/Services/SQLServer/ConnectionFromClient.cpp

@@ -41,7 +41,7 @@ Messages::SQLServer::ConnectResponse ConnectionFromClient::connect(DeprecatedStr
     return { database_connection->connection_id() };
 }
 
-void ConnectionFromClient::disconnect(int connection_id)
+void ConnectionFromClient::disconnect(u64 connection_id)
 {
     dbgln_if(SQLSERVER_DEBUG, "ConnectionFromClient::disconnect(connection_id: {})", connection_id);
     auto database_connection = DatabaseConnection::connection_for(connection_id);
@@ -51,27 +51,27 @@ void ConnectionFromClient::disconnect(int connection_id)
         dbgln("Database connection has disappeared");
 }
 
-Messages::SQLServer::PrepareStatementResponse ConnectionFromClient::prepare_statement(int connection_id, DeprecatedString const& sql)
+Messages::SQLServer::PrepareStatementResponse ConnectionFromClient::prepare_statement(u64 connection_id, DeprecatedString const& sql)
 {
     dbgln_if(SQLSERVER_DEBUG, "ConnectionFromClient::prepare_statement(connection_id: {}, sql: '{}')", connection_id, sql);
 
     auto database_connection = DatabaseConnection::connection_for(connection_id);
     if (!database_connection) {
         dbgln("Database connection has disappeared");
-        return { -1 };
+        return { {} };
     }
 
     auto result = database_connection->prepare_statement(sql);
     if (result.is_error()) {
         dbgln_if(SQLSERVER_DEBUG, "Could not parse SQL statement: {}", result.error().error_string());
-        return { -1 };
+        return { {} };
     }
 
     dbgln_if(SQLSERVER_DEBUG, "ConnectionFromClient::prepare_statement -> statement_id = {}", result.value());
     return { result.value() };
 }
 
-void ConnectionFromClient::execute_statement(int statement_id, Vector<SQL::Value> const& placeholder_values)
+void ConnectionFromClient::execute_statement(u64 statement_id, Vector<SQL::Value> const& placeholder_values)
 {
     dbgln_if(SQLSERVER_DEBUG, "ConnectionFromClient::execute_query_statement(statement_id: {})", statement_id);
     auto statement = SQLStatement::statement_for(statement_id);
@@ -80,7 +80,7 @@ void ConnectionFromClient::execute_statement(int statement_id, Vector<SQL::Value
         statement->execute(move(const_cast<Vector<SQL::Value>&>(placeholder_values)));
     } else {
         dbgln_if(SQLSERVER_DEBUG, "Statement has disappeared");
-        async_execution_error(statement_id, (int)SQL::SQLErrorCode::StatementUnavailable, DeprecatedString::formatted("{}", statement_id));
+        async_execution_error(statement_id, SQL::SQLErrorCode::StatementUnavailable, DeprecatedString::formatted("{}", statement_id));
     }
 }
 

Userland/Services/SQLServer/ConnectionFromClient.h

@@ -29,9 +29,9 @@ private:
     explicit ConnectionFromClient(NonnullOwnPtr<Core::Stream::LocalSocket>, int client_id);
 
     virtual Messages::SQLServer::ConnectResponse connect(DeprecatedString const&) override;
-    virtual Messages::SQLServer::PrepareStatementResponse prepare_statement(int, DeprecatedString const&) override;
+    virtual Messages::SQLServer::PrepareStatementResponse prepare_statement(u64, DeprecatedString const&) override;
-    virtual void execute_statement(int, Vector<SQL::Value> const& placeholder_values) override;
+    virtual void execute_statement(u64, Vector<SQL::Value> const& placeholder_values) override;
-    virtual void disconnect(int) override;
+    virtual void disconnect(u64) override;
 };
 
 }

Userland/Services/SQLServer/DatabaseConnection.cpp

@@ -11,9 +11,10 @@
 
 namespace SQLServer {
 
-static HashMap<int, NonnullRefPtr<DatabaseConnection>> s_connections;
+static HashMap<u64, NonnullRefPtr<DatabaseConnection>> s_connections;
+static u64 s_next_connection_id = 0;
 
-RefPtr<DatabaseConnection> DatabaseConnection::connection_for(int connection_id)
+RefPtr<DatabaseConnection> DatabaseConnection::connection_for(u64 connection_id)
 {
     if (s_connections.contains(connection_id))
         return *s_connections.get(connection_id).value();
@@ -21,8 +22,6 @@ RefPtr<DatabaseConnection> DatabaseConnection::connection_for(int connection_id)
     return nullptr;
 }
 
-static int s_next_connection_id = 0;
-
 DatabaseConnection::DatabaseConnection(DeprecatedString database_name, int client_id)
     : Object()
     , m_database_name(move(database_name))
@@ -31,17 +30,18 @@ DatabaseConnection::DatabaseConnection(DeprecatedString database_name, int clien
 {
     if (LexicalPath path(m_database_name); (path.title() != m_database_name) || (path.dirname() != ".")) {
         auto client_connection = ConnectionFromClient::client_connection_for(m_client_id);
-        client_connection->async_connection_error(m_connection_id, (int)SQL::SQLErrorCode::InvalidDatabaseName, m_database_name);
+        client_connection->async_connection_error(m_connection_id, SQL::SQLErrorCode::InvalidDatabaseName, m_database_name);
         return;
     }
 
     dbgln_if(SQLSERVER_DEBUG, "DatabaseConnection {} initiating connection with database '{}'", connection_id(), m_database_name);
     s_connections.set(m_connection_id, *this);
+
     deferred_invoke([this]() {
         m_database = SQL::Database::construct(DeprecatedString::formatted("/home/anon/sql/{}.db", m_database_name));
         auto client_connection = ConnectionFromClient::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, to_underlying(maybe_error.error().error()), maybe_error.error().error_string());
+            client_connection->async_connection_error(m_connection_id, maybe_error.error().error(), maybe_error.error().error_string());
             return;
         }
         m_accept_statements = true;
@@ -67,7 +67,7 @@ void DatabaseConnection::disconnect()
     });
 }
 
-SQL::ResultOr<int> DatabaseConnection::prepare_statement(StringView sql)
+SQL::ResultOr<u64> DatabaseConnection::prepare_statement(StringView sql)
 {
     dbgln_if(SQLSERVER_DEBUG, "DatabaseConnection::prepare_statement(connection_id {}, database '{}', sql '{}'", connection_id(), m_database_name, sql);
 

Userland/Services/SQLServer/DatabaseConnection.h

@@ -19,20 +19,20 @@ class DatabaseConnection final : public Core::Object {
 public:
     ~DatabaseConnection() override = default;
 
-    static RefPtr<DatabaseConnection> connection_for(int connection_id);
+    static RefPtr<DatabaseConnection> connection_for(u64 connection_id);
-    int connection_id() const { return m_connection_id; }
+    u64 connection_id() const { return m_connection_id; }
     int client_id() const { return m_client_id; }
     RefPtr<SQL::Database> database() { return m_database; }
     void disconnect();
-    SQL::ResultOr<int> prepare_statement(StringView sql);
+    SQL::ResultOr<u64> prepare_statement(StringView sql);
 
 private:
     DatabaseConnection(DeprecatedString database_name, int client_id);
 
     RefPtr<SQL::Database> m_database { nullptr };
     DeprecatedString m_database_name;
-    int m_connection_id;
+    u64 m_connection_id { 0 };
-    int m_client_id;
+    int m_client_id { 0 };
     bool m_accept_statements { false };
 };
 

Userland/Services/SQLServer/SQLClient.ipc

@@ -1,10 +1,12 @@
+#include <LibSQL/Result.h>
+
 endpoint SQLClient
 {
-    connected(int connection_id, DeprecatedString connected_to_database) =|
+    connected(u64 connection_id, DeprecatedString connected_to_database) =|
-    connection_error(int connection_id, int code, DeprecatedString message) =|
+    connection_error(u64 connection_id, SQL::SQLErrorCode code, DeprecatedString message) =|
-    execution_success(int statement_id, bool has_results, int created, int updated, int deleted) =|
+    execution_success(u64 statement_id, bool has_results, size_t created, size_t updated, size_t deleted) =|
-    next_result(int statement_id, Vector<DeprecatedString> row) =|
+    next_result(u64 statement_id, Vector<DeprecatedString> row) =|
-    results_exhausted(int statement_id, int total_rows) =|
+    results_exhausted(u64 statement_id, size_t total_rows) =|
-    execution_error(int statement_id, int code, DeprecatedString message) =|
+    execution_error(u64 statement_id, SQL::SQLErrorCode code, DeprecatedString message) =|
-    disconnected(int connection_id) =|
+    disconnected(u64 connection_id) =|
 }

Userland/Services/SQLServer/SQLServer.ipc

@@ -2,8 +2,8 @@
 
 endpoint SQLServer
 {
-    connect(DeprecatedString name) => (int connection_id)
+    connect(DeprecatedString name) => (u64 connection_id)
-    prepare_statement(int connection_id, DeprecatedString statement) => (int statement_id)
+    prepare_statement(u64 connection_id, DeprecatedString statement) => (Optional<u64> statement_id)
-    execute_statement(int statement_id, Vector<SQL::Value> placeholder_values) =|
+    execute_statement(u64 statement_id, Vector<SQL::Value> placeholder_values) =|
-    disconnect(int connection_id) =|
+    disconnect(u64 connection_id) =|
 }

Userland/Services/SQLServer/SQLStatement.cpp

@@ -12,9 +12,10 @@
 
 namespace SQLServer {
 
-static HashMap<int, NonnullRefPtr<SQLStatement>> s_statements;
+static HashMap<u64, NonnullRefPtr<SQLStatement>> s_statements;
+static u64 s_next_statement_id = 0;
 
-RefPtr<SQLStatement> SQLStatement::statement_for(int statement_id)
+RefPtr<SQLStatement> SQLStatement::statement_for(u64 statement_id)
 {
     if (s_statements.contains(statement_id))
         return *s_statements.get(statement_id).value();
@@ -22,8 +23,6 @@ RefPtr<SQLStatement> SQLStatement::statement_for(int statement_id)
     return nullptr;
 }
 
-static int s_next_statement_id = 0;
-
 SQL::ResultOr<NonnullRefPtr<SQLStatement>> SQLStatement::create(DatabaseConnection& connection, StringView sql)
 {
     auto parser = SQL::AST::Parser(SQL::AST::Lexer(sql));
@@ -54,7 +53,7 @@ void SQLStatement::report_error(SQL::Result result)
     remove_from_parent();
 
     if (client_connection)
-        client_connection->async_execution_error(statement_id(), (int)result.error(), result.error_string());
+        client_connection->async_execution_error(statement_id(), result.error(), result.error_string());
     else
         warnln("Cannot return execution error. Client disconnected");
 
@@ -129,7 +128,7 @@ void SQLStatement::next()
             next();
         });
     } else {
-        client_connection->async_results_exhausted(statement_id(), (int)m_index);
+        client_connection->async_results_exhausted(statement_id(), m_index);
     }
 }
 

Userland/Services/SQLServer/SQLStatement.h

@@ -25,8 +25,8 @@ public:
     static SQL::ResultOr<NonnullRefPtr<SQLStatement>> create(DatabaseConnection&, StringView sql);
     ~SQLStatement() override = default;
 
-    static RefPtr<SQLStatement> statement_for(int statement_id);
+    static RefPtr<SQLStatement> statement_for(u64 statement_id);
-    int statement_id() const { return m_statement_id; }
+    u64 statement_id() const { return m_statement_id; }
     DatabaseConnection* connection() { return dynamic_cast<DatabaseConnection*>(parent()); }
     void execute(Vector<SQL::Value> placeholder_values);
 
@@ -37,7 +37,7 @@ private:
     void next();
     void report_error(SQL::Result);
 
-    int m_statement_id;
+    u64 m_statement_id { 0 };
     size_t m_index { 0 };
     NonnullRefPtr<SQL::AST::Statement> m_statement;
     Optional<SQL::ResultSet> m_result {};

Userland/Utilities/sql.cpp

@@ -76,7 +76,7 @@ public:
 
         m_sql_client = SQL::SQLClient::try_create().release_value_but_fixme_should_propagate_errors();
 
-        m_sql_client->on_connected = [this](int connection_id, DeprecatedString const& connected_to_database) {
+        m_sql_client->on_connected = [this](auto connection_id, auto const& connected_to_database) {
             outln("Connected to \033[33;1m{}\033[0m", connected_to_database);
             m_current_database = connected_to_database;
             m_pending_database = "";
@@ -84,7 +84,7 @@ public:
             read_sql();
         };
 
-        m_sql_client->on_execution_success = [this](int, bool has_results, int updated, int created, int deleted) {
+        m_sql_client->on_execution_success = [this](auto, auto has_results, auto updated, auto created, auto deleted) {
             if (updated != 0 || created != 0 || deleted != 0) {
                 outln("{} row(s) updated, {} created, {} deleted", updated, created, deleted);
             }
@@ -93,35 +93,35 @@ public:
             }
         };
 
-        m_sql_client->on_next_result = [](int, Vector<DeprecatedString> const& row) {
+        m_sql_client->on_next_result = [](auto, auto const& row) {
             StringBuilder builder;
             builder.join(", "sv, row);
             outln("{}", builder.build());
         };
 
-        m_sql_client->on_results_exhausted = [this](int, int total_rows) {
+        m_sql_client->on_results_exhausted = [this](auto, auto total_rows) {
             outln("{} row(s)", total_rows);
             read_sql();
         };
 
-        m_sql_client->on_connection_error = [this](int, int code, DeprecatedString const& message) {
+        m_sql_client->on_connection_error = [this](auto, auto code, auto const& message) {
             outln("\033[33;1mConnection error:\033[0m {}", message);
-            m_loop.quit(code);
+            m_loop.quit(to_underlying(code));
         };
 
-        m_sql_client->on_execution_error = [this](int, int, DeprecatedString const& message) {
+        m_sql_client->on_execution_error = [this](auto, auto, auto const& message) {
             outln("\033[33;1mExecution error:\033[0m {}", message);
             read_sql();
         };
 
-        m_sql_client->on_disconnected = [this](int) {
+        m_sql_client->on_disconnected = [this](auto) {
             if (m_pending_database.is_empty()) {
                 outln("Disconnected from \033[33;1m{}\033[0m and terminating", m_current_database);
                 m_loop.quit(0);
             } else {
                 outln("Disconnected from \033[33;1m{}\033[0m", m_current_database);
                 m_current_database = "";
-                m_sql_client->connect(m_pending_database);
+                m_sql_client->async_connect(m_pending_database);
             }
         };
 
@@ -137,7 +137,7 @@ public:
     void connect(DeprecatedString const& database_name)
     {
         if (m_current_database.is_empty()) {
-            m_sql_client->connect(database_name);
+            m_sql_client->async_connect(database_name);
         } else {
             m_pending_database = database_name;
             m_sql_client->async_disconnect(m_connection_id);
@@ -169,7 +169,7 @@ private:
     DeprecatedString m_pending_database {};
     DeprecatedString m_current_database {};
     AK::RefPtr<SQL::SQLClient> m_sql_client { nullptr };
-    int m_connection_id { 0 };
+    u64 m_connection_id { 0 };
     Core::EventLoop m_loop;
     OwnPtr<Core::Stream::BufferedFile> m_input_file { nullptr };
     bool m_quit_when_files_read { false };
@@ -290,9 +290,8 @@ private:
                 m_loop.deferred_invoke([this]() {
                     read_sql();
                 });
-        } else {
+        } else if (auto statement_id = m_sql_client->prepare_statement(m_connection_id, piece); statement_id.has_value()) {
-            auto statement_id = m_sql_client->prepare_statement(m_connection_id, piece);
+            m_sql_client->async_execute_statement(*statement_id, {});
-            m_sql_client->async_execute_statement(statement_id, {});
         }
 
         // ...But m_keep_running can also be set to false by a command handler.