From 72019bc1e7526c1f6d73507d111e56182db5eb7d Mon Sep 17 00:00:00 2001 From: sascha Date: Thu, 21 May 2026 12:04:10 +0200 Subject: [PATCH] fixed insert_step_processor --- backends/postgres/src/postgres_error.cpp | 14 ++-- .../matador/query/insert_query_builder.hpp | 78 +++++++++++-------- test/backends/SessionInsertBelongsTo.cpp | 3 +- test/backends/SessionInsertHasMany.cpp | 12 ++- 4 files changed, 64 insertions(+), 43 deletions(-) diff --git a/backends/postgres/src/postgres_error.cpp b/backends/postgres/src/postgres_error.cpp index d4874af..a37eba5 100644 --- a/backends/postgres/src/postgres_error.cpp +++ b/backends/postgres/src/postgres_error.cpp @@ -6,7 +6,10 @@ namespace matador::backends::postgres { -utils::error make_error(const sql::error_code ec, const PGresult *res, const PGconn *db, const std::string &msg, +utils::error make_error(const sql::error_code ec, + const PGresult *res, + const PGconn *db, + const std::string &msg, const std::string &sql) { utils::error err(ec, msg); err.add_error_info("dbms", "postgres"); @@ -30,22 +33,19 @@ bool is_result_error(const PGresult *res) { return status != PGRES_TUPLES_OK && status != PGRES_COMMAND_OK; } -void throw_postgres_error(const char *what, const std::string &source) -{ +void throw_postgres_error(const char *what, const std::string &source) { std::stringstream msg; msg << "postgres error (" << source << "): " << what; throw std::logic_error(msg.str()); } -void throw_postgres_error(PGconn *db, const std::string &source) -{ +void throw_postgres_error(const PGconn *db, const std::string &source) { if (PQstatus(db) == CONNECTION_BAD) { throw_postgres_error(PQerrorMessage(db), source); } } -void throw_postgres_error(PGresult *res, PGconn *db, const std::string &source, const std::string &sql) -{ +void throw_postgres_error(const PGresult *res, const PGconn *db, const std::string &source, const std::string &sql) { if (res == nullptr) { std::stringstream msg; msg << "postgres error (" << source << ", " << PQerrorMessage(db) << ": " << sql; diff --git a/include/matador/query/insert_query_builder.hpp b/include/matador/query/insert_query_builder.hpp index 14f5882..0a7112b 100644 --- a/include/matador/query/insert_query_builder.hpp +++ b/include/matador/query/insert_query_builder.hpp @@ -64,7 +64,6 @@ static std::pair make_visit_key(const EntityType struct visit_key_hash { size_t operator()(const std::pair &p) const noexcept { - // combine hashes (simple + sufficient here) const size_t h1 = p.first.hash_code(); const size_t h2 = std::hash{}(p.second); return h1 ^ (h2 + 0x9e3779b97f4a7c15ULL + (h1 << 6) + (h1 >> 2)); @@ -117,7 +116,7 @@ public: : ctx_{ctx} {} - utils::result build(object::object_ptr ptr) { + utils::result build(object::object_ptr ptr, const bool as_relation_step = false) { if (!ptr) { return utils::failure(utils::error{error_code::InvalidObject, "Object is null"}); } @@ -137,12 +136,6 @@ public: // 1) Traverse relations first => dependencies will be inserted before this object try { access::process(*this, *ptr_); - - // relation inserts must run after all entity inserts were collected - for (auto &s : ctx_.relation_steps_) { - ctx_.steps_.push_back(std::move(s)); - } - ctx_.relation_steps_.clear(); } catch (const query_builder_exception &ex) { return utils::failure(ex.error()); } @@ -157,12 +150,11 @@ public: return utils::failure(utils::error{error_code::UnknownType, "Unknown type"}); } - if (it->second.pk_generator().type() == utils::generator_type::Manual) { - ctx_.steps_.push_back(std::make_unique>(cit->second.insert, ptr_)); - } else if (it->second.pk_generator().type() == utils::generator_type::Identity) { - ctx_.steps_.push_back(std::make_unique>(cit->second.insert, ptr_, info.primary_key_attribute()->name())); + auto step = create_insert_step(cit->second.insert, it->second); + if (as_relation_step) { + ctx_.relation_steps_.push_back(std::move(step)); } else { - ctx_.steps_.push_back(std::make_unique>(cit->second.insert, ptr_, it->second.pk_generator())); + ctx_.steps_.push_back(std::move(step)); } ptr_.reset(); @@ -199,17 +191,18 @@ public: } has_many_linker linker(ptr_, join_column); - insert_context ctx{ctx_.schema_, ctx_.contexts_by_type_}; - insert_step_processor processor{ctx}; + insert_step_processor processor{ctx_}; for (auto &obj : objects) { - if (!obj.is_transient()) { + if (!obj) { continue; } - const auto result = processor.build(obj); - if (!result) { - throw query_builder_exception(error_code::InvalidObject, "Invalid object"); - // return utils::failure(result.err()); + if (obj.is_transient()) { + const auto result = processor.build(obj, true); + if (!result) { + throw query_builder_exception(error_code::InvalidObject, "Invalid object"); + // return utils::failure(result.err()); + } } access::process(linker, *obj); @@ -268,7 +261,6 @@ public: objects, attr, [foreign_type, local_type](const char* relation_name) -> processing_many_to_many_key { - std::cout << "Processing many-to-many relation: " << local_type.name() << ":" << foreign_type.name() << ":" << relation_name << std::endl; return {std::string{relation_name}, local_type, foreign_type}; // return make_processing_many_to_many_key(relation_name); }, @@ -297,7 +289,6 @@ public: objects, attr, [foreign_type, local_type](const char* relation_name) -> processing_many_to_many_key { - std::cout << "Processing many-to-many relation: " << foreign_type.name() << ":" << local_type.name() << ":" << relation_name << std::endl; return {std::string{relation_name}, foreign_type, local_type}; return make_processing_many_to_many_key(relation_name); }, @@ -309,12 +300,11 @@ public: private: template void on_foreign_object(object::object_ptr &obj, const utils::foreign_attributes &attr) { - if (!utils::is_cascade_type_set(attr.cascade(), utils::cascade_type::Insert) || !obj || obj.is_transient()) { + if (!utils::is_cascade_type_set(attr.cascade(), utils::cascade_type::Insert) || !obj || !obj.is_transient()) { return; } - insert_context ctx{ctx_.schema_, ctx_.contexts_by_type_}; - insert_step_processor processor{ctx}; + insert_step_processor processor{ctx_}; const auto result = processor.build(obj); if (!result) { @@ -356,14 +346,16 @@ private: std::vector> insert_relation_steps; insert_step_processor processor(ctx_); for (auto &obj : objects) { - if (!obj || !obj.is_transient()) { + if (!obj) { continue; } - const auto result = processor.build(obj); - if (!result) { - throw query_builder_exception(error_code::InvalidObject, "Invalid object"); - // return utils::failure(result.err()); + if (obj.is_transient()) { + const auto result = processor.build(obj, true); + if (!result) { + throw query_builder_exception(error_code::InvalidObject, "Invalid object"); + // return utils::failure(result.err()); + } } auto rel = make_relation(obj); @@ -379,6 +371,16 @@ private: ctx_.processing_many_to_many_relations_.erase(key); } + std::unique_ptr create_insert_step(const sql::query_context& query_ctx, const schema_node& node) { + if (node.pk_generator().type() == utils::generator_type::Manual) { + return std::make_unique>(query_ctx, ptr_); + } + if (node.pk_generator().type() == utils::generator_type::Identity) { + return std::make_unique>(query_ctx, ptr_, node.node().info().primary_key_attribute()->name()); + } + return std::make_unique>(query_ctx, ptr_, node.pk_generator()); + } + private: insert_context& ctx_; object::object_ptr ptr_; @@ -388,15 +390,16 @@ template class insert_query_builder { public: explicit insert_query_builder(const basic_schema &schema, const std::unordered_map &contexts_by_type) - : context_{schema, contexts_by_type} + : schema_(schema) + , contexts_by_type_(contexts_by_type) {} utils::result>, utils::error> build(const object::object_ptr &ptr) { - if (const auto it = context_.schema_.find(typeid(ObjectType)); it == context_.schema_.end()) { + if (const auto it = schema_.find(typeid(ObjectType)); it == schema_.end()) { return utils::failure(utils::error{error_code::UnknownType, "Unknown type for insert query"}); } - insert_context ctx{context_.schema_, context_.contexts_by_type_}; + insert_context ctx{schema_, contexts_by_type_}; insert_step_processor processor{ctx}; const auto result = processor.build(ptr); @@ -404,11 +407,18 @@ public: return utils::failure(result.err()); } + // relation inserts must run after all entity inserts were collected + for (auto &s : ctx.relation_steps_) { + ctx.steps_.push_back(std::move(s)); + } + ctx.relation_steps_.clear(); + return utils::ok(std::move(ctx.steps_)); } private: - insert_context context_; + const basic_schema &schema_; + const std::unordered_map &contexts_by_type_; }; } #endif //MATADOR_INSERT_QUERY_BUILDER_HPP \ No newline at end of file diff --git a/test/backends/SessionInsertBelongsTo.cpp b/test/backends/SessionInsertBelongsTo.cpp index 577d641..e6e59ed 100644 --- a/test/backends/SessionInsertBelongsTo.cpp +++ b/test/backends/SessionInsertBelongsTo.cpp @@ -25,7 +25,8 @@ TEST_CASE_METHOD(SessionFixture, "Test insert object with belongs to relation wi const auto carrie = make_object("Carrie", s_king, 1974); REQUIRE(carrie.is_transient()); - REQUIRE(ses.insert(carrie).is_ok()); + const auto res = ses.insert(carrie); + REQUIRE(res.is_ok()); REQUIRE(carrie.is_persistent()); auto found_author = ses.find(s_king->id); diff --git a/test/backends/SessionInsertHasMany.cpp b/test/backends/SessionInsertHasMany.cpp index 1ed52f8..c9217b8 100644 --- a/test/backends/SessionInsertHasMany.cpp +++ b/test/backends/SessionInsertHasMany.cpp @@ -61,6 +61,16 @@ void validate_author_state(const object_ptr& ptr, object_state expec } } +namespace matador::utils { +template < typename ValueType > +std::ostream& operator<<(std::ostream& os, const result& value) { + if (value) { + return os; + } + return os << "Error: " << value.err(); +} +} + TEST_CASE_METHOD(SessionFixture, "Test insert object with has many relation", "[session][insert][has_many]") { const auto result = schema.attach("books") .and_then( [this] { return schema.attach("authors"); } ) @@ -79,7 +89,7 @@ TEST_CASE_METHOD(SessionFixture, "Test insert object with has many relation", "[ validate_author_state(s_king, object_state::Transient); auto res = ses.insert(s_king); - REQUIRE(res.is_ok()); + REQUIRE(res); validate_author_state(s_king, object_state::Persistent); }