Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Store db connection parameters in a their own class #2129

Merged
merged 2 commits into from
Feb 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 22 additions & 7 deletions src/command-line-app.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
6 changes: 3 additions & 3 deletions src/command-line-app.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
44 changes: 1 addition & 43 deletions src/command-line-parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -730,7 +688,7 @@ options_t parse_command_line(int argc, char *argv[])

check_options(&options);

options.conninfo = build_conninfo(app.database_options());
options.connection_params = app.connection_params();

if (!options.slim) {
options.middle_database_format = 0;
Expand Down
16 changes: 9 additions & 7 deletions src/db-copy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,12 @@ 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 &connection_params)
{
// 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
m_worker = std::thread{thread_t{conninfo, &m_shared}};
// 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{connection_params, &m_shared}};
}

db_copy_thread_t::~db_copy_thread_t() { finish(); }
Expand Down Expand Up @@ -119,14 +120,15 @@ void db_copy_thread_t::finish()
}
}

db_copy_thread_t::thread_t::thread_t(std::string conninfo, shared *shared)
: m_conninfo(std::move(conninfo)), m_shared(shared)
db_copy_thread_t::thread_t::thread_t(connection_params_t connection_params,
shared *shared)
: m_connection_params(std::move(connection_params)), m_shared(shared)
{}

void db_copy_thread_t::thread_t::operator()()
{
try {
m_conn = std::make_unique<pg_conn_t>(m_conninfo);
m_conn = std::make_unique<pg_conn_t>(m_connection_params);

// Let commits happen faster by delaying when they actually occur.
m_conn->exec("SET synchronous_commit = off");
Expand Down
6 changes: 3 additions & 3 deletions src/db-copy.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 &connection_params);

db_copy_thread_t(db_copy_thread_t const &) = delete;
db_copy_thread_t &operator=(db_copy_thread_t const &) = delete;
Expand Down Expand Up @@ -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 connection_params, shared *shared);

void operator()();

Expand All @@ -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_connection_params;
std::unique_ptr<pg_conn_t> m_conn;

// Target for copy operation currently ongoing.
Expand Down
15 changes: 8 additions & 7 deletions src/expire-output.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,16 @@

#include <system_error>

std::size_t expire_output_t::output(quadkey_list_t const &tile_list,
std::string const &conninfo) const
std::size_t
expire_output_t::output(quadkey_list_t const &tile_list,
connection_params_t const &connection_params) const
{
std::size_t num = 0;
if (!m_filename.empty()) {
num = output_tiles_to_file(tile_list);
}
if (!m_table.empty()) {
num = output_tiles_to_table(tile_list, conninfo);
num = output_tiles_to_table(tile_list, connection_params);
}
return num;
}
Expand All @@ -51,13 +52,13 @@ 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 &connection_params) const
{
auto const qn = qualified_name(m_schema, m_table);

pg_conn_t connection{conninfo};
pg_conn_t connection{connection_params};

auto const result = connection.exec("SELECT * FROM {} LIMIT 1", qn);

Expand Down
10 changes: 6 additions & 4 deletions src/expire-output.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <utility>

class pg_conn_t;
class connection_params_t;

/**
* Output for tile expiry.
Expand Down Expand Up @@ -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 &connection_params) const;

/**
* Write the list of tiles to a file.
Expand All @@ -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 connection_params 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 &connection_params) const;

/**
* Create table for tiles.
Expand Down
4 changes: 2 additions & 2 deletions src/flex-table.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -241,11 +241,11 @@ 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 &connection_params)
{
assert(!m_db_connection);

m_db_connection = std::make_unique<pg_conn_t>(conninfo);
m_db_connection = std::make_unique<pg_conn_t>(connection_params);
m_db_connection->exec("SET synchronous_commit = off");
}

Expand Down
2 changes: 1 addition & 1 deletion src/flex-table.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ class table_connection_t
{
}

void connect(std::string const &conninfo);
void connect(connection_params_t const &connection_params);

void start(bool append);

Expand Down
Loading