From 0e7f92acaf245582de2103c03670be9bddaedfd4 Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Fri, 13 Dec 2024 14:59:12 +0100 Subject: [PATCH 1/9] Move params.[ch]pp and template.[ch]pp into main src directory From the "gen" directory. As preparation for using this code in the main program. --- CMakeLists.txt | 2 -- src/CMakeLists.txt | 2 ++ src/middle-pgsql.cpp | 66 +++++++++++++++++++++----------------- src/{gen => }/params.cpp | 0 src/{gen => }/params.hpp | 0 src/{gen => }/template.cpp | 0 src/{gen => }/template.hpp | 0 7 files changed, 39 insertions(+), 31 deletions(-) rename src/{gen => }/params.cpp (100%) rename src/{gen => }/params.hpp (100%) rename src/{gen => }/template.cpp (100%) rename src/{gen => }/template.hpp (100%) diff --git a/CMakeLists.txt b/CMakeLists.txt index 41a89dd60..9ed00e3b7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -266,9 +266,7 @@ else() src/gen/gen-tile-sql.cpp src/gen/gen-tile-vector.cpp src/gen/gen-tile.cpp - src/gen/params.cpp src/gen/raster.cpp - src/gen/template.cpp src/gen/tracer.cpp) target_link_libraries(osm2pgsql-gen osm2pgsql_lib ${LIBS} ${POTRACE_LIBRARY} ${OpenCV_LIBS}) endif() diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index ee2b260b9..e10db77e6 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -47,6 +47,7 @@ target_sources(osm2pgsql_lib PRIVATE output-null.cpp output-pgsql.cpp output.cpp + params.cpp pgsql-capabilities.cpp pgsql-helper.cpp pgsql.cpp @@ -58,6 +59,7 @@ target_sources(osm2pgsql_lib PRIVATE tagtransform-c.cpp tagtransform-lua.cpp tagtransform.cpp + template.cpp thread-pool.cpp tile.cpp util.cpp diff --git a/src/middle-pgsql.cpp b/src/middle-pgsql.cpp index 24c88dcf1..b4bf1bcdb 100644 --- a/src/middle-pgsql.cpp +++ b/src/middle-pgsql.cpp @@ -42,6 +42,7 @@ #include "options.hpp" #include "osmtypes.hpp" #include "pgsql-helper.hpp" +#include "template.hpp" #include "util.hpp" namespace { @@ -72,37 +73,44 @@ void load_id_list(pg_conn_t const &db_connection, std::string const &table, std::string build_sql(options_t const &options, std::string const &templ) { - std::string const using_tablespace{options.tblsslim_index.empty() - ? "" - : "USING INDEX TABLESPACE " + - options.tblsslim_index}; - std::string const schema = "\"" + options.middle_dbschema + "\"."; - return fmt::format( - fmt::runtime(templ), fmt::arg("prefix", options.prefix), - fmt::arg("schema", schema), - fmt::arg("unlogged", options.droptemp ? "UNLOGGED" : ""), - fmt::arg("using_tablespace", using_tablespace), - fmt::arg("data_tablespace", tablespace_clause(options.tblsslim_data)), - fmt::arg("index_tablespace", tablespace_clause(options.tblsslim_index)), - fmt::arg("way_node_index_id_shift", 5), - fmt::arg("attribute_columns_definition", - options.extra_attributes ? " created timestamp with time zone," - " version int4," - " changeset_id int4," - " user_id int4," - : ""), - fmt::arg("attribute_columns_use", - options.extra_attributes - ? ", EXTRACT(EPOCH FROM created) AS created, version, " - "changeset_id, user_id, u.name" - : ""), - fmt::arg("users_table_access", - options.extra_attributes - ? "LEFT JOIN " + schema + '"' + options.prefix + - "_users\" u ON o.user_id = u.id" - : "")); + params_t params; + params.set("prefix", options.prefix); + params.set("schema", schema); + params.set("unlogged", options.droptemp ? "UNLOGGED" : ""); + params.set("data_tablespace", tablespace_clause(options.tblsslim_data)); + params.set("index_tablespace", tablespace_clause(options.tblsslim_index)); + params.set("way_node_index_id_shift", 5); + + if (options.tblsslim_index.empty()) { + params.set("using_tablespace", ""); + } else { + params.set("using_tablespace", + "USING INDEX TABLESPACE " + options.tblsslim_index); + } + + if (options.extra_attributes) { + params.set("attribute_columns_definition", + " created timestamp with time zone," + " version int4," + " changeset_id int4," + " user_id int4,"); + params.set("attribute_columns_use", + ", EXTRACT(EPOCH FROM created) AS created, version, " + "changeset_id, user_id, u.name"); + params.set("users_table_access", "LEFT JOIN " + schema + '"' + + options.prefix + + "_users\" u ON o.user_id = u.id"); + } else { + params.set("attribute_columns_definition", ""); + params.set("attribute_columns_use", ""); + params.set("users_table_access", ""); + } + + template_t sql_template{templ}; + sql_template.set_params(params); + return sql_template.render(); } std::vector build_sql(options_t const &options, diff --git a/src/gen/params.cpp b/src/params.cpp similarity index 100% rename from src/gen/params.cpp rename to src/params.cpp diff --git a/src/gen/params.hpp b/src/params.hpp similarity index 100% rename from src/gen/params.hpp rename to src/params.hpp diff --git a/src/gen/template.cpp b/src/template.cpp similarity index 100% rename from src/gen/template.cpp rename to src/template.cpp diff --git a/src/gen/template.hpp b/src/template.hpp similarity index 100% rename from src/gen/template.hpp rename to src/template.hpp From 1a7fc3e2f3032b40d7b83519bf0f9d841b4f72fb Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Fri, 3 Jan 2025 14:42:32 +0100 Subject: [PATCH 2/9] Add versions of params_t::set() function and add some tests It seems superfluous to have all these implementations of the set() function, but there are some problems with MSVC 2019, and this is the workaround. See https://developercommunity.visualstudio.com/t/10242620 --- src/params.hpp | 37 ++++++++++++++++++++++-- tests/CMakeLists.txt | 1 + tests/test-params.cpp | 65 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 101 insertions(+), 2 deletions(-) create mode 100644 tests/test-params.cpp diff --git a/src/params.hpp b/src/params.hpp index 174465820..14c957e80 100644 --- a/src/params.hpp +++ b/src/params.hpp @@ -33,10 +33,43 @@ std::string to_string(param_value_t const &value); class params_t { public: - template + // It seems superfluous to have all these implementations of the set() + // function, but there are some problems with MSVC 2019, and this is the + // workaround. See https://developercommunity.visualstudio.com/t/10242620 + template + void set(K &&key, bool value) + { + m_map.insert_or_assign(std::forward(key), value); + } + + template + void set(K &&key, double value) + { + m_map.insert_or_assign(std::forward(key), value); + } + + template + void set(K &&key, float value) + { + m_map.insert_or_assign(std::forward(key), value); + } + + template >, + bool> = true> + void set(K &&key, V value) + { + m_map.insert_or_assign(std::forward(key), + static_cast(value)); + } + + template < + typename K, typename V, + std::enable_if_t>, + bool> = true> void set(K &&key, V &&value) { - m_map.insert_or_assign(std::forward(key), std::forward(value)); + m_map.insert_or_assign(std::forward(key), std::string(value)); } template diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 3e85a303b..d3be5455a 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -86,6 +86,7 @@ set_test(test-output-pgsql-style-file) set_test(test-output-pgsql-tablespace LABELS Tablespace) set_test(test-output-pgsql-validgeom) set_test(test-output-pgsql-z_order) +set_test(test-params LABELS NoDB) set_test(test-persistent-cache LABELS NoDB) set_test(test-pgsql) set_test(test-pgsql-capabilities) diff --git a/tests/test-params.cpp b/tests/test-params.cpp new file mode 100644 index 000000000..3e5c707bc --- /dev/null +++ b/tests/test-params.cpp @@ -0,0 +1,65 @@ +/** + * SPDX-License-Identifier: GPL-2.0-or-later + * + * This file is part of osm2pgsql (https://osm2pgsql.org/). + * + * Copyright (C) 2006-2024 by the osm2pgsql developer community. + * For a full list of authors see the git log. + */ + +#include + +#include "params.hpp" + +TEST_CASE("Set param value", "[NoDB]") +{ + param_value_t const p_null{null_param_t()}; + param_value_t const p_str{std::string{"foo"}}; + param_value_t const p_int{static_cast(26)}; + param_value_t const p_double{3.141}; + param_value_t const p_true{true}; + param_value_t const p_false{false}; + + REQUIRE(to_string(p_null).empty()); + REQUIRE(to_string(p_str) == "foo"); + REQUIRE(to_string(p_int) == "26"); + REQUIRE(to_string(p_double) == "3.141"); + REQUIRE(to_string(p_true) == "true"); + REQUIRE(to_string(p_false) == "false"); +} + +TEST_CASE("Params with different value types", "[NoDB]") +{ + params_t params; + REQUIRE_FALSE(params.has("foo")); + + params.set("foo", 99); + REQUIRE(params.has("foo")); + REQUIRE(params.get("foo") == param_value_t(static_cast(99))); + REQUIRE(params.get_int64("foo") == 99); + + params.set("foo", "astring"); + REQUIRE(params.has("foo")); + REQUIRE(params.get("foo") == param_value_t(std::string("astring"))); + REQUIRE(params.get_string("foo") == "astring"); + REQUIRE_THROWS(params.get_int64("foo")); +} + +TEST_CASE("Set params with explicit type", "[NoDB]") +{ + params_t params; + + params.set("isstring", "hi"); + params.set("isint", 567); + params.set("isdouble", 567.0); + params.set("istrue", true); + params.set("isfalse", false); + + REQUIRE(params.get_string("isstring") == "hi"); + REQUIRE(params.get_int64("isint") == 567); + REQUIRE(params.get_double("isdouble") == Approx(567.0)); + REQUIRE(params.get_bool("istrue")); + REQUIRE_FALSE(params.get_bool("isfalse")); + + REQUIRE_THROWS(params.get("does not exist")); +} From cd2e128e670c54152fe4a60073792fb0a4f28b78 Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Fri, 13 Dec 2024 16:50:49 +0100 Subject: [PATCH 3/9] Refactor: Use templates to create middle tables Add new middle_pgsql_t::render_template() and middle_pgsql_t::dbexec() functions and use them to create middle tables. --- src/middle-pgsql.cpp | 155 ++++++++++++++++++++++++++----------------- src/middle-pgsql.hpp | 10 +-- 2 files changed, 101 insertions(+), 64 deletions(-) diff --git a/src/middle-pgsql.cpp b/src/middle-pgsql.cpp index b4bf1bcdb..e02a9b620 100644 --- a/src/middle-pgsql.cpp +++ b/src/middle-pgsql.cpp @@ -130,14 +130,25 @@ std::vector build_sql(options_t const &options, middle_pgsql_t::table_desc::table_desc(options_t const &options, table_sql const &ts) -: m_create_table(build_sql(options, ts.create_table)), - m_prepare_queries(build_sql(options, ts.prepare_queries)), +: m_prepare_queries(build_sql(options, ts.prepare_queries)), m_copy_target(std::make_shared( options.middle_dbschema, build_sql(options, ts.name), "id")) { m_create_fw_dep_indexes = build_sql(options, ts.create_fw_dep_indexes); } +std::string middle_pgsql_t::render_template(std::string_view templ) const +{ + template_t sql_template{templ}; + sql_template.set_params(m_params); + return sql_template.render(); +} + +void middle_pgsql_t::dbexec(std::string_view templ) const +{ + m_db_connection.exec(render_template(templ)); +} + void middle_query_pgsql_t::exec_sql(std::string const &sql_cmd) const { m_db_connection.exec(sql_cmd); @@ -171,14 +182,6 @@ void middle_pgsql_t::table_desc::build_index( } } -void middle_pgsql_t::table_desc::create_table( - pg_conn_t const &db_connection) const -{ - if (!m_create_table.empty()) { - db_connection.exec(m_create_table); - } -} - void middle_pgsql_t::table_desc::init_max_id(pg_conn_t const &db_connection) { auto const qual_name = qualified_name(schema(), name()); @@ -696,7 +699,7 @@ INSERT INTO osm2pgsql_changed_relations )"); for (auto const &query : queries) { - m_db_connection.exec(build_sql(*m_options, query)); + dbexec(query); } if (parent_ways) { @@ -741,12 +744,12 @@ void middle_pgsql_t::get_way_parents(idlist_t const &changed_ways, m_db_connection.exec("ANALYZE osm2pgsql_changed_ways"); - m_db_connection.exec(build_sql(*m_options, R"( + dbexec(R"( INSERT INTO osm2pgsql_changed_relations SELECT DISTINCT r.id FROM {schema}"{prefix}_rels" r, osm2pgsql_changed_ways c WHERE {schema}"{prefix}_member_ids"(r.members, 'W'::char) && ARRAY[c.id]; - )")); + )"); load_id_list(m_db_connection, "osm2pgsql_changed_relations", parent_relations); @@ -1046,18 +1049,6 @@ middle_query_pgsql_t::middle_query_pgsql_t( m_db_connection.set_config("max_parallel_workers_per_gather", "0"); } -namespace { - -void table_setup(pg_conn_t const &db_connection, - middle_pgsql_t::table_desc const &table) -{ - log_debug("Setting up table '{}'", table.name()); - drop_table_if_exists(db_connection, table.schema(), table.name()); - table.create_table(db_connection); -} - -} // anonymous namespace - void middle_pgsql_t::start() { assert(m_middle_state == middle_state::constructed); @@ -1080,11 +1071,46 @@ void middle_pgsql_t::start() } m_tables.ways().init_max_id(m_db_connection); m_tables.relations().init_max_id(m_db_connection); - } else { - table_setup(m_db_connection, m_users_table); - for (auto const &table : m_tables) { - table_setup(m_db_connection, table); - } + return; + } + + if (m_store_options.nodes) { + log_debug("Setting up table 'nodes'"); + dbexec(R"(DROP TABLE IF EXISTS {schema}"{prefix}_nodes" CASCADE)"); + dbexec("CREATE {unlogged} TABLE {schema}\"{prefix}_nodes\" (" + " id int8 PRIMARY KEY {using_tablespace}," + " lat int4 NOT NULL," + " lon int4 NOT NULL," + "{attribute_columns_definition}" + " tags jsonb" + ") {data_tablespace}"); + } + + log_debug("Setting up table 'ways'"); + dbexec(R"(DROP TABLE IF EXISTS {schema}"{prefix}_ways" CASCADE)"); + dbexec("CREATE {unlogged} TABLE {schema}\"{prefix}_ways\" (" + " id int8 PRIMARY KEY {using_tablespace}," + "{attribute_columns_definition}" + " nodes int8[] NOT NULL," + " tags jsonb" + ") {data_tablespace}"); + + log_debug("Setting up table 'rels'"); + dbexec(R"(DROP TABLE IF EXISTS {schema}"{prefix}_rels" CASCADE)"); + dbexec("CREATE {unlogged} TABLE {schema}\"{prefix}_rels\" (" + " id int8 PRIMARY KEY {using_tablespace}," + "{attribute_columns_definition}" + " members jsonb NOT NULL," + " tags jsonb" + ") {data_tablespace}"); + + if (m_store_options.with_attributes) { + log_debug("Setting up table 'users'"); + dbexec(R"(DROP TABLE IF EXISTS {schema}"{prefix}_users" CASCADE)"); + dbexec("CREATE TABLE {schema}\"{prefix}_users\" (" + " id INT4 PRIMARY KEY {using_tablespace}," + " name TEXT NOT NULL" + ") {data_tablespace}"); } } @@ -1168,13 +1194,6 @@ table_sql sql_for_users(middle_pgsql_options const &store_options) sql.name = "{prefix}_users"; - if (store_options.with_attributes) { - sql.create_table = "CREATE TABLE {schema}\"{prefix}_users\" (" - " id INT4 PRIMARY KEY {using_tablespace}," - " name TEXT NOT NULL" - ") {data_tablespace}"; - } - return sql; } @@ -1185,15 +1204,6 @@ table_sql sql_for_nodes(middle_pgsql_options const &options) sql.name = "{prefix}_nodes"; if (options.nodes) { - sql.create_table = - "CREATE {unlogged} TABLE {schema}\"{prefix}_nodes\" (" - " id int8 PRIMARY KEY {using_tablespace}," - " lat int4 NOT NULL," - " lon int4 NOT NULL," - "{attribute_columns_definition}" - " tags jsonb" - ") {data_tablespace}"; - sql.prepare_queries = { "PREPARE get_node_list(int8[]) AS" " SELECT id, lon, lat FROM {schema}\"{prefix}_nodes\"" @@ -1212,13 +1222,6 @@ table_sql sql_for_ways() sql.name = "{prefix}_ways"; - sql.create_table = "CREATE {unlogged} TABLE {schema}\"{prefix}_ways\" (" - " id int8 PRIMARY KEY {using_tablespace}," - "{attribute_columns_definition}" - " nodes int8[] NOT NULL," - " tags jsonb" - ") {data_tablespace}"; - sql.prepare_queries = {"PREPARE get_way(int8) AS" " SELECT nodes, tags{attribute_columns_use}" " FROM {schema}\"{prefix}_ways\" o" @@ -1251,13 +1254,6 @@ table_sql sql_for_relations() sql.name = "{prefix}_rels"; - sql.create_table = "CREATE {unlogged} TABLE {schema}\"{prefix}_rels\" (" - " id int8 PRIMARY KEY {using_tablespace}," - "{attribute_columns_definition}" - " members jsonb NOT NULL," - " tags jsonb" - ") {data_tablespace}"; - sql.prepare_queries = {"PREPARE get_rel(int8) AS" " SELECT members, tags{attribute_columns_use}" " FROM {schema}\"{prefix}_rels\" o" @@ -1284,6 +1280,43 @@ table_sql sql_for_relations() return sql; } +void init_params(params_t *params, options_t const &options) +{ + std::string const schema = "\"" + options.middle_dbschema + "\"."; + + params->set("prefix", options.prefix); + params->set("schema", schema); + params->set("unlogged", options.droptemp ? "UNLOGGED" : ""); + params->set("data_tablespace", tablespace_clause(options.tblsslim_data)); + params->set("index_tablespace", tablespace_clause(options.tblsslim_index)); + params->set("way_node_index_id_shift", 5); + + if (options.tblsslim_index.empty()) { + params->set("using_tablespace", ""); + } else { + params->set("using_tablespace", + "USING INDEX TABLESPACE " + options.tblsslim_index); + } + + if (options.extra_attributes) { + params->set("attribute_columns_definition", + " created timestamp with time zone," + " version int4," + " changeset_id int4," + " user_id int4,"); + params->set("attribute_columns_use", + ", EXTRACT(EPOCH FROM created) AS created, version, " + "changeset_id, user_id, u.name"); + params->set("users_table_access", "LEFT JOIN " + schema + '"' + + options.prefix + + "_users\" u ON o.user_id = u.id"); + } else { + params->set("attribute_columns_definition", ""); + params->set("attribute_columns_use", ""); + params->set("users_table_access", ""); + } +} + } // anonymous namespace middle_pgsql_t::middle_pgsql_t(std::shared_ptr thread_pool, @@ -1312,6 +1345,8 @@ middle_pgsql_t::middle_pgsql_t(std::shared_ptr thread_pool, log_debug("Mid: pgsql, cache={}", options->cache); + init_params(&m_params, *options); + m_tables.nodes() = table_desc{*options, sql_for_nodes(m_store_options)}; m_tables.ways() = table_desc{*options, sql_for_ways()}; m_tables.relations() = table_desc{*options, sql_for_relations()}; diff --git a/src/middle-pgsql.hpp b/src/middle-pgsql.hpp index 4c7dd9218..dfad00535 100644 --- a/src/middle-pgsql.hpp +++ b/src/middle-pgsql.hpp @@ -26,6 +26,7 @@ #include "db-copy-mgr.hpp" #include "idlist.hpp" #include "middle.hpp" +#include "params.hpp" #include "pgsql.hpp" class node_locations_t; @@ -88,7 +89,6 @@ class middle_query_pgsql_t : public middle_query_t struct table_sql { std::string name; - std::string create_table; std::vector prepare_queries; std::vector create_fw_dep_indexes; }; @@ -151,8 +151,6 @@ struct middle_pgsql_t : public middle_t std::chrono::microseconds task_wait() { return m_task_result.wait(); } - void create_table(pg_conn_t const &db_connection) const; - void init_max_id(pg_conn_t const &db_connection); osmid_t max_id() const noexcept { return m_max_id; } @@ -163,7 +161,6 @@ struct middle_pgsql_t : public middle_t } private: - std::string m_create_table; std::vector m_create_fw_dep_indexes; std::vector m_prepare_queries; std::shared_ptr m_copy_target; @@ -193,6 +190,9 @@ struct middle_pgsql_t : public middle_t void write_users_table(); void update_users_table(); + std::string render_template(std::string_view templ) const; + void dbexec(std::string_view templ) const; + std::map m_users; osmium::nwr_array m_tables; table_desc m_users_table; @@ -211,6 +211,8 @@ struct middle_pgsql_t : public middle_t /// Options for this middle. middle_pgsql_options m_store_options; + params_t m_params; + bool m_append; }; From 777394f55c008b8ebc29eea803d54ba36b350e66 Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Fri, 13 Dec 2024 17:08:32 +0100 Subject: [PATCH 4/9] Refactor: Use template rendering and explicit prepare function for preparing queries in the middle --- src/middle-pgsql.cpp | 71 +++++++++++++++++++++----------------------- src/middle-pgsql.hpp | 9 +----- 2 files changed, 35 insertions(+), 45 deletions(-) diff --git a/src/middle-pgsql.cpp b/src/middle-pgsql.cpp index e02a9b620..be6c2da56 100644 --- a/src/middle-pgsql.cpp +++ b/src/middle-pgsql.cpp @@ -130,8 +130,7 @@ std::vector build_sql(options_t const &options, middle_pgsql_t::table_desc::table_desc(options_t const &options, table_sql const &ts) -: m_prepare_queries(build_sql(options, ts.prepare_queries)), - m_copy_target(std::make_shared( +: m_copy_target(std::make_shared( options.middle_dbschema, build_sql(options, ts.name), "id")) { m_create_fw_dep_indexes = build_sql(options, ts.create_fw_dep_indexes); @@ -149,9 +148,10 @@ void middle_pgsql_t::dbexec(std::string_view templ) const m_db_connection.exec(render_template(templ)); } -void middle_query_pgsql_t::exec_sql(std::string const &sql_cmd) const +void middle_query_pgsql_t::prepare(std::string_view stmt, + std::string const &sql_cmd) const { - m_db_connection.exec(sql_cmd); + m_db_connection.prepare(stmt, fmt::runtime(sql_cmd)); } void middle_pgsql_t::table_desc::drop_table( @@ -1203,16 +1203,6 @@ table_sql sql_for_nodes(middle_pgsql_options const &options) sql.name = "{prefix}_nodes"; - if (options.nodes) { - sql.prepare_queries = { - "PREPARE get_node_list(int8[]) AS" - " SELECT id, lon, lat FROM {schema}\"{prefix}_nodes\"" - " WHERE id = ANY($1::int8[])", - "PREPARE get_node(int8) AS" - " SELECT id, lon, lat FROM {schema}\"{prefix}_nodes\"" - " WHERE id = $1"}; - } - return sql; } @@ -1222,17 +1212,6 @@ table_sql sql_for_ways() sql.name = "{prefix}_ways"; - sql.prepare_queries = {"PREPARE get_way(int8) AS" - " SELECT nodes, tags{attribute_columns_use}" - " FROM {schema}\"{prefix}_ways\" o" - " {users_table_access}" - " WHERE o.id = $1", - "PREPARE get_way_list(int8[]) AS" - " SELECT o.id, nodes, tags{attribute_columns_use}" - " FROM {schema}\"{prefix}_ways\" o" - " {users_table_access}" - " WHERE o.id = ANY($1::int8[])"}; - sql.create_fw_dep_indexes = { "CREATE OR REPLACE FUNCTION" " {schema}\"{prefix}_index_bucket\"(int8[])" @@ -1254,12 +1233,6 @@ table_sql sql_for_relations() sql.name = "{prefix}_rels"; - sql.prepare_queries = {"PREPARE get_rel(int8) AS" - " SELECT members, tags{attribute_columns_use}" - " FROM {schema}\"{prefix}_rels\" o" - " {users_table_access}" - " WHERE o.id = $1"}; - sql.create_fw_dep_indexes = { "CREATE OR REPLACE FUNCTION" " {schema}\"{prefix}_member_ids\"(jsonb, char)" @@ -1373,12 +1346,36 @@ middle_pgsql_t::get_query_instance() m_options->connection_params, m_cache, m_persistent_cache, m_store_options); - // We use a connection per table to enable the use of COPY - for (auto &table : m_tables) { - for (auto const &query : table.prepare_queries()) { - mid->exec_sql(query); - } - } + if (m_store_options.nodes) { + mid->prepare("get_node", + render_template( + "SELECT id, lon, lat FROM {schema}\"{prefix}_nodes\"" + " WHERE id = $1::int8")); + + mid->prepare("get_node_list", + render_template( + "SELECT id, lon, lat FROM {schema}\"{prefix}_nodes\"" + " WHERE id = ANY($1::int8[])")); + } + + mid->prepare("get_way", + render_template("SELECT nodes, tags{attribute_columns_use}" + " FROM {schema}\"{prefix}_ways\" o" + " {users_table_access}" + " WHERE o.id = $1::int8")); + + mid->prepare( + "get_way_list", + render_template("SELECT o.id, nodes, tags{attribute_columns_use}" + " FROM {schema}\"{prefix}_ways\" o" + " {users_table_access}" + " WHERE o.id = ANY($1::int8[])")); + + mid->prepare("get_rel", + render_template("SELECT members, tags{attribute_columns_use}" + " FROM {schema}\"{prefix}_rels\" o" + " {users_table_access}" + " WHERE o.id = $1::int8")); return std::shared_ptr(mid.release()); } diff --git a/src/middle-pgsql.hpp b/src/middle-pgsql.hpp index dfad00535..af9d8c4f6 100644 --- a/src/middle-pgsql.hpp +++ b/src/middle-pgsql.hpp @@ -71,7 +71,7 @@ class middle_query_pgsql_t : public middle_query_t bool relation_get(osmid_t id, osmium::memory::Buffer *buffer) const override; - void exec_sql(std::string const &sql_cmd) const; + void prepare(std::string_view stmt, std::string const &sql_cmd) const; private: osmium::Location get_node_location_flatnodes(osmid_t id) const; @@ -89,7 +89,6 @@ class middle_query_pgsql_t : public middle_query_t struct table_sql { std::string name; - std::vector prepare_queries; std::vector create_fw_dep_indexes; }; @@ -155,14 +154,8 @@ struct middle_pgsql_t : public middle_t osmid_t max_id() const noexcept { return m_max_id; } - std::vector const &prepare_queries() const noexcept - { - return m_prepare_queries; - } - private: std::vector m_create_fw_dep_indexes; - std::vector m_prepare_queries; std::shared_ptr m_copy_target; task_result_t m_task_result; From 094dfa06892f539b56c569aeb7d06c9b27cb57bf Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Fri, 13 Dec 2024 17:23:03 +0100 Subject: [PATCH 5/9] Refactor: Switch index creation to new template code in middle --- src/middle-pgsql.cpp | 114 +++++++++++++++++++------------------------ src/middle-pgsql.hpp | 5 -- 2 files changed, 49 insertions(+), 70 deletions(-) diff --git a/src/middle-pgsql.cpp b/src/middle-pgsql.cpp index be6c2da56..99102e101 100644 --- a/src/middle-pgsql.cpp +++ b/src/middle-pgsql.cpp @@ -113,19 +113,6 @@ std::string build_sql(options_t const &options, std::string const &templ) return sql_template.render(); } -std::vector build_sql(options_t const &options, - std::vector const &templs) -{ - std::vector out; - out.reserve(templs.size()); - - for (auto const &templ : templs) { - out.push_back(build_sql(options, templ)); - } - - return out; -} - } // anonymous namespace middle_pgsql_t::table_desc::table_desc(options_t const &options, @@ -133,7 +120,6 @@ middle_pgsql_t::table_desc::table_desc(options_t const &options, : m_copy_target(std::make_shared( options.middle_dbschema, build_sql(options, ts.name), "id")) { - m_create_fw_dep_indexes = build_sql(options, ts.create_fw_dep_indexes); } std::string middle_pgsql_t::render_template(std::string_view templ) const @@ -165,23 +151,6 @@ void middle_pgsql_t::table_desc::drop_table( util::human_readable_duration(timer.stop())); } -void middle_pgsql_t::table_desc::build_index( - connection_params_t const &connection_params) const -{ - if (m_create_fw_dep_indexes.empty()) { - return; - } - - // Use a temporary connection here because we might run in a separate - // thread context. - pg_conn_t const db_connection{connection_params, "middle.index"}; - - log_info("Building index on table '{}'", name()); - for (auto const &query : m_create_fw_dep_indexes) { - db_connection.exec(query); - } -} - void middle_pgsql_t::table_desc::init_max_id(pg_conn_t const &db_connection) { auto const qual_name = qualified_name(schema(), name()); @@ -1169,11 +1138,55 @@ void middle_pgsql_t::stop() table.drop_table(m_db_connection); } } 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->connection_params); })); - } + dbexec("CREATE OR REPLACE FUNCTION" + " {schema}\"{prefix}_index_bucket\"(int8[])" + " RETURNS int8[] AS $$" + " SELECT ARRAY(SELECT DISTINCT" + " unnest($1) >> {way_node_index_id_shift})" + "$$ LANGUAGE SQL IMMUTABLE"); + + auto const create_ways_index = render_template( + "CREATE INDEX \"{prefix}_ways_nodes_bucket_idx\"" + " ON {schema}\"{prefix}_ways\"" + " USING GIN ({schema}\"{prefix}_index_bucket\"(nodes))" + " WITH (fastupdate = off) {index_tablespace}"); + + log_info("Building index on middle ways table"); + m_tables.ways().task_set(thread_pool().submit([&, create_ways_index]() { + pg_conn_t const db_connection{m_options->connection_params, + "middle.index.ways"}; + db_connection.exec(create_ways_index); + })); + + dbexec("CREATE OR REPLACE FUNCTION" + " {schema}\"{prefix}_member_ids\"(jsonb, char)" + " RETURNS int8[] AS $$" + " SELECT array_agg((el->>'ref')::int8)" + " FROM jsonb_array_elements($1) AS el" + " WHERE el->>'type' = $2" + "$$ LANGUAGE SQL IMMUTABLE"); + + auto const create_rels_index_node_members = render_template( + "CREATE INDEX \"{prefix}_rels_node_members_idx\"" + " ON {schema}\"{prefix}_rels\" USING GIN" + " (({schema}\"{prefix}_member_ids\"(members, 'N'::char)))" + " WITH (fastupdate = off) {index_tablespace}"); + + auto const create_rels_index_way_members = render_template( + "CREATE INDEX \"{prefix}_rels_way_members_idx\"" + " ON {schema}\"{prefix}_rels\" USING GIN" + " (({schema}\"{prefix}_member_ids\"(members, 'W'::char)))" + " WITH (fastupdate = off) {index_tablespace}"); + + log_info("Building indexes on middle rels table"); + m_tables.relations().task_set( + thread_pool().submit([&, create_rels_index_node_members, + create_rels_index_way_members]() { + pg_conn_t const db_connection{m_options->connection_params, + "middle.index.rels"}; + db_connection.exec(create_rels_index_node_members); + db_connection.exec(create_rels_index_way_members); + })); } } @@ -1212,18 +1225,6 @@ table_sql sql_for_ways() sql.name = "{prefix}_ways"; - sql.create_fw_dep_indexes = { - "CREATE OR REPLACE FUNCTION" - " {schema}\"{prefix}_index_bucket\"(int8[])" - " RETURNS int8[] AS $$" - " SELECT ARRAY(SELECT DISTINCT" - " unnest($1) >> {way_node_index_id_shift})" - "$$ LANGUAGE SQL IMMUTABLE", - "CREATE INDEX \"{prefix}_ways_nodes_bucket_idx\"" - " ON {schema}\"{prefix}_ways\"" - " USING GIN ({schema}\"{prefix}_index_bucket\"(nodes))" - " WITH (fastupdate = off) {index_tablespace}"}; - return sql; } @@ -1233,23 +1234,6 @@ table_sql sql_for_relations() sql.name = "{prefix}_rels"; - sql.create_fw_dep_indexes = { - "CREATE OR REPLACE FUNCTION" - " {schema}\"{prefix}_member_ids\"(jsonb, char)" - " RETURNS int8[] AS $$" - " SELECT array_agg((el->>'ref')::int8)" - " FROM jsonb_array_elements($1) AS el" - " WHERE el->>'type' = $2" - "$$ LANGUAGE SQL IMMUTABLE", - "CREATE INDEX \"{prefix}_rels_node_members_idx\"" - " ON {schema}\"{prefix}_rels\" USING GIN" - " (({schema}\"{prefix}_member_ids\"(members, 'N'::char)))" - " WITH (fastupdate = off) {index_tablespace}", - "CREATE INDEX \"{prefix}_rels_way_members_idx\"" - " ON {schema}\"{prefix}_rels\" USING GIN" - " (({schema}\"{prefix}_member_ids\"(members, 'W'::char)))" - " WITH (fastupdate = off) {index_tablespace}"}; - return sql; } diff --git a/src/middle-pgsql.hpp b/src/middle-pgsql.hpp index af9d8c4f6..3744bba58 100644 --- a/src/middle-pgsql.hpp +++ b/src/middle-pgsql.hpp @@ -89,7 +89,6 @@ class middle_query_pgsql_t : public middle_query_t struct table_sql { std::string name; - std::vector create_fw_dep_indexes; }; struct middle_pgsql_t : public middle_t @@ -140,9 +139,6 @@ struct middle_pgsql_t : public middle_t ///< Drop table from database using existing database connection. void drop_table(pg_conn_t const &db_connection) const; - ///< Open a new database connection and build index on this table. - void build_index(connection_params_t const &connection_params) const; - void task_set(std::future &&future) { m_task_result.set(std::move(future)); @@ -155,7 +151,6 @@ struct middle_pgsql_t : public middle_t osmid_t max_id() const noexcept { return m_max_id; } private: - std::vector m_create_fw_dep_indexes; std::shared_ptr m_copy_target; task_result_t m_task_result; From f2195c37d5872c2f3a5f2552acaec3f3e98b51b5 Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Thu, 2 Jan 2025 20:39:27 +0100 Subject: [PATCH 6/9] Refactor: Remove the last part of the table_sql struct, the name --- src/middle-pgsql.cpp | 49 +++++++------------------------------------- src/middle-pgsql.hpp | 7 +------ 2 files changed, 8 insertions(+), 48 deletions(-) diff --git a/src/middle-pgsql.cpp b/src/middle-pgsql.cpp index 99102e101..3f1d0a804 100644 --- a/src/middle-pgsql.cpp +++ b/src/middle-pgsql.cpp @@ -116,9 +116,10 @@ std::string build_sql(options_t const &options, std::string const &templ) } // anonymous namespace middle_pgsql_t::table_desc::table_desc(options_t const &options, - table_sql const &ts) + std::string_view name) : m_copy_target(std::make_shared( - options.middle_dbschema, build_sql(options, ts.name), "id")) + options.middle_dbschema, fmt::format("{}_{}", options.prefix, name), + "id")) { } @@ -1201,42 +1202,6 @@ void middle_pgsql_t::wait() namespace { -table_sql sql_for_users(middle_pgsql_options const &store_options) -{ - table_sql sql{}; - - sql.name = "{prefix}_users"; - - return sql; -} - -table_sql sql_for_nodes(middle_pgsql_options const &options) -{ - table_sql sql{}; - - sql.name = "{prefix}_nodes"; - - return sql; -} - -table_sql sql_for_ways() -{ - table_sql sql{}; - - sql.name = "{prefix}_ways"; - - return sql; -} - -table_sql sql_for_relations() -{ - table_sql sql{}; - - sql.name = "{prefix}_rels"; - - return sql; -} - void init_params(params_t *params, options_t const &options) { std::string const schema = "\"" + options.middle_dbschema + "\"."; @@ -1304,11 +1269,11 @@ middle_pgsql_t::middle_pgsql_t(std::shared_ptr thread_pool, init_params(&m_params, *options); - m_tables.nodes() = table_desc{*options, sql_for_nodes(m_store_options)}; - m_tables.ways() = table_desc{*options, sql_for_ways()}; - m_tables.relations() = table_desc{*options, sql_for_relations()}; + m_tables.nodes() = table_desc{*options, "nodes"}; + m_tables.ways() = table_desc{*options, "ways"}; + m_tables.relations() = table_desc{*options, "rels"}; - m_users_table = table_desc{*options, sql_for_users(m_store_options)}; + m_users_table = table_desc{*options, "users"}; } void middle_pgsql_t::set_requirements( diff --git a/src/middle-pgsql.hpp b/src/middle-pgsql.hpp index 3744bba58..238b08cc7 100644 --- a/src/middle-pgsql.hpp +++ b/src/middle-pgsql.hpp @@ -86,11 +86,6 @@ class middle_query_pgsql_t : public middle_query_t middle_pgsql_options m_store_options; }; -struct table_sql -{ - std::string name; -}; - struct middle_pgsql_t : public middle_t { middle_pgsql_t(std::shared_ptr thread_pool, @@ -119,7 +114,7 @@ struct middle_pgsql_t : public middle_t { public: table_desc() = default; - table_desc(options_t const &options, table_sql const &ts); + table_desc(options_t const &options, std::string_view name); std::string const &schema() const noexcept { From 25f77a8a32c529984c1656221fbcb8b74f607824 Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Thu, 2 Jan 2025 20:59:02 +0100 Subject: [PATCH 7/9] Refactor: Remove m_users_table member variable Making the code simpler. --- src/middle-pgsql.cpp | 23 +++++++++++------------ src/middle-pgsql.hpp | 1 - 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/middle-pgsql.cpp b/src/middle-pgsql.cpp index 3f1d0a804..302cad55d 100644 --- a/src/middle-pgsql.cpp +++ b/src/middle-pgsql.cpp @@ -1086,10 +1086,12 @@ void middle_pgsql_t::start() void middle_pgsql_t::write_users_table() { - log_info("Writing {} entries to table '{}'...", m_users.size(), - m_users_table.name()); + auto const table_name = m_options->prefix + "_users"; + + log_info("Writing {} entries to table '{}'...", m_users.size(), table_name); + auto const users_table = std::make_shared( - m_users_table.schema(), m_users_table.name(), "id"); + m_options->dbschema, table_name, "id"); for (auto const &[id, name] : m_users) { m_db_copy.new_line(users_table); @@ -1100,20 +1102,20 @@ void middle_pgsql_t::write_users_table() m_users.clear(); - analyze_table(m_db_connection, m_users_table.schema(), - m_users_table.name()); + analyze_table(m_db_connection, m_options->dbschema, table_name); } void middle_pgsql_t::update_users_table() { - log_info("Writing {} entries to table '{}'...", m_users.size(), - m_users_table.name()); + auto const table_name = m_options->prefix + "_users"; + + log_info("Writing {} entries to table '{}'...", m_users.size(), table_name); m_db_connection.prepare( "insert_user", "INSERT INTO {}.\"{}\" (id, name) VALUES ($1::int8, $2::text)" " ON CONFLICT (id) DO UPDATE SET id=EXCLUDED.id", - m_users_table.schema(), m_users_table.name()); + m_options->dbschema, table_name); for (auto const &[id, name] : m_users) { m_db_connection.exec_prepared("insert_user", id, name); @@ -1121,8 +1123,7 @@ void middle_pgsql_t::update_users_table() m_users.clear(); - analyze_table(m_db_connection, m_users_table.schema(), - m_users_table.name()); + analyze_table(m_db_connection, m_options->dbschema, table_name); } void middle_pgsql_t::stop() @@ -1272,8 +1273,6 @@ middle_pgsql_t::middle_pgsql_t(std::shared_ptr thread_pool, m_tables.nodes() = table_desc{*options, "nodes"}; m_tables.ways() = table_desc{*options, "ways"}; m_tables.relations() = table_desc{*options, "rels"}; - - m_users_table = table_desc{*options, "users"}; } void middle_pgsql_t::set_requirements( diff --git a/src/middle-pgsql.hpp b/src/middle-pgsql.hpp index 238b08cc7..1df054d81 100644 --- a/src/middle-pgsql.hpp +++ b/src/middle-pgsql.hpp @@ -178,7 +178,6 @@ struct middle_pgsql_t : public middle_t std::map m_users; osmium::nwr_array m_tables; - table_desc m_users_table; options_t const *m_options; From b26f92841a287e7a4c0f9f8ce40d152def5234d3 Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Fri, 3 Jan 2025 09:16:51 +0100 Subject: [PATCH 8/9] Remove now unused build_sql() function --- src/middle-pgsql.cpp | 42 ------------------------------------------ 1 file changed, 42 deletions(-) diff --git a/src/middle-pgsql.cpp b/src/middle-pgsql.cpp index 302cad55d..0ed048136 100644 --- a/src/middle-pgsql.cpp +++ b/src/middle-pgsql.cpp @@ -71,48 +71,6 @@ void load_id_list(pg_conn_t const &db_connection, std::string const &table, } } -std::string build_sql(options_t const &options, std::string const &templ) -{ - std::string const schema = "\"" + options.middle_dbschema + "\"."; - - params_t params; - params.set("prefix", options.prefix); - params.set("schema", schema); - params.set("unlogged", options.droptemp ? "UNLOGGED" : ""); - params.set("data_tablespace", tablespace_clause(options.tblsslim_data)); - params.set("index_tablespace", tablespace_clause(options.tblsslim_index)); - params.set("way_node_index_id_shift", 5); - - if (options.tblsslim_index.empty()) { - params.set("using_tablespace", ""); - } else { - params.set("using_tablespace", - "USING INDEX TABLESPACE " + options.tblsslim_index); - } - - if (options.extra_attributes) { - params.set("attribute_columns_definition", - " created timestamp with time zone," - " version int4," - " changeset_id int4," - " user_id int4,"); - params.set("attribute_columns_use", - ", EXTRACT(EPOCH FROM created) AS created, version, " - "changeset_id, user_id, u.name"); - params.set("users_table_access", "LEFT JOIN " + schema + '"' + - options.prefix + - "_users\" u ON o.user_id = u.id"); - } else { - params.set("attribute_columns_definition", ""); - params.set("attribute_columns_use", ""); - params.set("users_table_access", ""); - } - - template_t sql_template{templ}; - sql_template.set_params(params); - return sql_template.render(); -} - } // anonymous namespace middle_pgsql_t::table_desc::table_desc(options_t const &options, From 6259590d295dbdd178e79e2747bf36620b8ddc45 Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Fri, 3 Jan 2025 10:30:34 +0100 Subject: [PATCH 9/9] Fix unused variable --- src/template.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/template.cpp b/src/template.cpp index 78d6bed80..b157087f5 100644 --- a/src/template.cpp +++ b/src/template.cpp @@ -19,7 +19,7 @@ std::string template_t::render() const { try { return fmt::vformat(m_template, m_format_store); - } catch (fmt::format_error const &e) { + } catch (fmt::format_error const &) { log_error("Missing parameter for template: '{}'", m_template); throw; }