Merge pull request #1158 from joto/refactor-exec-prepared

Refactor exec_prepared

Author: lonvia

src/flex-table.cpp

@@ -314,11 +314,10 @@ pg_result_t table_connection_t::get_geom_by_id(osmium::item_type type,
     assert(m_db_connection);
     std::string const id_str = fmt::to_string(id);
     if (table().has_multicolumn_id_index()) {
-        char const *param_values[] = {type_to_char(type), id_str.c_str()};
-        return m_db_connection->exec_prepared("get_wkb", 2, param_values);
+        return m_db_connection->exec_prepared(
+            "get_wkb", type_to_char(type), id_str.c_str());
     }
-    char const *param_values[] = {id_str.c_str()};
-    return m_db_connection->exec_prepared("get_wkb", 1, param_values);
+    return m_db_connection->exec_prepared("get_wkb", id_str);
 }
 
 void table_connection_t::delete_rows_with(osmium::item_type type, osmid_t id)

src/middle-pgsql.cpp

@@ -57,27 +57,6 @@ middle_pgsql_t::table_desc::table_desc(options_t const &options,
     m_copy_target->id = "id"; // XXX hardcoded column name
 }
 
-pg_result_t middle_query_pgsql_t::exec_prepared(char const *stmt,
-                                                char const *param) const
-{
-    return m_sql_conn.exec_prepared(stmt, 1, &param);
-}
-
-pg_result_t middle_query_pgsql_t::exec_prepared(char const *stmt,
-                                                osmid_t osm_id) const
-{
-    util::integer_to_buffer buffer{osm_id};
-    return exec_prepared(stmt, buffer.c_str());
-}
-
-pg_result_t middle_pgsql_t::exec_prepared(char const *stmt,
-                                          osmid_t osm_id) const
-{
-    util::integer_to_buffer buffer{osm_id};
-    char const *const bptr = buffer.c_str();
-    return m_db_connection.exec_prepared(stmt, 1, &bptr);
-}
-
 void middle_query_pgsql_t::exec_sql(std::string const &sql_cmd) const
 {
     m_sql_conn.exec(sql_cmd);
@@ -259,7 +238,7 @@ middle_query_pgsql_t::local_nodes_get_list(osmium::WayNodeList *nodes) const
     buffer[buffer.size() - 1] = '}';
 
     // Nodes must have been written back at this point.
-    auto const res = exec_prepared("get_node_list", buffer.c_str());
+    auto const res = m_sql_conn.exec_prepared("get_node_list", buffer);
     std::unordered_map<osmid_t, osmium::Location> locs;
     for (int i = 0; i < res.num_tuples(); ++i) {
         locs.emplace(
@@ -321,15 +300,15 @@ void middle_pgsql_t::node_changed(osmid_t osm_id)
     }
 
     // Find all ways referencing this node and mark them as pending.
-    auto res = exec_prepared("mark_ways_by_node", osm_id);
+    auto res = m_db_connection.exec_prepared("mark_ways_by_node", osm_id);
     for (int i = 0; i < res.num_tuples(); ++i) {
         osmid_t const marked = osmium::string_to_object_id(res.get_value(i, 0));
         way_changed(marked);
         m_ways_pending_tracker->mark(marked);
     }
 
     // Find all relations referencing this node and mark them as pending.
-    res = exec_prepared("mark_rels_by_node", osm_id);
+    res = m_db_connection.exec_prepared("mark_rels_by_node", osm_id);
     for (int i = 0; i < res.num_tuples(); ++i) {
         osmid_t const marked = osmium::string_to_object_id(res.get_value(i, 0));
         m_rels_pending_tracker->mark(marked);
@@ -357,7 +336,7 @@ void middle_pgsql_t::way_set(osmium::Way const &way)
 bool middle_query_pgsql_t::way_get(osmid_t id,
                                    osmium::memory::Buffer &buffer) const
 {
-    auto const res = exec_prepared("get_way", id);
+    auto const res = m_sql_conn.exec_prepared("get_way", id);
 
     if (res.num_tuples() != 1) {
         return false;
@@ -395,7 +374,7 @@ middle_query_pgsql_t::rel_way_members_get(osmium::Relation const &rel,
     // replace last , with } to complete list of ids
     id_list.back() = '}';
 
-    auto const res = exec_prepared("get_way_list", id_list.c_str());
+    auto const res = m_sql_conn.exec_prepared("get_way_list", id_list);
     idlist_t wayidspg;
     for (int i = 0; i < res.num_tuples(); ++i) {
         wayidspg.push_back(osmium::string_to_object_id(res.get_value(i, 0)));
@@ -459,7 +438,7 @@ void middle_pgsql_t::way_changed(osmid_t osm_id)
     }
 
     //keep track of whatever rels this way intersects
-    auto const res = exec_prepared("mark_rels_by_way", osm_id);
+    auto const res = m_db_connection.exec_prepared("mark_rels_by_way", osm_id);
     for (int i = 0; i < res.num_tuples(); ++i) {
         osmid_t const marked = osmium::string_to_object_id(res.get_value(i, 0));
         m_rels_pending_tracker->mark(marked);
@@ -512,7 +491,7 @@ void middle_pgsql_t::relation_set(osmium::Relation const &rel)
 bool middle_query_pgsql_t::relation_get(osmid_t id,
                                         osmium::memory::Buffer &buffer) const
 {
-    auto const res = exec_prepared("get_rel", id);
+    auto const res = m_sql_conn.exec_prepared("get_rel", id);
     // Fields are: members, tags, member_count */
     //
     if (res.num_tuples() != 1) {
@@ -536,7 +515,7 @@ void middle_pgsql_t::relation_delete(osmid_t osm_id)
 {
     assert(m_append);
     //keep track of whatever ways this relation interesects
-    auto const res = exec_prepared("mark_ways_by_rel", osm_id);
+    auto const res = m_db_connection.exec_prepared("mark_ways_by_rel", osm_id);
     for (int i = 0; i < res.num_tuples(); ++i) {
         osmid_t const marked = osmium::string_to_object_id(res.get_value(i, 0));
         m_ways_pending_tracker->mark(marked);
@@ -564,7 +543,7 @@ void middle_pgsql_t::relation_changed(osmid_t osm_id)
     //keep track of whatever ways and rels these nodes intersect
     //TODO: dont need to stop the copy above since we are only reading?
     //TODO: can we just mark the id without querying? the where clause seems intersect reltable.parts with the id
-    auto const res = exec_prepared("mark_rels", osm_id);
+    auto const res = m_db_connection.exec_prepared("mark_rels", osm_id);
     for (int i = 0; i < res.num_tuples(); ++i) {
         osmid_t const marked = osmium::string_to_object_id(res.get_value(i, 0));
         m_rels_pending_tracker->mark(marked);
@@ -573,7 +552,7 @@ void middle_pgsql_t::relation_changed(osmid_t osm_id)
 
 idlist_t middle_query_pgsql_t::relations_using_way(osmid_t way_id) const
 {
-    auto const result = exec_prepared("rels_using_way", way_id);
+    auto const result = m_sql_conn.exec_prepared("rels_using_way", way_id);
     int const ntuples = result.num_tuples();
     idlist_t rel_ids;
     rel_ids.resize((size_t)ntuples);

src/middle-pgsql.hpp

@@ -41,9 +41,6 @@ class middle_query_pgsql_t : public middle_query_t
 private:
     size_t local_nodes_get_list(osmium::WayNodeList *nodes) const;
 
-    pg_result_t exec_prepared(char const *stmt, char const *param) const;
-    pg_result_t exec_prepared(char const *stmt, osmid_t osm_id) const;
-
     pg_conn_t m_sql_conn;
     std::shared_ptr<node_ram_cache> m_cache;
     std::shared_ptr<node_persistent_cache> m_persistent_cache;
@@ -117,7 +114,6 @@ struct middle_pgsql_t : public slim_middle_t
     };
 
     void buffer_store_tags(osmium::OSMObject const &obj, bool attrs);
-    pg_result_t exec_prepared(char const *stmt, osmid_t osm_id) const;
 
     table_desc m_tables[NUM_TABLES];
 

src/pgsql.cpp

@@ -1,7 +1,9 @@
 /* Helper functions for the postgresql connections */
 #include "format.hpp"
 #include "pgsql.hpp"
+#include "util.hpp"
 
+#include <array>
 #include <cstdarg>
 #include <cstdio>
 
@@ -93,17 +95,16 @@ void pg_conn_t::end_copy(std::string const &context) const
     }
 }
 
-pg_result_t pg_conn_t::exec_prepared(char const *stmt, int num_params,
-                                     char const *const *param_values,
-                                     const ExecStatusType expect) const
+pg_result_t
+pg_conn_t::exec_prepared_internal(char const *stmt, int num_params,
+                                  char const *const *param_values) const
 {
 #ifdef DEBUG_PGSQL
     fmt::print(stderr, "ExecPrepared: {}\n", stmt);
 #endif
-    //run the prepared statement
     pg_result_t res{PQexecPrepared(m_conn.get(), stmt, num_params, param_values,
                                    nullptr, nullptr, 0)};
-    if (PQresultStatus(res.get()) != expect) {
+    if (PQresultStatus(res.get()) != PGRES_TUPLES_OK) {
         fmt::print(stderr, "Prepared statement failed with: {} ({})\n",
                    error_msg(), PQresultStatus(res.get()));
         fmt::print(stderr, "Query: {}\n", stmt);
@@ -120,6 +121,29 @@ pg_result_t pg_conn_t::exec_prepared(char const *stmt, int num_params,
     return res;
 }
 
+pg_result_t pg_conn_t::exec_prepared(char const *stmt, char const *p1, char const *p2) const
+{
+    std::array<const char *, 2> params{{p1, p2}};
+    return exec_prepared_internal(stmt, params.size(), params.data());
+}
+
+pg_result_t pg_conn_t::exec_prepared(char const *stmt, char const *param) const
+{
+    return exec_prepared_internal(stmt, 1, &param);
+}
+
+pg_result_t pg_conn_t::exec_prepared(char const *stmt,
+                                     std::string const &param) const
+{
+    return exec_prepared(stmt, param.c_str());
+}
+
+pg_result_t pg_conn_t::exec_prepared(char const *stmt, osmid_t id) const
+{
+    util::integer_to_buffer buffer{id};
+    return exec_prepared(stmt, buffer.c_str());
+}
+
 std::string tablespace_clause(std::string const &name)
 {
     std::string sql;

src/pgsql.hpp

@@ -3,6 +3,8 @@
 
 /* Helper functions for PostgreSQL access */
 
+#include "osmtypes.hpp"
+
 #include <libpq-fe.h>
 
 #include <cassert>
@@ -99,9 +101,17 @@ class pg_conn_t
 public:
     explicit pg_conn_t(std::string const &conninfo);
 
-    pg_result_t exec_prepared(char const *stmt, int num_params,
-                              char const *const *param_values,
-                              ExecStatusType expect = PGRES_TUPLES_OK) const;
+    /// Execute a prepared statement with one parameter.
+    pg_result_t exec_prepared(char const *stmt, char const *param) const;
+
+    /// Execute a prepared statement with two parameters.
+    pg_result_t exec_prepared(char const *stmt, char const *p1, char const *p2) const;
+
+    /// Execute a prepared statement with one string parameter.
+    pg_result_t exec_prepared(char const *stmt, std::string const &param) const;
+
+    /// Execute a prepared statement with one integer parameter.
+    pg_result_t exec_prepared(char const *stmt, osmid_t id) const;
 
     pg_result_t query(ExecStatusType expect, char const *sql) const;
 
@@ -121,6 +131,9 @@ class pg_conn_t
     void close() noexcept { m_conn.reset(); }
 
 private:
+    pg_result_t exec_prepared_internal(char const *stmt, int num_params,
+                                       char const *const *param_values) const;
+
     struct pg_conn_deleter_t
     {
         void operator()(PGconn *p) const noexcept { PQfinish(p); }

src/table.cpp

@@ -456,15 +456,8 @@ void table_t::escape_type(std::string const &value, ColumnType flags)
     }
 }
 
-table_t::wkb_reader table_t::get_wkb_reader(osmid_t const id)
+table_t::wkb_reader table_t::get_wkb_reader(osmid_t id)
 {
-    util::integer_to_buffer tmp{id};
-    char const *param_values[] = {tmp.c_str()};
-
-    // the prepared statement get_wkb will behave differently depending on the
-    // sql_conn
-    // each table has its own sql_connection with the get_way referring to the
-    // appropriate table
-    auto res = m_sql_conn->exec_prepared("get_wkb", 1, param_values);
+    auto res = m_sql_conn->exec_prepared("get_wkb", id);
     return wkb_reader{std::move(res)};
 }

src/table.hpp

@@ -59,7 +59,7 @@ class table_t
         int m_count;
         int m_current;
     };
-    wkb_reader get_wkb_reader(osmid_t const id);
+    wkb_reader get_wkb_reader(osmid_t id);
 
 protected:
     void connect();