Merge pull request #1213 from joto/pgsql-related-cleanups

Pgsql related cleanups

Author: lonvia

src/flex-table.cpp

@@ -75,18 +75,16 @@ flex_table_column_t &flex_table_t::add_column(std::string const &name,
 
 std::string flex_table_t::build_sql_prepare_get_wkb() const
 {
-    if (has_geom_column()) {
-        if (has_multicolumn_id_index()) {
-            return "PREPARE get_wkb(char(1), bigint) AS"
-                   " SELECT \"{}\" FROM {} WHERE \"{}\" = $1 AND \"{}\" = $2"_format(
-                       geom_column().name(), full_name(), m_columns[0].name(),
-                       m_columns[1].name());
-        }
-        return "PREPARE get_wkb(bigint) AS"
-               " SELECT \"{}\" FROM {} WHERE \"{}\" = $1"_format(
-                   geom_column().name(), full_name(), id_column_names());
+    if (has_multicolumn_id_index()) {
+        return "PREPARE get_wkb(char(1), bigint) AS"
+               " SELECT \"{}\" FROM {} WHERE \"{}\" = $1 AND \"{}\" = $2"_format(
+                   geom_column().name(), full_name(), m_columns[0].name(),
+                   m_columns[1].name());
     }
-    return "PREPARE get_wkb(bigint) AS SELECT ''";
+
+    return "PREPARE get_wkb(bigint) AS"
+           " SELECT \"{}\" FROM {} WHERE \"{}\" = $1"_format(
+               geom_column().name(), full_name(), id_column_names());
 }
 
 std::string flex_table_t::build_sql_create_table(table_type ttype,
@@ -225,14 +223,10 @@ void table_connection_t::stop(bool updateable, bool append)
                 table().geom_column().name());
         }
 
-        auto const res = m_db_connection->query(
-            PGRES_TUPLES_OK,
-            "SELECT regexp_split_to_table(postgis_lib_version(), '\\.')");
-        auto const postgis_major = std::stoi(res.get_value_as_string(0, 0));
-        auto const postgis_minor = std::stoi(res.get_value_as_string(1, 0));
+        auto const postgis_version = get_postgis_version(*m_db_connection);
 
         sql += " ORDER BY ";
-        if (postgis_major == 2 && postgis_minor < 4) {
+        if (postgis_version.major == 2 && postgis_version.minor < 4) {
             fmt::print(stderr, "Using GeoHash for clustering\n");
             if (table().srid() == 4326) {
                 sql += "ST_GeoHash({},10)"_format(table().geom_column().name());
@@ -302,6 +296,14 @@ void table_connection_t::stop(bool updateable, bool append)
     teardown();
 }
 
+void table_connection_t::prepare()
+{
+    assert(m_db_connection);
+    if (table().has_id_column() && table().has_geom_column()) {
+        m_db_connection->exec(table().build_sql_prepare_get_wkb());
+    }
+}
+
 pg_result_t table_connection_t::get_geom_by_id(osmium::item_type type,
                                                osmid_t id) const
 {

src/flex-table.hpp

@@ -213,13 +213,7 @@ class table_connection_t
 
     void teardown() { m_db_connection.reset(); }
 
-    void prepare()
-    {
-        assert(m_db_connection);
-        if (table().has_id_column()) {
-            m_db_connection->exec(table().build_sql_prepare_get_wkb());
-        }
-    }
+    void prepare();
 
     void create_id_index()
     {

src/pgsql.cpp

@@ -174,3 +174,12 @@ std::string qualified_name(std::string const &schema, std::string const &name)
 
     return result;
 }
+
+postgis_version get_postgis_version(pg_conn_t const &db_connection) {
+    auto const res = db_connection.query(
+            PGRES_TUPLES_OK,
+            "SELECT regexp_split_to_table(postgis_lib_version(), '\\.')");
+
+    return {std::stoi(res.get_value_as_string(0, 0)),
+            std::stoi(res.get_value_as_string(1, 0))};
+}

src/pgsql.hpp

@@ -1,7 +1,13 @@
 #ifndef OSM2PGSQL_PGSQL_HPP
 #define OSM2PGSQL_PGSQL_HPP
 
-/* Helper functions for PostgreSQL access */
+/**
+ * \file
+ *
+ * This file is part of osm2pgsql (https://github.com/openstreetmap/osm2pgsql).
+ *
+ * Helper classes and functions for PostgreSQL access.
+ */
 
 #include "osmtypes.hpp"
 
@@ -154,4 +160,13 @@ std::string tablespace_clause(std::string const &name);
  */
 std::string qualified_name(std::string const &schema, std::string const &name);
 
+struct postgis_version
+{
+    int major;
+    int minor;
+};
+
+/// Get PostGIS major and minor version.
+postgis_version get_postgis_version(pg_conn_t const &db_connection);
+
 #endif // OSM2PGSQL_PGSQL_HPP

src/table.cpp

@@ -198,14 +198,10 @@ void table_t::stop(bool updateable, bool enable_hstore_index,
             sql += " WHERE ST_IsValid(way)";
         }
 
-        auto res = m_sql_conn->query(
-            PGRES_TUPLES_OK,
-            "SELECT regexp_split_to_table(postgis_lib_version(), '\\.')");
-        auto const postgis_major = std::stoi(res.get_value_as_string(0, 0));
-        auto const postgis_minor = std::stoi(res.get_value_as_string(1, 0));
+        auto const postgis_version = get_postgis_version(*m_sql_conn);
 
         sql += " ORDER BY ";
-        if (postgis_major == 2 && postgis_minor < 4) {
+        if (postgis_version.major == 2 && postgis_version.minor < 4) {
             fmt::print(stderr, "Using GeoHash for clustering\n");
             if (m_srid == "4326") {
                 sql += "ST_GeoHash(way,10)";

tests/test-pgsql.cpp

@@ -1,7 +1,10 @@
 #include <catch.hpp>
 
+#include "common-import.hpp"
 #include "pgsql.hpp"
 
+static testing::db::import_t db;
+
 TEST_CASE("Tablespace clause with no tablespace")
 {
     REQUIRE(tablespace_clause("") == "");
@@ -21,3 +24,10 @@ TEST_CASE("Table name with schema")
 {
     REQUIRE(qualified_name("osm", "foo") == R"("osm"."foo")");
 }
+
+TEST_CASE("PostGIS version")
+{
+    auto conn = db.db().connect();
+    auto const postgis_version = get_postgis_version(conn);
+    REQUIRE(postgis_version.major >= 2);
+}