From 336b2a7342eefd12e1c07f6b1e8385db0d9b3c9a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sascha=20K=C3=BChl?= <s.kuehl@tms-bonn.de>
Date: Thu, 13 Feb 2025 16:13:20 +0100
Subject: [PATCH] fixed backend tests

---
 backends/postgres/src/postgres_connection.cpp |  8 ++-
 backends/postgres/src/postgres_error.cpp      |  2 +-
 backends/postgres/test/CMakeLists.txt         |  2 +-
 include/matador/query/query_compiler.hpp      |  5 ++
 include/matador/query/query_data.hpp          | 18 +++--
 include/matador/utils/value.hpp               |  1 +
 source/core/object/attribute_definition.cpp   |  5 +-
 source/core/utils/value.cpp                   | 65 ++++++++++++++++++-
 .../query/intermediates/executable_query.cpp  |  3 +
 .../query/intermediates/fetchable_query.cpp   |  5 ++
 .../intermediates/query_into_intermediate.cpp | 12 ++--
 source/orm/query/query_compiler.cpp           | 29 +++++----
 source/orm/sql/connection.cpp                 |  2 +-
 test/backends/ConnectionTest.cpp              | 12 +++-
 test/backends/QueryFixture.cpp                | 14 ++--
 test/backends/QueryRecordTest.cpp             |  3 +-
 test/backends/StatementCacheTest.cpp          |  2 +-
 17 files changed, 142 insertions(+), 46 deletions(-)

diff --git a/backends/postgres/src/postgres_connection.cpp b/backends/postgres/src/postgres_connection.cpp
index eabf5fe..69cff5c 100644
--- a/backends/postgres/src/postgres_connection.cpp
+++ b/backends/postgres/src/postgres_connection.cpp
@@ -20,7 +20,7 @@ postgres_connection::postgres_connection(const sql::connection_info &info)
 }
 
 utils::result<void, utils::error> postgres_connection::open() {
-  if (is_open()) {
+  if (conn_ != nullptr) {
     return utils::ok<void>();
   }
 
@@ -221,7 +221,11 @@ utils::result<bool, utils::error> postgres_connection::exists(const std::string
     return utils::failure(make_error(sql::error_code::TABLE_EXISTS_FAILED, res, conn_, "Failed check if table exists", stmt));
   }
 
-  return utils::ok(utils::to<size_t>(PQcmdTuples(res)) == 1);
+  const auto result = utils::to<size_t>(PQcmdTuples(res));
+  if (!result) {
+    return utils::failure(make_error(sql::error_code::FAILURE, res, conn_, "Failed to convert result value", stmt));
+  }
+  return utils::ok(*result == 1);
 }
 
 std::string postgres_connection::to_escaped_string(const utils::blob& value) const
diff --git a/backends/postgres/src/postgres_error.cpp b/backends/postgres/src/postgres_error.cpp
index cb8dd1b..d4874af 100644
--- a/backends/postgres/src/postgres_error.cpp
+++ b/backends/postgres/src/postgres_error.cpp
@@ -27,7 +27,7 @@ bool is_result_error(const PGresult *res) {
     return true;
   }
   const auto status = PQresultStatus(res);
-  return status != PGRES_TUPLES_OK && status == PGRES_COMMAND_OK;
+  return status != PGRES_TUPLES_OK && status != PGRES_COMMAND_OK;
 }
 
 void throw_postgres_error(const char *what, const std::string &source)
diff --git a/backends/postgres/test/CMakeLists.txt b/backends/postgres/test/CMakeLists.txt
index 2cc6ac5..2edc12c 100644
--- a/backends/postgres/test/CMakeLists.txt
+++ b/backends/postgres/test/CMakeLists.txt
@@ -2,7 +2,7 @@ CPMAddPackage("gh:catchorg/Catch2@3.7.1")
 
 list(APPEND CMAKE_MODULE_PATH ${catch2_SOURCE_DIR}/extras)
 
-set(POSTGRES_CONNECTION_STRING "postgres://test:test123!@localhost:5432/matador")
+set(POSTGRES_CONNECTION_STRING "postgres://news:news@localhost:15432/matador")
 
 configure_file(Connection.hpp.in ${PROJECT_BINARY_DIR}/backends/postgres/test/connection.hpp @ONLY IMMEDIATE)
 
diff --git a/include/matador/query/query_compiler.hpp b/include/matador/query/query_compiler.hpp
index ecffc2a..5ee62d0 100644
--- a/include/matador/query/query_compiler.hpp
+++ b/include/matador/query/query_compiler.hpp
@@ -6,6 +6,8 @@
 
 #include "matador/sql/query_context.hpp"
 
+#include "matador/utils/placeholder.hpp"
+
 namespace matador::sql {
 class connection_impl;
 class dialect;
@@ -14,6 +16,7 @@ class dialect;
 namespace matador::query {
 
 struct query_data;
+struct value_visitor;
 
 class query_compiler final : public query_part_visitor
 {
@@ -52,6 +55,8 @@ protected:
 
   static std::string build_table_name(sql::dialect_token token, const sql::dialect &d, const sql::table& t);
 
+  std::string determine_value(value_visitor &visitor, const std::variant<utils::placeholder, utils::database_type> &val);
+
 protected:
   const query_data *data_{};
   sql::query_context query_;
diff --git a/include/matador/query/query_data.hpp b/include/matador/query/query_data.hpp
index 7ced617..f2409c3 100644
--- a/include/matador/query/query_data.hpp
+++ b/include/matador/query/query_data.hpp
@@ -5,17 +5,25 @@
 #include <vector>
 
 #include "matador/object/attribute_definition.hpp"
+
 #include "matador/sql/table.hpp"
 
 #include "matador/query/query_part.hpp"
 
 namespace matador::query {
-struct query_data
-{
-  std::vector<std::unique_ptr<query_part>> parts{};
-  std::vector<object::attribute_definition> columns{};
-  std::unordered_map<std::string, sql::table> tables{};
+
+enum struct query_mode {
+    Direct = 0,
+    Prepared = 1
 };
+
+struct query_data {
+    std::vector<std::unique_ptr<query_part>> parts{};
+    std::vector<object::attribute_definition> columns{};
+    std::unordered_map<std::string, sql::table> tables{};
+    query_mode mode = query_mode::Direct;
+};
+
 }
 
 #endif //QUERY_DATA_HPP
diff --git a/include/matador/utils/value.hpp b/include/matador/utils/value.hpp
index 0194bd3..dc0e185 100644
--- a/include/matador/utils/value.hpp
+++ b/include/matador/utils/value.hpp
@@ -68,6 +68,7 @@ public:
 
   [[nodiscard]] size_t size() const;
   [[nodiscard]] basic_type type() const;
+  void type(basic_type t);
 
   [[nodiscard]] bool is_integer() const;
   [[nodiscard]] bool is_floating_point() const;
diff --git a/source/core/object/attribute_definition.cpp b/source/core/object/attribute_definition.cpp
index 1442cbd..0f74f75 100644
--- a/source/core/object/attribute_definition.cpp
+++ b/source/core/object/attribute_definition.cpp
@@ -137,9 +137,8 @@ bool attribute_definition::is_null() const {
   return value_.is_null();
 }
 
-void attribute_definition::type(utils::basic_type /*type*/) {
-  // type_ = type;
-  //  utils::initialize_by_utils::basic_type(type, value_);
+void attribute_definition::type(const utils::basic_type type) {
+  value_.type(type);
 }
 
 std::string attribute_definition::str() const {
diff --git a/source/core/utils/value.cpp b/source/core/utils/value.cpp
index e5c1754..097ff09 100644
--- a/source/core/utils/value.cpp
+++ b/source/core/utils/value.cpp
@@ -5,6 +5,8 @@
 namespace matador::utils {
 namespace detail {
 
+void initialize_by_basic_type(basic_type type, database_type &val);
+
 size_t determine_size(const std::string &val)
 {
   return val.size();
@@ -56,11 +58,15 @@ size_t value::size() const
   return size_;
 }
 
-basic_type value::type() const
-{
+basic_type value::type() const {
   return type_;
 }
 
+void value::type(const basic_type t) {
+    type_ = t;
+    detail::initialize_by_basic_type(type_, value_);
+}
+
 bool value::is_integer() const
 {
   return type_ >= basic_type::type_int8 && type_ <= basic_type::type_uint64;
@@ -106,4 +112,59 @@ bool value::is_null() const
   return type_ == basic_type::type_null;
 }
 
+namespace detail {
+void initialize_by_basic_type(const basic_type type, database_type &val) {
+    switch (type) {
+        case basic_type::type_int8:
+            val.emplace<int8_t>();
+        break;
+        case basic_type::type_int16:
+            val.emplace<int16_t>();
+        break;
+        case basic_type::type_int32:
+            val.emplace<int32_t>();
+        break;
+        case basic_type::type_int64:
+            val.emplace<int64_t>();
+        break;
+        case basic_type::type_uint8:
+            val.emplace<uint8_t>();
+        break;
+        case basic_type::type_uint16:
+            val.emplace<uint16_t>();
+        break;
+        case basic_type::type_uint32:
+            val.emplace<uint32_t>();
+        break;
+        case basic_type::type_uint64:
+            val.emplace<uint64_t>();
+        break;
+        case basic_type::type_bool:
+            val.emplace<bool>();
+        break;
+        case basic_type::type_float:
+            val.emplace<float>();
+        break;
+        case basic_type::type_double:
+            val.emplace<double>();
+        break;
+        case basic_type::type_varchar:
+        case basic_type::type_text:
+          val.emplace<std::string>();
+        break;
+        // case basic_type::type_date:
+        //     val.emplace<date>();
+        // break;
+        // case basic_type::type_time:
+        //     val.emplace<time>();
+        // break;
+        case basic_type::type_blob:
+            val.emplace<utils::blob>();
+        break;
+        default:
+            val.emplace<nullptr_t>();
+    }
+
+}
+}
 }
\ No newline at end of file
diff --git a/source/orm/query/intermediates/executable_query.cpp b/source/orm/query/intermediates/executable_query.cpp
index 0c95470..5d7e592 100644
--- a/source/orm/query/intermediates/executable_query.cpp
+++ b/source/orm/query/intermediates/executable_query.cpp
@@ -9,18 +9,21 @@ namespace matador::query {
 utils::result<size_t, utils::error> executable_query::execute(const sql::executor &exec) const
 {
   query_compiler compiler;
+  context_->mode = query_mode::Direct;
   return exec.execute(compiler.compile(*context_, exec.dialect(), std::nullopt));
 }
 
 utils::result<sql::statement, utils::error> executable_query::prepare(const sql::executor &exec) const
 {
   query_compiler compiler;
+  context_->mode = query_mode::Prepared;
   return exec.prepare(compiler.compile(*context_, exec.dialect(), std::nullopt));
 }
 
 std::string executable_query::str(const sql::executor &exec) const
 {
   query_compiler compiler;
+  context_->mode = query_mode::Direct;
   return exec.str(compiler.compile(*context_, exec.dialect(), std::nullopt));
 }
 
diff --git a/source/orm/query/intermediates/fetchable_query.cpp b/source/orm/query/intermediates/fetchable_query.cpp
index aeb48cb..3170234 100644
--- a/source/orm/query/intermediates/fetchable_query.cpp
+++ b/source/orm/query/intermediates/fetchable_query.cpp
@@ -8,6 +8,7 @@ namespace matador::query {
 utils::result<sql::query_result<sql::record>, utils::error> fetchable_query::fetch_all(const sql::executor &exec) const
 {
   query_compiler compiler;
+  context_->mode = query_mode::Direct;
   return exec.fetch(compiler.compile(*context_, exec.dialect(), std::nullopt))
     .and_then([](auto &&res) {
       return utils::ok(sql::query_result<sql::record>(std::forward<decltype(res)>(res)));
@@ -22,6 +23,7 @@ utils::result<sql::query_result<sql::record>, utils::error> fetchable_query::fet
 utils::result<std::optional<sql::record>, utils::error> fetchable_query::fetch_one(const sql::executor &exec) const
 {
   query_compiler compiler;
+  context_->mode = query_mode::Direct;
   auto result = exec.fetch(compiler.compile(*context_, exec.dialect(), std::nullopt));
   if (!result.is_ok()) {
     return utils::failure(result.err());
@@ -39,18 +41,21 @@ utils::result<std::optional<sql::record>, utils::error> fetchable_query::fetch_o
 std::string fetchable_query::str(const sql::executor &exec) const
 {
   query_compiler compiler;
+  context_->mode = query_mode::Direct;
   return exec.str(compiler.compile(*context_, exec.dialect(), std::nullopt));
 }
 
 utils::result<std::unique_ptr<sql::query_result_impl>, utils::error> fetchable_query::fetch(const sql::executor &exec) const
 {
   query_compiler compiler;
+  context_->mode = query_mode::Direct;
   return exec.fetch(compiler.compile(*context_, exec.dialect(), std::nullopt));
 }
 
 utils::result<sql::statement, utils::error> fetchable_query::prepare(const sql::executor &exec) const
 {
   query_compiler compiler;
+  context_->mode = query_mode::Prepared;
   return exec.prepare(compiler.compile(*context_, exec.dialect(), std::nullopt));
 }
 
diff --git a/source/orm/query/intermediates/query_into_intermediate.cpp b/source/orm/query/intermediates/query_into_intermediate.cpp
index 2e85e79..9ba962d 100644
--- a/source/orm/query/intermediates/query_into_intermediate.cpp
+++ b/source/orm/query/intermediates/query_into_intermediate.cpp
@@ -15,10 +15,12 @@ executable_query query_into_intermediate::values(std::vector<std::variant<utils:
 }
 
 executable_query query_into_intermediate::values( std::vector<utils::database_type>&& values ) {
-    std::vector<std::variant<utils::placeholder, utils::database_type>> transformed_values;
-    for (auto&& val : values) {
-        transformed_values.emplace_back(val);
-    }
-    return this->values(std::move(transformed_values));
+  std::vector<std::variant<utils::placeholder, utils::database_type>> transformed_values;
+  transformed_values.reserve(values.size());
+  for (auto&& val : values) {
+    transformed_values.emplace_back(val);
+  }
+  return this->values(std::move(transformed_values));
 }
+
 } // namespace matador::query
diff --git a/source/orm/query/query_compiler.cpp b/source/orm/query/query_compiler.cpp
index 70c7443..ba357d7 100644
--- a/source/orm/query/query_compiler.cpp
+++ b/source/orm/query/query_compiler.cpp
@@ -174,8 +174,16 @@ struct value_visitor {
   internal::basic_type_to_string_visitor value_to_string_visitor;
 };
 
-void query_compiler::visit(internal::query_values_part &values_part)
-{
+std::string query_compiler::determine_value(value_visitor &visitor, const std::variant<utils::placeholder, utils::database_type> &val) {
+    if (data_->mode == query_mode::Direct) {
+        std::visit(visitor, val);
+        return visitor.value_to_string_visitor.result;
+    }
+
+    query_.bind_vars.emplace_back(std::string("value_") + std::to_string(query_.bind_vars.size() + 1));
+    return dialect_->next_placeholder(query_.bind_vars);
+}
+void query_compiler::visit(internal::query_values_part &values_part) {
   query_.sql += " " + dialect_->token_at(sql::dialect_token::VALUES);
 
   attribute_string_writer writer(*dialect_, connection_);
@@ -184,19 +192,16 @@ void query_compiler::visit(internal::query_values_part &values_part)
   std::string result{"("};
   if (values_part.values().size() < 2) {
     for (const auto& val: values_part.values()) {
-      std::visit(visitor, val);
-      result.append(visitor.value_to_string_visitor.result);
+      result.append(determine_value(visitor, val));
     }
   } else {
     auto it = values_part.values().begin();
     auto val = *it++;
-    std::visit(visitor, val);
-    result.append(visitor.value_to_string_visitor.result);
+      result.append(determine_value(visitor, val));
     for (; it != values_part.values().end(); ++it) {
       result.append(", ");
       val = *it;
-      std::visit(visitor, val);
-      result.append(visitor.value_to_string_visitor.result);
+      result.append(determine_value(visitor, val));
     }
   }
   result += (")");
@@ -267,11 +272,11 @@ void query_compiler::visit(internal::query_create_table_part &create_table_part)
   if (!context.primary_keys.empty()) {
     result.append(", CONSTRAINT PK_" + create_table_part.table().name + " PRIMARY KEY (" + utils::join(context.primary_keys, ", ") + ")");
   }
-  for (const auto &fk: context.foreign_contexts) {
+  for (const auto &[column, reference_column]: context.foreign_contexts) {
     result += ", CONSTRAINT FK_" + create_table_part.table().name;
-    result += "_" + fk.column;
-    result += " FOREIGN KEY (" + fk.column + ")";
-    result += " REFERENCES " + fk.reference_column->table_name() + "(" + fk.reference_column->name() + ")";
+    result += "_" + column;
+    result += " FOREIGN KEY (" + column + ")";
+    result += " REFERENCES " + reference_column->table_name() + "(" + reference_column->name() + ")";
   }
 
   result += ")";
diff --git a/source/orm/sql/connection.cpp b/source/orm/sql/connection.cpp
index 5dd2386..5351fbb 100644
--- a/source/orm/sql/connection.cpp
+++ b/source/orm/sql/connection.cpp
@@ -73,7 +73,7 @@ utils::result<void, utils::error> connection::open() const
   if (res.is_error()) {
     return utils::failure(res.err());
   }
-  if (*res) {
+  if (!*res) {
     logger_->on_connect();
     return connection_->open();
   }
diff --git a/test/backends/ConnectionTest.cpp b/test/backends/ConnectionTest.cpp
index a0cf8ca..04d4d16 100644
--- a/test/backends/ConnectionTest.cpp
+++ b/test/backends/ConnectionTest.cpp
@@ -8,13 +8,19 @@ using namespace matador::sql;
 
 TEST_CASE("Create connection test", "[connection]") {
   const connection c(matador::test::connection::dns);
-  REQUIRE(!c.is_open());
+  auto res = c.is_open();
+  REQUIRE(res);
+  REQUIRE(!*res);
 
   auto result = c.open();
   REQUIRE(result.is_ok());
-  REQUIRE(c.is_open());
+  res = c.is_open();
+  REQUIRE(res);
+  REQUIRE(*res);
 
   result = c.close();
   REQUIRE(result.is_ok());
-  REQUIRE(!c.is_open());
+  res = c.is_open();
+  REQUIRE(res);
+  REQUIRE(!*res);
 }
diff --git a/test/backends/QueryFixture.cpp b/test/backends/QueryFixture.cpp
index 32f9b23..9b4ea7a 100644
--- a/test/backends/QueryFixture.cpp
+++ b/test/backends/QueryFixture.cpp
@@ -37,15 +37,13 @@ void QueryFixture::check_table_not_exists(const std::string &table_name) const
 }
 
 void QueryFixture::drop_table_if_exists(const std::string &table_name) const {
-  const auto result = db.exists(table_name).and_then([&table_name, this](bool exists) {
+  const auto result = db.exists(table_name).and_then([&table_name, this](const bool exists) {
     if (exists) {
-      if (query::query::drop()
-      .table(table_name)
-      .execute(db).is_ok()) {
-        this->check_table_not_exists(table_name);
-      } else {
-        FAIL("Failed to drop table");
-      }
+        auto res = query::query::drop()
+        .table(table_name)
+        .execute(db);
+      REQUIRE(res);
+      this->check_table_not_exists(table_name);
     }
     return utils::ok(true);
   });
diff --git a/test/backends/QueryRecordTest.cpp b/test/backends/QueryRecordTest.cpp
index c2f5b80..49543b8 100644
--- a/test/backends/QueryRecordTest.cpp
+++ b/test/backends/QueryRecordTest.cpp
@@ -476,8 +476,7 @@ TEST_CASE_METHOD(QueryFixture, "Execute select statement with group by and order
   }
 }
 
-TEST_CASE_METHOD(QueryFixture, "Execute delete statement", "[query][record]")
-{
+TEST_CASE_METHOD(QueryFixture, "Execute delete statement", "[query][record]") {
   auto res = query::create()
   .table("person", {
     make_pk_column<uint32_t>("id"),
diff --git a/test/backends/StatementCacheTest.cpp b/test/backends/StatementCacheTest.cpp
index 2959ac8..44f353b 100644
--- a/test/backends/StatementCacheTest.cpp
+++ b/test/backends/StatementCacheTest.cpp
@@ -20,7 +20,7 @@ public:
   ~StatementCacheFixture() = default;
 
 protected:
-  sql::connection_pool<matador::sql::connection> pool;
+  sql::connection_pool<sql::connection> pool;
   orm::session ses;
 };