Merge pull request #1025 from joto/refactor-tests-pg-code

lonvia · web-flow · commit 35a266ce3a6d · 2019-12-14T22:23:59.000+01:00
Refactor tests pg code
diff --git a/src/middle-pgsql.cpp b/src/middle-pgsql.cpp
@@ -345,16 +345,16 @@ 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 res = exec_prepared("get_node_list", buffer.c_str());
-    auto countPG = PQntuples(res.get());
+    auto const res = exec_prepared("get_node_list", buffer.c_str());
+    auto const countPG = res.num_tuples();
 
     std::unordered_map<osmid_t, osmium::Location> locs;
     for (int i = 0; i < countPG; ++i) {
         locs.emplace(
-            strtoosmid(PQgetvalue(res.get(), i, 0), nullptr, 10),
+            strtoosmid(res.get_value(i, 0), nullptr, 10),
             osmium::Location(
-                (int)strtol(PQgetvalue(res.get(), i, 2), nullptr, 10),
-                (int)strtol(PQgetvalue(res.get(), i, 1), nullptr, 10)));
+                (int)strtol(res.get_value(i, 2), nullptr, 10),
+                (int)strtol(res.get_value(i, 1), nullptr, 10)));
     }
 
     for (auto &n : *nodes) {
@@ -408,17 +408,17 @@ void middle_pgsql_t::node_changed(osmid_t osm_id)
 
     //keep track of whatever ways and rels these nodes intersect
     auto res = exec_prepared("mark_ways_by_node", osm_id);
-    for (int i = 0; i < PQntuples(res.get()); ++i) {
+    for (int i = 0; i < res.num_tuples(); ++i) {
         char *end;
-        osmid_t marked = strtoosmid(PQgetvalue(res.get(), i, 0), &end, 10);
+        osmid_t marked = strtoosmid(res.get_value(i, 0), &end, 10);
         ways_pending_tracker->mark(marked);
     }
 
     //do the rels too
     res = exec_prepared("mark_rels_by_node", osm_id);
-    for (int i = 0; i < PQntuples(res.get()); ++i) {
+    for (int i = 0; i < res.num_tuples(); ++i) {
         char *end;
-        osmid_t marked = strtoosmid(PQgetvalue(res.get(), i, 0), &end, 10);
+        osmid_t marked = strtoosmid(res.get_value(i, 0), &end, 10);
         rels_pending_tracker->mark(marked);
     }
 }
@@ -444,18 +444,18 @@ void middle_pgsql_t::ways_set(osmium::Way const &way)
 bool middle_query_pgsql_t::ways_get(osmid_t id,
                                     osmium::memory::Buffer &buffer) const
 {
-    auto res = exec_prepared("get_way", id);
+    auto const res = exec_prepared("get_way", id);
 
-    if (PQntuples(res.get()) != 1) {
+    if (res.num_tuples() != 1) {
         return false;
     }
 
     {
         osmium::builder::WayBuilder builder(buffer);
         builder.set_id(id);
 
-        pgsql_parse_nodes(PQgetvalue(res.get(), 0, 0), buffer, builder);
-        pgsql_parse_tags(PQgetvalue(res.get(), 0, 1), buffer, builder);
+        pgsql_parse_nodes(res.get_value(0, 0), buffer, builder);
+        pgsql_parse_tags(res.get_value(0, 1), buffer, builder);
     }
 
     buffer.commit();
@@ -486,14 +486,14 @@ middle_query_pgsql_t::rel_way_members_get(osmium::Relation const &rel,
     tmp2[tmp2.length() - 1] = '}';
 
     // Make sures all ways have been written back.
-    auto res = exec_prepared("get_way_list", tmp2.c_str());
-    int countPG = PQntuples(res.get());
+    auto const res = exec_prepared("get_way_list", tmp2.c_str());
+    int const countPG = res.num_tuples();
 
     idlist_t wayidspg;
 
     for (int i = 0; i < countPG; i++) {
         wayidspg.push_back(
-            strtoosmid(PQgetvalue(res.get(), i, 0), nullptr, 10));
+            strtoosmid(res.get_value(i, 0), nullptr, 10));
     }
 
     // Match the list of ways coming from postgres in a different order
@@ -509,9 +509,9 @@ middle_query_pgsql_t::rel_way_members_get(osmium::Relation const &rel,
                     osmium::builder::WayBuilder builder(buffer);
                     builder.set_id(m.ref());
 
-                    pgsql_parse_nodes(PQgetvalue(res.get(), j, 1), buffer,
+                    pgsql_parse_nodes(res.get_value(j, 1), buffer,
                                       builder);
-                    pgsql_parse_tags(PQgetvalue(res.get(), j, 2), buffer,
+                    pgsql_parse_tags(res.get_value(j, 2), buffer,
                                      builder);
                 }
 
@@ -551,10 +551,10 @@ void middle_pgsql_t::iterate_ways(middle_t::pending_processor &pf)
 void middle_pgsql_t::way_changed(osmid_t osm_id)
 {
     //keep track of whatever rels this way intersects
-    auto res = exec_prepared("mark_rels_by_way", osm_id);
-    for (int i = 0; i < PQntuples(res.get()); ++i) {
+    auto const res = exec_prepared("mark_rels_by_way", osm_id);
+    for (int i = 0; i < res.num_tuples(); ++i) {
         char *end;
-        osmid_t marked = strtoosmid(PQgetvalue(res.get(), i, 0), &end, 10);
+        osmid_t marked = strtoosmid(res.get_value(i, 0), &end, 10);
         rels_pending_tracker->mark(marked);
     }
 }
@@ -605,19 +605,19 @@ void middle_pgsql_t::relations_set(osmium::Relation const &rel)
 bool middle_query_pgsql_t::relations_get(osmid_t id,
                                          osmium::memory::Buffer &buffer) const
 {
-    auto res = exec_prepared("get_rel", id);
+    auto const res = exec_prepared("get_rel", id);
     // Fields are: members, tags, member_count */
     //
-    if (PQntuples(res.get()) != 1) {
+    if (res.num_tuples() != 1) {
         return false;
     }
 
     {
         osmium::builder::RelationBuilder builder(buffer);
         builder.set_id(id);
 
-        pgsql_parse_members(PQgetvalue(res.get(), 0, 0), buffer, builder);
-        pgsql_parse_tags(PQgetvalue(res.get(), 0, 1), buffer, builder);
+        pgsql_parse_members(res.get_value(0, 0), buffer, builder);
+        pgsql_parse_tags(res.get_value(0, 1), buffer, builder);
     }
 
     buffer.commit();
@@ -629,9 +629,9 @@ void middle_pgsql_t::relations_delete(osmid_t osm_id)
 {
     //keep track of whatever ways this relation interesects
     auto const res = exec_prepared("mark_ways_by_rel", osm_id);
-    for (int i = 0; i < PQntuples(res.get()); ++i) {
+    for (int i = 0; i < res.num_tuples(); ++i) {
         char *end;
-        osmid_t marked = strtoosmid(PQgetvalue(res.get(), i, 0), &end, 10);
+        osmid_t marked = strtoosmid(res.get_value(i, 0), &end, 10);
         ways_pending_tracker->mark(marked);
     }
 
@@ -658,22 +658,22 @@ 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 res = exec_prepared("mark_rels", osm_id);
-    for (int i = 0; i < PQntuples(res.get()); ++i) {
+    auto const res = exec_prepared("mark_rels", osm_id);
+    for (int i = 0; i < res.num_tuples(); ++i) {
         char *end;
-        osmid_t marked = strtoosmid(PQgetvalue(res.get(), i, 0), &end, 10);
+        osmid_t marked = strtoosmid(res.get_value(i, 0), &end, 10);
         rels_pending_tracker->mark(marked);
     }
 }
 
 idlist_t middle_query_pgsql_t::relations_using_way(osmid_t way_id) const
 {
-    auto result = exec_prepared("rels_using_way", way_id);
-    const int ntuples = PQntuples(result.get());
+    auto const result = exec_prepared("rels_using_way", way_id);
+    int const ntuples = result.num_tuples();
     idlist_t rel_ids;
     rel_ids.resize((size_t)ntuples);
     for (int i = 0; i < ntuples; ++i) {
-        rel_ids[i] = strtoosmid(PQgetvalue(result.get(), i, 0), nullptr, 10);
+        rel_ids[i] = strtoosmid(result.get_value(i, 0), nullptr, 10);
     }
 
     return rel_ids;
diff --git a/src/pgsql.cpp b/src/pgsql.cpp
@@ -4,8 +4,8 @@
 #include <cstdarg>
 #include <cstdio>
 
-pg_conn_t::pg_conn_t(std::string const &connection)
-: m_conn(PQconnectdb(connection.c_str()))
+pg_conn_t::pg_conn_t(std::string const &conninfo)
+: m_conn(PQconnectdb(conninfo.c_str()))
 {
     if (PQstatus(m_conn.get()) != CONNECTION_OK) {
         fprintf(stderr, "Connection to database failed: %s\n", error_msg());
diff --git a/src/pgsql.hpp b/src/pgsql.hpp
@@ -5,40 +5,93 @@
 
 #include <libpq-fe.h>
 
+#include <cassert>
 #include <memory>
 #include <string>
 
-struct pg_result_deleter_t
+/**
+ * PostgreSQL query result.
+ *
+ * Wraps the PGresult object of the libpq library.
+ */
+class pg_result_t
 {
-    void operator()(PGresult *p) const noexcept { PQclear(p); }
-};
+public:
+    explicit pg_result_t(PGresult *result) noexcept : m_result(result) {}
+
+    /// Get a pointer to the underlying PGresult object.
+    PGresult *get() const noexcept { return m_result.get(); }
+
+    /// Get the status of this result.
+    ExecStatusType status() const noexcept
+    {
+        return PQresultStatus(m_result.get());
+    }
+
+    /// The number of fields (columns) in this result.
+    int num_fields() const noexcept { return PQnfields(m_result.get()); }
+
+    /// The number of tuples (rows) in this result.
+    int num_tuples() const noexcept { return PQntuples(m_result.get()); }
+
+    /// Does the field at (row, col) has the NULL value?
+    bool is_null(int row, int col) const noexcept
+    {
+        assert(row < num_tuples() && col < num_fields());
+        return PQgetisnull(m_result.get(), row, col) != 0;
+    }
+
+    /// The length of the field at (row, col) in bytes.
+    int get_length(int row, int col) const noexcept
+    {
+        assert(row < num_tuples() && col < num_fields());
+        return PQgetlength(m_result.get(), row, col);
+    }
+
+    /**
+     * Get value of the field at (row, col) as char pointer. The string is
+     * null-terminated. Only valid as long as the pg_result_t is in scope.
+     */
+    char const *get_value(int row, int col) const noexcept
+    {
+        assert(row < num_tuples() && col < num_fields());
+        return PQgetvalue(m_result.get(), row, col);
+    }
+
+    /**
+     * Create a std::string with the value of the field at (row, col). This
+     * does the correct thing for binary data.
+     */
+    std::string get_value_as_string(int row, int col) const noexcept
+    {
+        return std::string(get_value(row, col), get_length(row, col));
+    }
 
-using pg_result_t = std::unique_ptr<PGresult, pg_result_deleter_t>;
+private:
+    struct pg_result_deleter_t
+    {
+        void operator()(PGresult *p) const noexcept { PQclear(p); }
+    };
 
-struct pg_conn_deleter_t
-{
-    void operator()(PGconn *p) const noexcept { PQfinish(p); }
+    std::unique_ptr<PGresult, pg_result_deleter_t> m_result;
 };
 
-using pg_conn_wrapper_t = std::unique_ptr<PGconn, pg_conn_deleter_t>;
-
 /**
- * Simple postgres connection.
+ * PostgreSQL connection.
+ *
+ * Wraps the PGconn object of the libpq library.
  *
  * The connection is automatically closed when the object is destroyed.
  */
 class pg_conn_t
 {
 public:
-    pg_conn_t(std::string const &connection);
+    explicit pg_conn_t(std::string const &conninfo);
 
     pg_result_t exec_prepared(char const *stmt, int num_params,
                               const char *const *param_values,
                               ExecStatusType expect = PGRES_TUPLES_OK) const;
 
-    void copy_data(std::string const &sql, std::string const &context) const;
-    void end_copy(std::string const &context) const;
-
     pg_result_t query(ExecStatusType expect, char const *sql) const;
 
     pg_result_t query(ExecStatusType expect, std::string const &sql) const;
@@ -47,10 +100,19 @@ class pg_conn_t
 
     void exec(std::string const &sql) const;
 
+    void copy_data(std::string const &sql, std::string const &context) const;
+
+    void end_copy(std::string const &context) const;
+
     char const *error_msg() const noexcept;
 
 private:
-    pg_conn_wrapper_t m_conn;
+    struct pg_conn_deleter_t
+    {
+        void operator()(PGconn *p) const noexcept { PQfinish(p); }
+    };
+
+    std::unique_ptr<PGconn, pg_conn_deleter_t> m_conn;
 };
 
 #endif // OSM2PGSQL_PGSQL_HPP
diff --git a/src/table.hpp b/src/table.hpp
@@ -45,22 +45,16 @@ class table_t
         const char *get_next()
         {
             if (m_current < m_count) {
-                return PQgetvalue(m_result.get(), m_current++, 0);
+                return m_result.get_value(m_current++, 0);
             }
             return nullptr;
         }
 
         int get_count() const { return m_count; }
-        void reset()
-        {
-            //NOTE: PQgetvalue doc doesn't say if you can call it
-            //      multiple times with the same row col
-            m_current = 0;
-        }
 
     private:
         wkb_reader(pg_result_t &&result)
-        : m_result(std::move(result)), m_count(PQntuples(m_result.get())),
+        : m_result(std::move(result)), m_count(m_result.num_tuples()),
           m_current(0)
         {}
 
diff --git a/tests/common-pg.hpp b/tests/common-pg.hpp
