Store db connection parameters in a their own class

The connection parameters (such as dbname, user, host, ...) are stored
in their original format and not concatenated into a conninfo string any
more. We are now using a different database connection function from
libpq that takes them in this form. This allows us to get rid of the
code that transforms those parameters into the conninfo string that
takes various forms of connection parameters into account, especially
around the URI form of the connection parameters. The libpq has this
code anyway, see also the last parameter "expand_dbname" of the
PQconnectdbParams() function
(https://www.postgresql.org/docs/current/libpq-connect.html).

This also allow us to have slightly different database connection
parameters for different database connection. This is used here to add
the connection id into the fallback_application_name parameter. This
connection id also reported when using --log-sql. This way you can
correlate the ids from the log with the information shown in
pg_stat_activity.

This commit looks large, because it changes the type std::string to
connection_parameters_t in many places. I have kept the parameter name
"conninfo" in an effort to keep the commit small, the next commit will
change that also.

Author: joto

src/command-line-app.cpp

@@ -32,29 +32,44 @@ bool command_line_app_t::want_version() const { return count("--version"); }
 
 void command_line_app_t::init_database_options()
 {
-    add_option("-d,--database", m_database_options.db)
+    add_option_function<std::string>("-d,--database",
+                                     [&](std::string const &value) {
+                                         m_connection_params.set("dbname",
+                                                                 value);
+                                     })
         ->description("Database name or PostgreSQL conninfo string.")
         ->type_name("DB")
         ->group("Database options");
 
-    add_option("-U,--user", m_database_options.username)
+    add_option_function<std::string>("-U,--user",
+                                     [&](std::string const &value) {
+                                         m_connection_params.set("user", value);
+                                     })
         ->description("Database user.")
         ->type_name("USERNAME")
         ->group("Database options");
 
-    add_flag_function(
-        "-W,--password",
-        [&](int64_t) { m_database_options.password = util::get_password(); })
+    add_flag_function("-W,--password",
+                      [&](int64_t) {
+                          m_connection_params.set("password",
+                                                  util::get_password());
+                      })
         ->description("Force password prompt.")
         ->group("Database options");
 
-    add_option("-H,--host", m_database_options.host)
+    add_option_function<std::string>("-H,--host",
+                                     [&](std::string const &value) {
+                                         m_connection_params.set("host", value);
+                                     })
         ->description(
             "Database server hostname or unix domain socket location.")
         ->type_name("HOST")
         ->group("Database options");
 
-    add_option("-P,--port", m_database_options.port)
+    add_option_function<std::string>("-P,--port",
+                                     [&](std::string const &value) {
+                                         m_connection_params.set("port", value);
+                                     })
         ->description("Database server port.")
         ->type_name("PORT")
         ->group("Database options");

src/command-line-app.hpp

@@ -26,13 +26,13 @@ class command_line_app_t : public CLI::App
 
     bool want_version() const;
 
-    database_options_t database_options() const noexcept
+    connection_params_t connection_params() const noexcept
     {
-        return m_database_options;
+        return m_connection_params;
     }
 
 private:
-    database_options_t m_database_options;
+    connection_params_t m_connection_params;
 
     void init_database_options();
     void init_logging_options();

src/command-line-parser.cpp

@@ -32,48 +32,6 @@
 #include <stdexcept>
 #include <thread> // for number of threads
 
-static bool compare_prefix(std::string const &str,
-                           std::string const &prefix) noexcept
-{
-    return std::strncmp(str.c_str(), prefix.c_str(), prefix.size()) == 0;
-}
-
-std::string build_conninfo(database_options_t const &opt)
-{
-    if (compare_prefix(opt.db, "postgresql://") ||
-        compare_prefix(opt.db, "postgres://")) {
-        return opt.db;
-    }
-
-    util::string_joiner_t joiner{' '};
-    joiner.add("fallback_application_name='osm2pgsql'");
-
-    if (std::strchr(opt.db.c_str(), '=') != nullptr) {
-        joiner.add(opt.db);
-        return joiner();
-    }
-
-    joiner.add("client_encoding='UTF8'");
-
-    if (!opt.db.empty()) {
-        joiner.add(fmt::format("dbname='{}'", opt.db));
-    }
-    if (!opt.username.empty()) {
-        joiner.add(fmt::format("user='{}'", opt.username));
-    }
-    if (!opt.password.empty()) {
-        joiner.add(fmt::format("password='{}'", opt.password));
-    }
-    if (!opt.host.empty()) {
-        joiner.add(fmt::format("host='{}'", opt.host));
-    }
-    if (!opt.port.empty()) {
-        joiner.add(fmt::format("port='{}'", opt.port));
-    }
-
-    return joiner();
-}
-
 static osmium::Box parse_bbox_param(std::string const &arg)
 {
     double minx = NAN;
@@ -730,7 +688,7 @@ options_t parse_command_line(int argc, char *argv[])
 
     check_options(&options);
 
-    options.conninfo = build_conninfo(app.database_options());
+    options.conninfo = app.connection_params();
 
     if (!options.slim) {
         options.middle_database_format = 0;

src/db-copy.cpp

@@ -81,10 +81,11 @@ void db_deleter_by_type_and_id_t::delete_rows(std::string const &table,
     conn->exec(sql.data());
 }
 
-db_copy_thread_t::db_copy_thread_t(std::string const &conninfo)
+db_copy_thread_t::db_copy_thread_t(connection_params_t const &conninfo)
 {
-    // conninfo is captured by copy here, because we don't know wether the
-    // reference will still be valid once we get around to running the thread
+    // Connection params are captured by copy here, because we don't know
+    // whether the reference will still be valid once we get around to running
+    // the thread.
     m_worker = std::thread{thread_t{conninfo, &m_shared}};
 }
 
@@ -119,7 +120,8 @@ void db_copy_thread_t::finish()
     }
 }
 
-db_copy_thread_t::thread_t::thread_t(std::string conninfo, shared *shared)
+db_copy_thread_t::thread_t::thread_t(connection_params_t conninfo,
+                                     shared *shared)
 : m_conninfo(std::move(conninfo)), m_shared(shared)
 {}
 

src/db-copy.hpp

@@ -249,7 +249,7 @@ struct db_cmd_finish_t : public db_cmd_t
 class db_copy_thread_t
 {
 public:
-    explicit db_copy_thread_t(std::string const &conninfo);
+    explicit db_copy_thread_t(connection_params_t const &conninfo);
 
     db_copy_thread_t(db_copy_thread_t const &) = delete;
     db_copy_thread_t &operator=(db_copy_thread_t const &) = delete;
@@ -290,7 +290,7 @@ class db_copy_thread_t
     class thread_t
     {
     public:
-        thread_t(std::string conninfo, shared *shared);
+        thread_t(connection_params_t conninfo, shared *shared);
 
         void operator()();
 
@@ -300,7 +300,7 @@ class db_copy_thread_t
         void finish_copy();
         void delete_rows(db_cmd_copy_t *buffer);
 
-        std::string m_conninfo;
+        connection_params_t m_conninfo;
         std::unique_ptr<pg_conn_t> m_conn;
 
         // Target for copy operation currently ongoing.

src/expire-output.cpp

@@ -17,7 +17,7 @@
 #include <system_error>
 
 std::size_t expire_output_t::output(quadkey_list_t const &tile_list,
-                                    std::string const &conninfo) const
+                                    connection_params_t const &conninfo) const
 {
     std::size_t num = 0;
     if (!m_filename.empty()) {
@@ -51,9 +51,9 @@ std::size_t expire_output_t::output_tiles_to_file(
     return count;
 }
 
-std::size_t
-expire_output_t::output_tiles_to_table(quadkey_list_t const &tiles_at_maxzoom,
-                                       std::string const &conninfo) const
+std::size_t expire_output_t::output_tiles_to_table(
+    quadkey_list_t const &tiles_at_maxzoom,
+    connection_params_t const &conninfo) const
 {
     auto const qn = qualified_name(m_schema, m_table);
 

src/expire-output.hpp

@@ -18,6 +18,7 @@
 #include <utility>
 
 class pg_conn_t;
+class connection_params_t;
 
 /**
  * Output for tile expiry.
@@ -52,7 +53,7 @@ class expire_output_t
     void set_maxzoom(uint32_t maxzoom) noexcept { m_maxzoom = maxzoom; }
 
     std::size_t output(quadkey_list_t const &tile_list,
-                       std::string const &conninfo) const;
+                       connection_params_t const &conninfo) const;
 
     /**
      * Write the list of tiles to a file.
@@ -66,10 +67,11 @@ class expire_output_t
      * Write the list of tiles to a database table.
      *
      * \param tiles_at_maxzoom The list of tiles at maximum zoom level
-     * \param conninfo database connection info
+     * \param conninfo Database connection parameters
      */
-    std::size_t output_tiles_to_table(quadkey_list_t const &tiles_at_maxzoom,
-                                      std::string const &conninfo) const;
+    std::size_t
+    output_tiles_to_table(quadkey_list_t const &tiles_at_maxzoom,
+                          connection_params_t const &conninfo) const;
 
     /**
      * Create table for tiles.

src/flex-table.cpp

@@ -241,7 +241,7 @@ bool flex_table_t::has_columns_with_expire() const noexcept
                        [](auto const &column) { return column.has_expire(); });
 }
 
-void table_connection_t::connect(std::string const &conninfo)
+void table_connection_t::connect(connection_params_t const &conninfo)
 {
     assert(!m_db_connection);
 

src/flex-table.hpp

@@ -259,7 +259,7 @@ class table_connection_t
     {
     }
 
-    void connect(std::string const &conninfo);
+    void connect(connection_params_t const &conninfo);
 
     void start(bool append);
 

src/gen/osm2pgsql-gen.cpp

@@ -195,8 +195,9 @@ class tile_processor_t
     std::size_t m_num_tiles;
 };
 
-void run_tile_gen(std::string const &conninfo, gen_base_t *master_generalizer,
-                  params_t params, uint32_t zoom,
+void run_tile_gen(connection_params_t const &conninfo,
+                  gen_base_t *master_generalizer, params_t params,
+                  uint32_t zoom,
                   std::vector<std::pair<uint32_t, uint32_t>> *queue,
                   std::mutex *mut, unsigned int n)
 {
@@ -231,9 +232,9 @@ void run_tile_gen(std::string const &conninfo, gen_base_t *master_generalizer,
 class genproc_t
 {
 public:
-    genproc_t(std::string const &filename, std::string conninfo,
-              std::string dbschema, bool append, bool updatable,
-              uint32_t jobs);
+    genproc_t(std::string const &filename,
+              connection_params_t conninfo, std::string dbschema,
+              bool append, bool updatable, uint32_t jobs);
 
     int app_define_table()
     {
@@ -512,7 +513,7 @@ class genproc_t
     std::vector<flex_table_t> m_tables;
     std::vector<expire_output_t> m_expire_outputs;
 
-    std::string m_conninfo;
+    connection_params_t m_conninfo;
     std::string m_dbschema;
     uint32_t m_jobs;
     bool m_append;
@@ -524,7 +525,7 @@ TRAMPOLINE(app_define_expire_output, define_expire_output)
 TRAMPOLINE(app_run_gen, run_gen)
 TRAMPOLINE(app_run_sql, run_sql)
 
-genproc_t::genproc_t(std::string const &filename, std::string conninfo,
+genproc_t::genproc_t(std::string const &filename, connection_params_t conninfo,
                      std::string dbschema, bool append, bool updatable,
                      uint32_t jobs)
 : m_conninfo(std::move(conninfo)), m_dbschema(std::move(dbschema)),
@@ -694,15 +695,15 @@ int main(int argc, char *argv[])
                 jobs);
         }
 
-        auto const conninfo = build_conninfo(app.database_options());
+        auto const connection_params = app.connection_params();
 
         log_debug("Checking database capabilities...");
         {
-            pg_conn_t const db_connection{conninfo};
+            pg_conn_t const db_connection{connection_params};
             init_database_capabilities(db_connection);
         }
 
-        properties_t properties{conninfo, middle_dbschema};
+        properties_t properties{connection_params, middle_dbschema};
         properties.load();
 
         if (style.empty()) {
@@ -719,7 +720,8 @@ int main(int argc, char *argv[])
         }
 
         bool const updatable = properties.get_bool("updatable", false);
-        genproc_t gen{style, conninfo, dbschema, append, updatable, jobs};
+        genproc_t gen{style,  connection_params, dbschema,
+                      append, updatable,         jobs};
         gen.run();
 
         osmium::MemoryUsage const mem;