From 85d297995d76bd3cb3852e8771113863d6549ef6 Mon Sep 17 00:00:00 2001 From: sascha Date: Thu, 7 May 2026 09:27:21 +0200 Subject: [PATCH] insert query builder insert step improvements --- .../matador/query/insert_query_builder.hpp | 217 ++---------------- include/matador/query/insert_step.hpp | 157 +++++++++++++ include/matador/query/session.hpp | 42 +--- include/matador/sql/connection_pool.hpp | 2 + source/orm/CMakeLists.txt | 9 +- source/orm/query/insert_query_builder.cpp | 8 +- source/orm/sql/connection_pool.cpp | 6 +- 7 files changed, 200 insertions(+), 241 deletions(-) create mode 100644 include/matador/query/insert_step.hpp diff --git a/include/matador/query/insert_query_builder.hpp b/include/matador/query/insert_query_builder.hpp index 0587217..3080256 100644 --- a/include/matador/query/insert_query_builder.hpp +++ b/include/matador/query/insert_query_builder.hpp @@ -7,11 +7,11 @@ #include "matador/query/basic_schema.hpp" #include "matador/query/intermediates/executable_query.hpp" +#include "matador/query/insert_step.hpp" #include "matador/query/query.hpp" #include "matador/query/query_contexts.hpp" #include "matador/query/query_builder_exception.hpp" -#include "matador/sql/execute_result.hpp" #include "matador/sql/statement.hpp" #include "matador/utils/primary_key_accessor.hpp" @@ -56,157 +56,6 @@ private: std::string join_column_; }; -struct insert_step { - sql::query_context ctx; - - utils::result acquire_and_bind(sql::statement &stmt) const; - - // Session uses these to handle manual/sequence/table pre-insert PKs - utils::generator_type pk_generator{utils::generator_type::Manual}; - utils::primary_key_accessor pk_accessor; - - // Identity post-insert - std::function apply_returning{}; - std::function apply_primary_key{}; - std::function bind_object{}; - std::function make_object_persistent{}; -}; - -class insert_step2 { -public: - explicit insert_step2(sql::query_context ctx) - : ctx_(std::move(ctx)) {} - virtual ~insert_step2() = default; - - virtual utils::result prepare(sql::executor &conn) const = 0; - virtual utils::result insert(sql::statement &stmt) const = 0; - -protected: - utils::primary_key_accessor pk_accessor_; - sql::query_context ctx_; -}; - -template -class insert_step_pk_generated : public insert_step2 { -public: - insert_step_pk_generated(sql::query_context ctx, const object::object_ptr& ptr, abstract_pk_generator& pk_generator) - : insert_step2(std::move(ctx)) - , ptr_(ptr) - , pk_generator_(pk_generator){} - - utils::result prepare(sql::executor& conn) const override { - auto result = pk_generator_.next_id(conn); - if (!result.is_ok()) { - return utils::failure(result.err()); - } - pk_accessor_.set(*ptr_, *result); - - return utils::ok(); - } - - utils::result insert(sql::statement& stmt) const override { - stmt.bind(*ptr_); - - if (const auto exec_result = stmt.execute(); !exec_result.is_ok()) { - return utils::failure(exec_result.err()); - } - - ptr_.change_state(object::object_state::Persistent); - return utils::ok(); - } - -private: - object::object_ptr ptr_; - abstract_pk_generator& pk_generator_; -}; - -template -class insert_step_pk_identity : public insert_step2 { -public: - insert_step_pk_identity(sql::query_context ctx, const object::object_ptr& ptr, const std::string &pk_column_name) - : insert_step2(std::move(ctx)) - , ptr_(ptr) - , pk_column_name_(pk_column_name){} - - utils::result prepare(sql::executor&) const override { - return utils::ok(); - } - - utils::result insert(sql::statement& stmt) const override { - stmt.bind(*ptr_); - - auto result = stmt.fetch_one(); - if (!result.is_ok()) { - return utils::failure(result.err()); - } - if (!result.value().has_value()) { - return utils::failure(utils::error(error_code::FailedToFindObject, "Failed to insert object and retrieve identity.")); - } - - auto rec = result->value(); - const auto& f = rec.at(pk_column_name_); - utils::identifier id; - if (auto res = id.assign(f.value()); !res.is_ok()) { - return utils::failure(res.err()); - } - pk_accessor_.set(*ptr_, id); - - ptr_.change_state(object::object_state::Persistent); - return utils::ok(); - } - -private: - object::object_ptr ptr_; - std::string pk_column_name_; -}; - -template -class insert_step_pk_manual : public insert_step2 { -public: - insert_step_pk_manual(sql::query_context ctx, const object::object_ptr& ptr) - : insert_step2(std::move(ctx)) - , ptr_(ptr) {} - - utils::result prepare(sql::executor &) const override { - return utils::ok(); - } - utils::result insert(sql::statement &stmt) const override { - stmt.bind(*ptr_); - if (const auto exec_result = stmt.execute(); !exec_result.is_ok()) { - return utils::failure(exec_result.err()); - } - - ptr_.change_state(object::object_state::Persistent); - return utils::ok(); - } - -private: - object::object_ptr ptr_; -}; - -template -class insert_step_relation : public insert_step2 { -public: - insert_step_relation(sql::query_context ctx, const object::object_ptr& ptr) - : insert_step2(std::move(ctx)) - , ptr_(ptr) {} - - utils::result prepare(sql::executor &) const override { - return utils::ok(); - } - utils::result insert(sql::statement &stmt) const override { - stmt.bind(*ptr_); - if (const auto exec_result = stmt.execute(); !exec_result.is_ok()) { - return utils::failure(exec_result.err()); - } - - return utils::ok(); - } - -private: - object::object_ptr ptr_; -}; - template class insert_query_builder { public: @@ -215,7 +64,7 @@ public: , contexts_by_type_{contexts_by_type} {} - utils::result, utils::error> build(const object::object_ptr &ptr) { + utils::result>, utils::error> build(const object::object_ptr &ptr) { if (const auto it = schema_.find(typeid(ObjectType)); it == schema_.end()) { return utils::failure(utils::error{error_code::UnknownType, "Unknown type for insert query"}); } @@ -236,7 +85,7 @@ public: } relation_steps_.clear(); - return utils::ok(steps_); + return utils::ok(std::move(steps_)); } template < class PrimaryKeyType > @@ -303,7 +152,7 @@ public: } std::ignore = processing_many_to_many_relations_.insert(id); - std::vector rel_steps; + std::vector> insert_relation_steps; for (auto &obj : objects) { if (!obj) { continue; @@ -318,15 +167,9 @@ public: access::process(*this, *rel); - insert_step step{}; - step.pk_generator = utils::generator_type::None; - step.ctx = cit->second.insert; - step.bind_object = [rel](sql::statement &stmt) { stmt.bind(*rel); }; - step.make_object_persistent = [] {}; - - rel_steps.push_back(std::move(step)); + insert_relation_steps.push_back(std::make_unique>(cit->second.insert, rel)); } - relation_steps_.insert(relation_steps_.end(), rel_steps.begin(), rel_steps.end()); + relation_steps_.insert(relation_steps_.end(), std::make_move_iterator(insert_relation_steps.begin()), std::make_move_iterator(insert_relation_steps.end())); processing_many_to_many_relations_.erase(id); } @@ -360,7 +203,7 @@ public: } std::ignore = processing_many_to_many_relations_.insert(id); - std::vector rel_steps; + std::vector> insert_relation_steps; for (auto &obj : objects) { if (!obj) { continue; @@ -375,15 +218,9 @@ public: access::process(*this, *rel); - insert_step step{}; - step.pk_generator = utils::generator_type::None; - step.ctx = cit->second.insert; - step.bind_object = [rel](sql::statement &stmt) { stmt.bind(*rel); }; - step.make_object_persistent = [] {}; - - rel_steps.push_back(std::move(step)); + insert_relation_steps.push_back(std::make_unique>(cit->second.insert, rel)); } - relation_steps_.insert(relation_steps_.end(), rel_steps.begin(), rel_steps.end()); + relation_steps_.insert(relation_steps_.end(), std::make_move_iterator(insert_relation_steps.begin()), std::make_move_iterator(insert_relation_steps.end())); processing_many_to_many_relations_.erase(id); } @@ -403,7 +240,7 @@ private: }; template - utils::result build_for(const object::object_ptr &ptr, std::vector &steps) { + utils::result build_for(const object::object_ptr &ptr, std::vector> &steps) { if (!ptr) { return utils::failure(utils::error{error_code::InvalidObject, "Object is null"}); } @@ -428,7 +265,6 @@ private: // 2) Build INSERT for this object const auto &info = it->second.node().info(); - insert_step step{}; if (!info.has_primary_key() || it->second.pk_generator().type() == utils::generator_type::None) { return utils::failure(utils::error{error_code::MissingPrimaryKey, "Type " + info.name() + " has no primary key"}); } @@ -438,34 +274,12 @@ private: } if (it->second.pk_generator().type() == utils::generator_type::Manual) { - steps2_.push_back(std::make_unique>(cit->second.insert, ptr)); + steps.push_back(std::make_unique>(cit->second.insert, ptr)); } else if (it->second.pk_generator().type() == utils::generator_type::Identity) { - steps2_.push_back(std::make_unique>(cit->second.insert, ptr, info.primary_key_attribute()->name())); + steps.push_back(std::make_unique>(cit->second.insert, ptr, info.primary_key_attribute()->name())); } else { - steps2_.push_back(std::make_unique>(cit->second.insert, ptr)); + steps.push_back(std::make_unique>(cit->second.insert, ptr, it->second.pk_generator())); } - step.pk_generator = it->second.pk_generator().type(); - - step.ctx = cit->second.insert; - step.bind_object = [ptr](sql::statement &stmt) { - stmt.bind(*ptr); - }; - if (info.has_primary_key() && step.pk_generator == utils::generator_type::Identity) { - const auto pk_name = info.primary_key_attribute()->name(); - const table_column pk_col(&it->second.table(), pk_name); - step.apply_returning = [ptr, &step, pk_name = pk_name](const sql::record &rec) { - const auto& f = rec.at(pk_name); - utils::identifier id; - id.assign(f.value()); - step.pk_accessor.set(*ptr, id); - }; - } else if (info.has_primary_key() && (step.pk_generator == utils::generator_type::Sequence || step.pk_generator == utils::generator_type::Table)) { - step.apply_primary_key = [ptr, &step](const utils::identifier &id) { - step.pk_accessor.set(*ptr, id); - }; - } - step.make_object_persistent = [ptr] { ptr.change_state(object::object_state::Persistent); }; - steps.push_back(std::move(step)); return utils::ok(); } @@ -490,9 +304,8 @@ private: object::object_ptr ptr_; - std::vector> steps2_; - std::vector steps_; - std::vector relation_steps_; + std::vector> steps_; + std::vector> relation_steps_; std::unordered_set, visit_key_hash> visited_; std::unordered_set processing_many_to_many_relations_; }; diff --git a/include/matador/query/insert_step.hpp b/include/matador/query/insert_step.hpp new file mode 100644 index 0000000..4c92f6b --- /dev/null +++ b/include/matador/query/insert_step.hpp @@ -0,0 +1,157 @@ +#ifndef MATADOR_INSERT_STEP_HPP +#define MATADOR_INSERT_STEP_HPP + +#include + +#include "matador/utils/primary_key_accessor.hpp" +#include "matador/utils/error.hpp" +#include "matador/utils/result.hpp" + +#include "matador/object/object_ptr.hpp" + +#include "matador/sql/query_context.hpp" +#include "matador/sql/executor.hpp" +#include "matador/sql/execute_result.hpp" +#include "matador/sql/statement.hpp" + +#include "matador/query/error_code.hpp" +#include "matador/query/abstract_pk_generator.hpp" + +namespace matador::query { +class insert_step { +public: + explicit insert_step(sql::query_context ctx) + : ctx_(std::move(ctx)) {} + virtual ~insert_step() = default; + + virtual utils::result prepare(sql::executor &conn) = 0; + virtual utils::result insert(sql::statement &stmt) = 0; + + [[nodiscard]] const sql::query_context& ctx() const { return ctx_; } +protected: + utils::primary_key_accessor pk_accessor_; + sql::query_context ctx_; +}; + +template +class insert_step_pk_generated : public insert_step { +public: + insert_step_pk_generated(sql::query_context ctx, const object::object_ptr& ptr, abstract_pk_generator& pk_generator) + : insert_step(std::move(ctx)) + , ptr_(ptr) + , pk_generator_(pk_generator){} + + utils::result prepare(sql::executor& conn) override { + auto result = pk_generator_.next_id(conn); + if (!result.is_ok()) { + return utils::failure(result.err()); + } + pk_accessor_.set(*ptr_, utils::identifier{*result}); + + return utils::ok(); + } + + utils::result insert(sql::statement& stmt) override { + stmt.bind(*ptr_); + + if (const auto exec_result = stmt.execute(); !exec_result.is_ok()) { + return utils::failure(exec_result.err()); + } + + ptr_.change_state(object::object_state::Persistent); + return utils::ok(); + } + +private: + object::object_ptr ptr_; + abstract_pk_generator& pk_generator_; +}; + +template +class insert_step_pk_identity : public insert_step { +public: + insert_step_pk_identity(sql::query_context ctx, const object::object_ptr& ptr, std::string pk_column_name) + : insert_step(std::move(ctx)) + , ptr_(ptr) + , pk_column_name_(std::move(pk_column_name)){} + + utils::result prepare(sql::executor&) override { + return utils::ok(); + } + + utils::result insert(sql::statement& stmt) override { + stmt.bind(*ptr_); + + auto result = stmt.fetch_one(); + if (!result.is_ok()) { + return utils::failure(result.err()); + } + if (!result.value().has_value()) { + return utils::failure(utils::error(error_code::FailedToFindObject, "Failed to insert object and retrieve identity.")); + } + + auto rec = result->value(); + const auto& f = rec.at(pk_column_name_); + utils::identifier id; + if (auto res = id.assign(f.value()); !res.is_ok()) { + return utils::failure(res.err()); + } + pk_accessor_.set(*ptr_, id); + + ptr_.change_state(object::object_state::Persistent); + return utils::ok(); + } + +private: + object::object_ptr ptr_; + std::string pk_column_name_; +}; + +template +class insert_step_pk_manual : public insert_step { +public: + insert_step_pk_manual(sql::query_context ctx, const object::object_ptr& ptr) + : insert_step(std::move(ctx)) + , ptr_(ptr) {} + + utils::result prepare(sql::executor &) override { + return utils::ok(); + } + utils::result insert(sql::statement &stmt) override { + stmt.bind(*ptr_); + if (const auto exec_result = stmt.execute(); !exec_result.is_ok()) { + return utils::failure(exec_result.err()); + } + + ptr_.change_state(object::object_state::Persistent); + return utils::ok(); + } + +private: + object::object_ptr ptr_; +}; + +template +class insert_step_relation : public insert_step { +public: + insert_step_relation(sql::query_context ctx, const object::object_ptr& ptr) + : insert_step(std::move(ctx)) + , ptr_(ptr) {} + + utils::result prepare(sql::executor &) override { + return utils::ok(); + } + utils::result insert(sql::statement &stmt) override { + stmt.bind(*ptr_); + if (const auto exec_result = stmt.execute(); !exec_result.is_ok()) { + return utils::failure(exec_result.err()); + } + + return utils::ok(); + } + +private: + object::object_ptr ptr_; +}; +} +#endif //MATADOR_INSERT_STEP_HPP diff --git a/include/matador/query/session.hpp b/include/matador/query/session.hpp index b629d30..5f8a36c 100644 --- a/include/matador/query/session.hpp +++ b/include/matador/query/session.hpp @@ -81,8 +81,7 @@ utils::result, utils::error> session::insert(object::ob return utils::ok(obj); } - const auto it = schema_.find(typeid(Type)); - if (it == schema_.end()) { + if (const auto it = schema_.find(typeid(Type)); it == schema_.end()) { return utils::failure(make_error(error_code::UnknownType, "Failed to determine requested type.")); } @@ -94,42 +93,25 @@ utils::result, utils::error> session::insert(object::ob } // Execute all steps; for Identity steps read RETURNING and write pk back into the object - for (insert_step &step : *steps) { - if (step.pk_generator == utils::generator_type::Sequence || step.pk_generator == utils::generator_type::Table) { - const auto conn = pool_.acquire(); - if (!conn.valid()) { - return utils::failure(make_error(error_code::FailedToAcquirePool, "Failed to acquire connection pool for primary key generation.")); - } - auto result = it->second.pk_generator().next_id(*conn); - if (!result.is_ok()) { - return utils::failure(result.err()); - } - step.apply_primary_key(utils::identifier{*result}); + for (auto &step : *steps) { + const auto conn = pool_.acquire(); + if (!conn.valid()) { + return utils::failure(make_error(error_code::FailedToAcquirePool, "Failed to acquire connection pool for primary key generation.")); } - auto stmt = cache_.acquire(step.ctx); + if (const auto result = step->prepare(*conn); !result.is_ok()) { + return utils::failure(result.err()); + } + conn.release(); + + auto stmt = cache_.acquire(step->ctx()); if (!stmt.is_ok()) { return utils::failure(stmt.err()); } - if (auto result = step.acquire_and_bind(*stmt); !result.is_ok()) { + if (const auto result = step->insert(*stmt); !result.is_ok()) { return utils::failure(result.err()); } - - if (step.pk_generator == utils::generator_type::Identity) { - // insert and read RETURNING - auto record = stmt->fetch_one(); - if (!record.is_ok()) { - return utils::failure(record.err()); - } - if (!record.value().has_value()) { - return utils::failure(make_error(error_code::FailedToFindObject, "Failed to insert object and retrieve identity.")); - } - step.apply_returning(*record.value()); - } else if (const auto exec_result = stmt->execute(); !exec_result.is_ok()) { - return utils::failure(exec_result.err()); - } - step.make_object_persistent(); } obj.change_state(object::object_state::Persistent); diff --git a/include/matador/sql/connection_pool.hpp b/include/matador/sql/connection_pool.hpp index 92cd4dd..439b509 100644 --- a/include/matador/sql/connection_pool.hpp +++ b/include/matador/sql/connection_pool.hpp @@ -36,6 +36,8 @@ public: [[nodiscard]] std::optional id() const; [[nodiscard]] bool valid() const; + void release() const; + private: friend class connection_pool; diff --git a/source/orm/CMakeLists.txt b/source/orm/CMakeLists.txt index 92b7186..e3230c7 100644 --- a/source/orm/CMakeLists.txt +++ b/source/orm/CMakeLists.txt @@ -1,5 +1,4 @@ add_library(matador-orm STATIC - ../../include/matador/query/session.hpp ../../include/matador/query/abstract_pk_generator.hpp ../../include/matador/query/attribute_string_writer.hpp ../../include/matador/query/basic_schema.hpp @@ -29,8 +28,10 @@ add_library(matador-orm STATIC ../../include/matador/query/expression_evaluator.hpp ../../include/matador/query/fk_value_extractor.hpp ../../include/matador/query/generator.hpp + ../../include/matador/query/identity_pk_generator.hpp ../../include/matador/query/insert_query_builder.hpp ../../include/matador/query/insert_query_builder.hpp + ../../include/matador/query/insert_step.hpp ../../include/matador/query/intermediates/executable_query.hpp ../../include/matador/query/intermediates/fetchable_query.hpp ../../include/matador/query/intermediates/query_alter_intermediate.hpp @@ -81,6 +82,7 @@ add_library(matador-orm STATIC ../../include/matador/query/schema.hpp ../../include/matador/query/select_query_builder.hpp ../../include/matador/query/sequence_pk_generator.hpp + ../../include/matador/query/session.hpp ../../include/matador/query/table.hpp ../../include/matador/query/table_column.hpp ../../include/matador/query/table_constraint.hpp @@ -119,7 +121,6 @@ add_library(matador-orm STATIC ../../include/matador/sql/sql_functions.hpp ../../include/matador/sql/statement.hpp ../../include/matador/sql/statement_cache.hpp - query/session.cpp query/abstract_pk_generator.cpp query/attribute_string_writer.cpp query/basic_schema.cpp @@ -143,6 +144,7 @@ add_library(matador-orm STATIC query/expression/value_expression.cpp query/expression_evaluator.cpp query/generator.cpp + query/identity_pk_generator.cpp query/insert_query_builder.cpp query/intermediates/executable_query.cpp query/intermediates/fetchable_query.cpp @@ -188,6 +190,7 @@ add_library(matador-orm STATIC query/schema.cpp query/select_query_builder.cpp query/sequence_pk_generator.cpp + query/session.cpp query/table.cpp query/table_column.cpp query/table_constraint.cpp @@ -218,8 +221,6 @@ add_library(matador-orm STATIC sql/resolver_service.cpp sql/statement.cpp sql/statement_cache.cpp - ../../include/matador/query/identity_pk_generator.hpp - query/identity_pk_generator.cpp ) target_include_directories(matador-orm diff --git a/source/orm/query/insert_query_builder.cpp b/source/orm/query/insert_query_builder.cpp index bf9cd0f..75fc211 100644 --- a/source/orm/query/insert_query_builder.cpp +++ b/source/orm/query/insert_query_builder.cpp @@ -3,9 +3,9 @@ #include "matador/sql/statement_cache.hpp" namespace matador::query { -utils::result insert_step::acquire_and_bind(sql::statement &stmt) const { - bind_object(stmt); +// utils::result insert_step::acquire_and_bind(sql::statement &stmt) const { + // bind_object(stmt); - return utils::ok(); -} + // return utils::ok(); +// } } // namespace matador::query \ No newline at end of file diff --git a/source/orm/sql/connection_pool.cpp b/source/orm/sql/connection_pool.cpp index b94dbc4..80cfc4c 100644 --- a/source/orm/sql/connection_pool.cpp +++ b/source/orm/sql/connection_pool.cpp @@ -9,7 +9,7 @@ identifiable_connection::identifiable_connection(const size_t id, connection con , conn(std::move(conn)){} connection_ptr::~connection_ptr() { - pool_->release(connection_); + release(); } connection_ptr::connection_ptr(identifiable_connection* c, connection_pool* pool) @@ -53,6 +53,10 @@ bool connection_ptr::valid() const { return connection_ != nullptr; } +void connection_ptr::release() const { + pool_->release(connection_); +} + connection_pool::connection_pool(const std::string& dns, const size_t count) : connection_pool(dns, count, [](const connection_info &info) { return connection{info}; }) {}