Merge pull request #1834 from joto/refactor-conninfo

Refactor: Build conninfo string only once on program start

Author: lonvia

src/db-check.cpp

@@ -36,7 +36,7 @@ static bool has_table(pg_conn_t const &db_connection, std::string const &schema,
 
 void check_db(options_t const &options)
 {
-    pg_conn_t db_connection{options.database_options.conninfo()};
+    pg_conn_t db_connection{options.conninfo};
 
     auto const settings = get_postgresql_settings(db_connection);
 

src/middle-pgsql.cpp

@@ -691,9 +691,8 @@ void middle_pgsql_t::stop()
     } else if (!m_options->append) {
         // Building the indexes takes time, so do it asynchronously.
         for (auto &table : m_tables) {
-            table.task_set(thread_pool().submit([&]() {
-                table.build_index(m_options->database_options.conninfo());
-            }));
+            table.task_set(thread_pool().submit(
+                [&]() { table.build_index(m_options->conninfo); }));
         }
     }
 }
@@ -837,9 +836,8 @@ middle_pgsql_t::middle_pgsql_t(std::shared_ptr<thread_pool_t> thread_pool,
 : middle_t(std::move(thread_pool)), m_options(options),
   m_cache(std::make_unique<node_locations_t>(
       static_cast<std::size_t>(options->cache) * 1024UL * 1024UL)),
-  m_db_connection(m_options->database_options.conninfo()),
-  m_copy_thread(
-      std::make_shared<db_copy_thread_t>(options->database_options.conninfo())),
+  m_db_connection(m_options->conninfo),
+  m_copy_thread(std::make_shared<db_copy_thread_t>(options->conninfo)),
   m_db_copy(m_copy_thread)
 {
     if (!options->flat_node_file.empty()) {
@@ -871,7 +869,7 @@ middle_pgsql_t::get_query_instance()
     // NOTE: this is thread safe for use in pending async processing only because
     // during that process they are only read from
     auto mid = std::make_unique<middle_query_pgsql_t>(
-        m_options->database_options.conninfo(), m_cache, m_persistent_cache);
+        m_options->conninfo, m_cache, m_persistent_cache);
 
     // We use a connection per table to enable the use of COPY
     for (auto &table : m_tables) {

src/options.cpp

@@ -248,35 +248,35 @@ static bool compare_prefix(std::string const &str,
     return std::strncmp(str.c_str(), prefix.c_str(), prefix.size()) == 0;
 }
 
-std::string database_options_t::conninfo() const
+std::string build_conninfo(database_options_t const &opt)
 {
-    if (compare_prefix(db, "postgresql://") ||
-        compare_prefix(db, "postgres://")) {
-        return db;
+    if (compare_prefix(opt.db, "postgresql://") ||
+        compare_prefix(opt.db, "postgres://")) {
+        return opt.db;
     }
 
     std::string out{"fallback_application_name='osm2pgsql'"};
 
-    if (std::strchr(db.c_str(), '=') != nullptr) {
+    if (std::strchr(opt.db.c_str(), '=') != nullptr) {
         out += " ";
-        out += db;
+        out += opt.db;
         return out;
     }
 
-    if (!db.empty()) {
-        out += " dbname='{}'"_format(db);
+    if (!opt.db.empty()) {
+        out += " dbname='{}'"_format(opt.db);
     }
-    if (!username.empty()) {
-        out += " user='{}'"_format(username);
+    if (!opt.username.empty()) {
+        out += " user='{}'"_format(opt.username);
     }
-    if (!password.empty()) {
-        out += " password='{}'"_format(password);
+    if (!opt.password.empty()) {
+        out += " password='{}'"_format(opt.password);
     }
-    if (!host.empty()) {
-        out += " host='{}'"_format(host);
+    if (!opt.host.empty()) {
+        out += " host='{}'"_format(opt.host);
     }
-    if (!port.empty()) {
-        out += " port='{}'"_format(port);
+    if (!opt.port.empty()) {
+        out += " port='{}'"_format(opt.port);
     }
 
     return out;
@@ -347,6 +347,7 @@ options_t::options_t(int argc, char *argv[]) : options_t()
         return;
     }
 
+    database_options_t database_options;
     bool help_verbose = false; // Will be set when -v/--verbose is set
 
     int c = 0;
@@ -646,6 +647,8 @@ options_t::options_t(int argc, char *argv[]) : options_t()
     if (pass_prompt) {
         database_options.password = util::get_password();
     }
+
+    conninfo = build_conninfo(database_options);
 }
 
 void options_t::check_options()

src/options.hpp

@@ -30,20 +30,18 @@ enum class hstore_column : char
     all = 2
 };
 
-/**
- * Database options, not specific to a table
- */
+/// Database connection options.
 struct database_options_t
 {
     std::string db;
     std::string username;
     std::string host;
     std::string password;
     std::string port;
-
-    std::string conninfo() const;
 };
 
+std::string build_conninfo(database_options_t const &opt);
+
 /**
  * Outputs can signal their requirements to the middle by setting these fields.
  */
@@ -93,7 +91,7 @@ class options_t
         return m_print_help;
     }
 
-    database_options_t database_options;
+    std::string conninfo; ///< connection info for database
 
     std::string prefix{"planet_osm"};         ///< prefix for table names
 

src/osmdata.cpp

@@ -30,7 +30,7 @@ osmdata_t::osmdata_t(std::unique_ptr<dependency_manager_t> dependency_manager,
                      std::shared_ptr<middle_t> mid,
                      std::shared_ptr<output_t> output, options_t const &options)
 : m_dependency_manager(std::move(dependency_manager)), m_mid(std::move(mid)),
-  m_output(std::move(output)), m_conninfo(options.database_options.conninfo()),
+  m_output(std::move(output)), m_conninfo(options.conninfo),
   m_bbox(options.bbox), m_num_procs(options.num_procs),
   m_append(options.append), m_droptemp(options.droptemp),
   m_with_extra_attrs(options.extra_attributes),

src/output-flex.cpp

@@ -1967,7 +1967,7 @@ void output_flex_t::relation_modify(osmium::Relation const &rel)
 void output_flex_t::start()
 {
     for (auto &table : m_table_connections) {
-        table.connect(get_options()->database_options.conninfo());
+        table.connect(get_options()->conninfo);
         table.start(get_options()->append);
     }
 }
@@ -1987,7 +1987,7 @@ output_flex_t::output_flex_t(output_flex_t const *other,
 {
     for (auto &table : *m_tables) {
         auto &tc = m_table_connections.emplace_back(&table, m_copy_thread);
-        tc.connect(get_options()->database_options.conninfo());
+        tc.connect(get_options()->conninfo);
         tc.prepare();
     }
 }

src/output-gazetteer.cpp

@@ -49,7 +49,7 @@ void output_gazetteer_t::start()
     if (!get_options()->append) {
         int const srid = get_options()->projection->target_srs();
 
-        pg_conn_t conn{get_options()->database_options.conninfo()};
+        pg_conn_t conn{get_options()->conninfo};
 
         /* Drop any existing table */
         conn.exec("DROP TABLE IF EXISTS place CASCADE");

src/output-pgsql.cpp

@@ -407,8 +407,7 @@ void output_pgsql_t::start()
 {
     for (auto &t : m_tables) {
         //setup the table in postgres
-        t->start(get_options()->database_options.conninfo(),
-                 get_options()->tblsmain_data);
+        t->start(get_options()->conninfo, get_options()->tblsmain_data);
     }
 }
 

src/output.cpp

@@ -30,8 +30,7 @@ output_t::create_output(std::shared_ptr<middle_query_t> const &mid,
                         std::shared_ptr<thread_pool_t> thread_pool,
                         options_t const &options)
 {
-    auto copy_thread =
-        std::make_shared<db_copy_thread_t>(options.database_options.conninfo());
+    auto copy_thread = std::make_shared<db_copy_thread_t>(options.conninfo);
 
     if (options.output_backend == "pgsql") {
         return std::make_shared<output_pgsql_t>(mid, std::move(thread_pool),

tests/common-import.hpp

@@ -134,7 +134,7 @@ class import_t
                     std::initializer_list<std::string> input_data,
                     std::string const &format = "opl")
     {
-        options.database_options = m_db.db_options();
+        options.conninfo = m_db.conninfo();
 
         auto thread_pool = std::make_shared<thread_pool_t>(1U);
         auto middle = create_middle(thread_pool, options);
@@ -172,7 +172,7 @@ class import_t
 
     void run_file(options_t options, char const *file = nullptr)
     {
-        options.database_options = m_db.db_options();
+        options.conninfo = m_db.conninfo();
 
         auto thread_pool = std::make_shared<thread_pool_t>(1U);
         auto middle = std::make_shared<middle_ram_t>(thread_pool, &options);