From f011782ace22904c70e9a345b8a34ff59e0a0099 Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Mon, 2 Jan 2023 11:52:43 +0100 Subject: [PATCH] Add a variant of the database exec() command with fmt parameters Makes lots of code simpler. --- src/flex-table.cpp | 13 ++++++------ src/middle-pgsql.cpp | 13 ++++++------ src/pgsql-helper.cpp | 43 +++++++++++++++++++------------------- src/pgsql.cpp | 4 ++-- src/pgsql.hpp | 14 +++++++++++++ src/table.cpp | 41 +++++++++++++++++------------------- tests/common-pg.hpp | 8 +++---- tests/test-db-copy-mgr.cpp | 4 ++-- 8 files changed, 76 insertions(+), 64 deletions(-) diff --git a/src/flex-table.cpp b/src/flex-table.cpp index 2330a2fda..565e79928 100644 --- a/src/flex-table.cpp +++ b/src/flex-table.cpp @@ -287,13 +287,12 @@ void table_connection_t::start(bool append) m_db_connection->exec("SET client_min_messages = WARNING"); if (!append) { - m_db_connection->exec( - "DROP TABLE IF EXISTS {} CASCADE"_format(table().full_name())); + m_db_connection->exec("DROP TABLE IF EXISTS {} CASCADE", + table().full_name()); } // These _tmp tables can be left behind if we run out of disk space. - m_db_connection->exec( - "DROP TABLE IF EXISTS {}"_format(table().full_tmp_name())); + m_db_connection->exec("DROP TABLE IF EXISTS {}", table().full_tmp_name()); m_db_connection->exec("RESET client_min_messages"); if (!append) { @@ -362,9 +361,9 @@ void table_connection_t::stop(bool updateable, bool append) m_db_connection->exec(sql); - m_db_connection->exec("DROP TABLE {}"_format(table().full_name())); - m_db_connection->exec(R"(ALTER TABLE {} RENAME TO "{}")"_format( - table().full_tmp_name(), table().name())); + m_db_connection->exec("DROP TABLE {}", table().full_name()); + m_db_connection->exec(R"(ALTER TABLE {} RENAME TO "{}")", + table().full_tmp_name(), table().name()); m_id_index_created = false; if (updateable) { diff --git a/src/middle-pgsql.cpp b/src/middle-pgsql.cpp index 1d4376e6f..db329e362 100644 --- a/src/middle-pgsql.cpp +++ b/src/middle-pgsql.cpp @@ -85,7 +85,7 @@ void middle_pgsql_t::table_desc::drop_table( log_info("Dropping table '{}'", name()); auto const qual_name = qualified_name(schema(), name()); - db_connection.exec("DROP TABLE IF EXISTS {}"_format(qual_name)); + db_connection.exec("DROP TABLE IF EXISTS {}", qual_name); log_info("Table '{}' dropped in {}", name(), util::human_readable_duration(timer.stop())); @@ -668,8 +668,7 @@ void middle_pgsql_t::start() for (auto const &table : m_tables) { log_debug("Setting up table '{}'", table.name()); auto const qual_name = qualified_name(table.schema(), table.name()); - m_db_connection.exec( - "DROP TABLE IF EXISTS {} CASCADE"_format(qual_name)); + m_db_connection.exec("DROP TABLE IF EXISTS {} CASCADE", qual_name); if (!table.m_create_table.empty()) { m_db_connection.exec(table.m_create_table); } @@ -826,9 +825,11 @@ static table_sql sql_for_relations() noexcept static bool check_bucket_index(pg_conn_t *db_connection, std::string const &prefix) { - auto const res = db_connection->exec( - "SELECT relname FROM pg_class WHERE relkind='i' AND" - " relname = '{}_ways_nodes_bucket_idx';"_format(prefix)); + auto const res = + db_connection->exec("SELECT relname FROM pg_class" + " WHERE relkind='i'" + " AND relname = '{}_ways_nodes_bucket_idx'", + prefix); return res.num_tuples() > 0; } diff --git a/src/pgsql-helper.cpp b/src/pgsql-helper.cpp index 8c0392525..04928a7a6 100644 --- a/src/pgsql-helper.cpp +++ b/src/pgsql-helper.cpp @@ -39,24 +39,24 @@ void create_geom_check_trigger(pg_conn_t *db_connection, { std::string func_name = qualified_name(schema, table + "_osm2pgsql_valid"); - db_connection->exec( - "CREATE OR REPLACE FUNCTION {}()\n" - "RETURNS TRIGGER AS $$\n" - "BEGIN\n" - " IF {} THEN \n" - " RETURN NEW;\n" - " END IF;\n" - " RETURN NULL;\n" - "END;" - "$$ LANGUAGE plpgsql;"_format(func_name, condition)); + db_connection->exec("CREATE OR REPLACE FUNCTION {}()\n" + "RETURNS TRIGGER AS $$\n" + "BEGIN\n" + " IF {} THEN \n" + " RETURN NEW;\n" + " END IF;\n" + " RETURN NULL;\n" + "END;" + "$$ LANGUAGE plpgsql", + func_name, condition); - db_connection->exec( - "CREATE TRIGGER \"{}\"" - " BEFORE INSERT OR UPDATE" - " ON {}" - " FOR EACH ROW EXECUTE PROCEDURE" - " {}();"_format(table + "_osm2pgsql_valid", - qualified_name(schema, table), func_name)); + db_connection->exec("CREATE TRIGGER \"{}\"" + " BEFORE INSERT OR UPDATE" + " ON {}" + " FOR EACH ROW EXECUTE PROCEDURE" + " {}()", + table + "_osm2pgsql_valid", + qualified_name(schema, table), func_name); } void drop_geom_check_trigger(pg_conn_t *db_connection, @@ -65,17 +65,18 @@ void drop_geom_check_trigger(pg_conn_t *db_connection, { std::string func_name = qualified_name(schema, table + "_osm2pgsql_valid"); - db_connection->exec(R"(DROP TRIGGER "{}" ON {};)"_format( - table + "_osm2pgsql_valid", qualified_name(schema, table))); + db_connection->exec(R"(DROP TRIGGER "{}" ON {})", + table + "_osm2pgsql_valid", + qualified_name(schema, table)); - db_connection->exec("DROP FUNCTION IF EXISTS {} ();"_format(func_name)); + db_connection->exec("DROP FUNCTION IF EXISTS {} ()", func_name); } void analyze_table(pg_conn_t const &db_connection, std::string const &schema, std::string const &name) { auto const qual_name = qualified_name(schema, name); - db_connection.exec("ANALYZE {}"_format(qual_name)); + db_connection.exec("ANALYZE {}", qual_name); } bool has_table(pg_conn_t const &db_connection, std::string const &schema, diff --git a/src/pgsql.cpp b/src/pgsql.cpp index b110407fe..6fb94f7a4 100644 --- a/src/pgsql.cpp +++ b/src/pgsql.cpp @@ -42,8 +42,8 @@ void pg_conn_t::set_config(char const *setting, char const *value) const // Update pg_settings instead of using SET because it does not yield // errors on older versions of PostgreSQL where the settings are not // implemented. - exec("UPDATE pg_settings SET setting = '{}' WHERE name = '{}'"_format( - value, setting)); + exec("UPDATE pg_settings SET setting = '{}' WHERE name = '{}'", value, + setting); } pg_result_t pg_conn_t::exec(char const *sql) const diff --git a/src/pgsql.hpp b/src/pgsql.hpp index 386d727fb..39c16a268 100644 --- a/src/pgsql.hpp +++ b/src/pgsql.hpp @@ -189,6 +189,20 @@ class pg_conn_t pg_result_t exec(char const *sql) const; pg_result_t exec(std::string const &sql) const; + /** + * Run the specified SQL command. + * + * \param sql The SQL command using fmt lib patterns. + * \param params Any number of arguments for the fmt lib. + * \throws std::runtime_exception If the command failed (didn't return + * status code PGRES_COMMAND_OK or PGRES_TUPLES_OK). + */ + template + pg_result_t exec(char const *sql, TArgs... params) const + { + return exec(fmt::format(sql, std::forward(params)...)); + } + /** * Run the named prepared SQL statement and return the results. * diff --git a/src/table.cpp b/src/table.cpp index 1556e8bfe..61538a2fc 100644 --- a/src/table.cpp +++ b/src/table.cpp @@ -92,12 +92,11 @@ void table_t::start(std::string const &conninfo, std::string const &table_space) // we are making a new table if (!m_append) { - m_sql_conn->exec( - "DROP TABLE IF EXISTS {} CASCADE"_format(qual_name)); + m_sql_conn->exec("DROP TABLE IF EXISTS {} CASCADE", qual_name); } // These _tmp tables can be left behind if we run out of disk space. - m_sql_conn->exec("DROP TABLE IF EXISTS {}"_format(qual_tmp_name)); + m_sql_conn->exec("DROP TABLE IF EXISTS {}", qual_tmp_name); m_sql_conn->exec("RESET client_min_messages"); //making a new table @@ -149,9 +148,9 @@ void table_t::prepare() { //let postgres cache this query as it will presumably happen a lot auto const qual_name = qualified_name(m_target->schema, m_target->name); - m_sql_conn->exec( - "PREPARE get_wkb(int8) AS SELECT way FROM {} WHERE osm_id = $1"_format( - qual_name)); + m_sql_conn->exec("PREPARE get_wkb(int8) AS" + " SELECT way FROM {} WHERE osm_id = $1", + qual_name); } void table_t::generate_copy_column_list() @@ -229,23 +228,22 @@ void table_t::stop(bool updateable, bool enable_hstore_index, m_sql_conn->exec(sql); - m_sql_conn->exec("DROP TABLE {}"_format(qual_name)); - m_sql_conn->exec(R"(ALTER TABLE {} RENAME TO "{}")"_format( - qual_tmp_name, m_target->name)); + m_sql_conn->exec("DROP TABLE {}", qual_name); + m_sql_conn->exec(R"(ALTER TABLE {} RENAME TO "{}")", qual_tmp_name, + m_target->name); log_info("Creating geometry index on table '{}'...", m_target->name); // Use fillfactor 100 for un-updatable imports - m_sql_conn->exec("CREATE INDEX ON {} USING GIST (way) {} {}"_format( - qual_name, (updateable ? "" : "WITH (fillfactor = 100)"), - tablespace_clause(table_space_index))); + m_sql_conn->exec("CREATE INDEX ON {} USING GIST (way) {} {}", qual_name, + (updateable ? "" : "WITH (fillfactor = 100)"), + tablespace_clause(table_space_index)); /* slim mode needs this to be able to apply diffs */ if (updateable) { log_info("Creating osm_id index on table '{}'...", m_target->name); - m_sql_conn->exec( - "CREATE INDEX ON {} USING BTREE (osm_id) {}"_format( - qual_name, tablespace_clause(table_space_index))); + m_sql_conn->exec("CREATE INDEX ON {} USING BTREE (osm_id) {}", + qual_name, tablespace_clause(table_space_index)); if (m_srid != "4326") { create_geom_check_trigger(m_sql_conn.get(), m_target->schema, m_target->name, @@ -258,15 +256,14 @@ void table_t::stop(bool updateable, bool enable_hstore_index, log_info("Creating hstore indexes on table '{}'...", m_target->name); if (m_hstore_mode != hstore_column::none) { - m_sql_conn->exec( - "CREATE INDEX ON {} USING GIN (tags) {}"_format( - qual_name, tablespace_clause(table_space_index))); + m_sql_conn->exec("CREATE INDEX ON {} USING GIN (tags) {}", + qual_name, + tablespace_clause(table_space_index)); } for (auto const &hcolumn : m_hstore_columns) { - m_sql_conn->exec( - R"(CREATE INDEX ON {} USING GIN ("{}") {})"_format( - qual_name, hcolumn, - tablespace_clause(table_space_index))); + m_sql_conn->exec(R"(CREATE INDEX ON {} USING GIN ("{}") {})", + qual_name, hcolumn, + tablespace_clause(table_space_index)); } } log_info("Analyzing table '{}'...", m_target->name); diff --git a/tests/common-pg.hpp b/tests/common-pg.hpp index b253159c1..aa0c30493 100644 --- a/tests/common-pg.hpp +++ b/tests/common-pg.hpp @@ -101,9 +101,9 @@ class tempdb_t conn_t conn{"dbname=postgres"}; m_db_name = "osm2pgsql-test-{}-{}"_format(getpid(), time(nullptr)); - conn.exec("DROP DATABASE IF EXISTS \"{}\""_format(m_db_name)); - conn.exec("CREATE DATABASE \"{}\" WITH ENCODING 'UTF8'"_format( - m_db_name)); + conn.exec(R"(DROP DATABASE IF EXISTS "{}")", m_db_name); + conn.exec(R"(CREATE DATABASE "{}" WITH ENCODING 'UTF8')", + m_db_name); conn_t local = connect(); local.exec("CREATE EXTENSION postgis"); @@ -136,7 +136,7 @@ class tempdb_t } try { conn_t conn{"dbname=postgres"}; - conn.exec("DROP DATABASE IF EXISTS \"{}\""_format(m_db_name)); + conn.exec(R"(DROP DATABASE IF EXISTS "{}")", m_db_name); } catch (...) { fprintf(stderr, "DROP DATABASE failed. Ignored.\n"); } diff --git a/tests/test-db-copy-mgr.cpp b/tests/test-db-copy-mgr.cpp index 8a4da2f18..8ecd376c9 100644 --- a/tests/test-db-copy-mgr.cpp +++ b/tests/test-db-copy-mgr.cpp @@ -24,8 +24,8 @@ static std::shared_ptr setup_table(std::string const &cols) { auto const conn = db.connect(); conn.exec("DROP TABLE IF EXISTS test_copy_mgr"); - conn.exec("CREATE TABLE test_copy_mgr (id int8{}{})"_format( - cols.empty() ? "" : ",", cols)); + conn.exec("CREATE TABLE test_copy_mgr (id int8{}{})", + cols.empty() ? "" : ",", cols); auto table = std::make_shared(); table->name = "test_copy_mgr";