Skip to content

Properly set schema for osm2pgsql_find_changed_ways() #2261

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Oct 13, 2024
Merged

Properly set schema for osm2pgsql_find_changed_ways() #2261

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a3a21a2
Properly set schema for osm2pgsql_find_changed_ways()
falko17 Oct 2, 2024
cd17e6b
Add tests to make sure no schema except specified one is touched
falko17 Oct 2, 2024
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
6 changes: 3 additions & 3 deletions src/middle-pgsql.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,7 @@ void middle_pgsql_t::get_node_parents(idlist_t const &changed_nodes,
// better to do a full table scan which totally destroys performance.
// This is due to the PostgreSQL statistics on ARRAYs being way off.
queries.emplace_back(R"(
CREATE OR REPLACE FUNCTION osm2pgsql_find_changed_ways() RETURNS void AS $$
CREATE OR REPLACE FUNCTION {schema}osm2pgsql_find_changed_ways() RETURNS void AS $$
DECLARE
changed_buckets RECORD;
BEGIN
Expand All @@ -677,8 +677,8 @@ BEGIN
END;
$$ LANGUAGE plpgsql
)");
queries.emplace_back("SELECT osm2pgsql_find_changed_ways()");
queries.emplace_back("DROP FUNCTION osm2pgsql_find_changed_ways()");
queries.emplace_back("SELECT {schema}osm2pgsql_find_changed_ways()");
queries.emplace_back("DROP FUNCTION {schema}osm2pgsql_find_changed_ways()");

queries.emplace_back(R"(
INSERT INTO osm2pgsql_changed_relations
Expand Down
7 changes: 7 additions & 0 deletions src/pgsql-params.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,13 @@ class connection_params_t

auto end() const noexcept { return m_params.end(); }

void merge_with(connection_params_t const &other)
{
for (auto const &p : other.m_params) {
m_params[p.first] = p.second;
}
}

private:
std::map<std::string, std::string> m_params;

Expand Down
4 changes: 2 additions & 2 deletions tests/common-import.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ class import_t
std::initializer_list<std::string> input_data,
std::string const &format = "opl")
{
options.connection_params = m_db.connection_params();
options.connection_params.merge_with(m_db.connection_params());

properties_t const properties{options.connection_params,
options.middle_dbschema};
Expand Down Expand Up @@ -165,7 +165,7 @@ class import_t

void run_file(options_t options, char const *file = nullptr)
{
options.connection_params = m_db.connection_params();
options.connection_params.merge_with(m_db.connection_params());

properties_t const properties{options.connection_params,
options.middle_dbschema};
Expand Down
15 changes: 15 additions & 0 deletions tests/common-options.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,21 @@ class opt_t
return *this;
}

opt_t &schema(char const *schema_name)
{
m_opt.dbschema = schema_name;
m_opt.middle_dbschema = schema_name;
m_opt.output_dbschema = schema_name;
return *this;
}

opt_t &user(char const *user, char const *password)
{
m_opt.connection_params.set("user", user);
m_opt.connection_params.set("password", password);
return *this;
}

private:
options_t m_opt;
};
Expand Down
155 changes: 103 additions & 52 deletions tests/test-output-flex-update.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

#include "common-import.hpp"
#include "common-options.hpp"
#include "common-pg.hpp"

namespace {

Expand Down Expand Up @@ -38,8 +39,50 @@ struct options_slim_expire
}
};

struct options_slim_schema
{
static options_t options()
{
auto conn = db.db().connect();
// Create limited user (if it doesn't exist yet),
// which we need to test that the public schema won't be touched.
// If the public schema is tried to be modified at any point, this user won't have the
// necessary permissions, and hence the test will fail.
conn.exec(R"(
DO
$$
BEGIN
IF NOT EXISTS (SELECT FROM pg_catalog.pg_roles WHERE rolname = 'limited') THEN
CREATE ROLE limited LOGIN PASSWORD 'password_limited';
END IF;
END
$$;
)");
conn.exec("REVOKE ALL PRIVILEGES ON ALL TABLES IN SCHEMA public FROM "
"PUBLIC, limited;");
conn.exec("REVOKE CREATE ON SCHEMA public FROM PUBLIC, limited;");
conn.exec(
"CREATE SCHEMA IF NOT EXISTS myschema AUTHORIZATION limited;");
conn.close();
return testing::opt_t()
.slim()
.flex(conf_file)
.schema("myschema")
.user("limited", "password_limited");
}
};

// Return a string with the schema name prepended to the table name.
std::string with_schema(char const *table_name, options_t const &options)
{
if (options.dbschema.empty()) {
return {table_name};
}
return options.dbschema + "." + table_name;
}

TEMPLATE_TEST_CASE("updating a node", "", options_slim_default,
options_slim_expire)
options_slim_expire, options_slim_schema)
{
options_t options = TestType::options();

Expand All @@ -48,16 +91,16 @@ TEMPLATE_TEST_CASE("updating a node", "", options_slim_default,

auto conn = db.db().connect();

REQUIRE(0 == conn.get_count("osm2pgsql_test_point"));
REQUIRE(0 == conn.get_count(with_schema("osm2pgsql_test_point", options)));

// give the node a tag...
options.append = true;
REQUIRE_NOTHROW(
db.run_import(options, "n10 v2 dV x10 y10 Tamenity=restaurant\n"));

REQUIRE(1 == conn.get_count("osm2pgsql_test_point"));
REQUIRE(1 == conn.get_count(with_schema("osm2pgsql_test_point", options)));
REQUIRE(1 ==
conn.get_count("osm2pgsql_test_point",
conn.get_count(with_schema("osm2pgsql_test_point", options),
"node_id = 10 AND tags->'amenity' = 'restaurant'"));

SECTION("remove the tag from node")
Expand All @@ -70,11 +113,11 @@ TEMPLATE_TEST_CASE("updating a node", "", options_slim_default,
REQUIRE_NOTHROW(db.run_import(options, "n10 v3 dD\n"));
}

REQUIRE(0 == conn.get_count("osm2pgsql_test_point"));
REQUIRE(0 == conn.get_count(with_schema("osm2pgsql_test_point", options)));
}

TEMPLATE_TEST_CASE("updating a way", "", options_slim_default,
options_slim_expire)
options_slim_expire, options_slim_schema)
{
options_t options = TestType::options();

Expand All @@ -86,9 +129,9 @@ TEMPLATE_TEST_CASE("updating a way", "", options_slim_default,

auto conn = db.db().connect();

REQUIRE(0 == conn.get_count("osm2pgsql_test_point"));
REQUIRE(1 == conn.get_count("osm2pgsql_test_line"));
REQUIRE(1 == conn.get_count("osm2pgsql_test_line",
REQUIRE(0 == conn.get_count(with_schema("osm2pgsql_test_point", options)));
REQUIRE(1 == conn.get_count(with_schema("osm2pgsql_test_line", options)));
REQUIRE(1 == conn.get_count(with_schema("osm2pgsql_test_line", options),
"osm_id = 20 AND tags->'highway' = 'primary' "
"AND ST_NumPoints(geom) = 2"));

Expand All @@ -97,18 +140,18 @@ TEMPLATE_TEST_CASE("updating a way", "", options_slim_default,
REQUIRE_NOTHROW(
db.run_import(options, "w20 v2 dV Thighway=secondary Nn10,n11\n"));

REQUIRE(0 == conn.get_count("osm2pgsql_test_point"));
REQUIRE(1 == conn.get_count("osm2pgsql_test_line"));
REQUIRE(1 == conn.get_count("osm2pgsql_test_line",
REQUIRE(0 == conn.get_count(with_schema("osm2pgsql_test_point", options)));
REQUIRE(1 == conn.get_count(with_schema("osm2pgsql_test_line", options)));
REQUIRE(1 == conn.get_count(with_schema("osm2pgsql_test_line", options),
"osm_id = 20 AND tags->'highway' = "
"'secondary' AND ST_NumPoints(geom) = 2"));

// now change a node in the way...
REQUIRE_NOTHROW(db.run_import(options, "n10 v2 dV x10.0 y10.3\n"));

REQUIRE(0 == conn.get_count("osm2pgsql_test_point"));
REQUIRE(1 == conn.get_count("osm2pgsql_test_line"));
REQUIRE(1 == conn.get_count("osm2pgsql_test_line",
REQUIRE(0 == conn.get_count(with_schema("osm2pgsql_test_point", options)));
REQUIRE(1 == conn.get_count(with_schema("osm2pgsql_test_line", options)));
REQUIRE(1 == conn.get_count(with_schema("osm2pgsql_test_line", options),
"osm_id = 20 AND tags->'highway' = "
"'secondary' AND ST_NumPoints(geom) = 2"));

Expand All @@ -117,21 +160,21 @@ TEMPLATE_TEST_CASE("updating a way", "", options_slim_default,
options, "n12 v1 dV x10.2 y10.1\n"
"w20 v3 dV Thighway=residential Nn10,n11,n12\n"));

REQUIRE(0 == conn.get_count("osm2pgsql_test_point"));
REQUIRE(1 == conn.get_count("osm2pgsql_test_line"));
REQUIRE(1 == conn.get_count("osm2pgsql_test_line",
REQUIRE(0 == conn.get_count(with_schema("osm2pgsql_test_point", options)));
REQUIRE(1 == conn.get_count(with_schema("osm2pgsql_test_line", options)));
REQUIRE(1 == conn.get_count(with_schema("osm2pgsql_test_line", options),
"osm_id = 20 AND tags->'highway' = "
"'residential' AND ST_NumPoints(geom) = 3"));

// now delete the way...
REQUIRE_NOTHROW(db.run_import(options, "w20 v4 dD\n"));

REQUIRE(0 == conn.get_count("osm2pgsql_test_point"));
REQUIRE(0 == conn.get_count("osm2pgsql_test_line"));
REQUIRE(0 == conn.get_count(with_schema("osm2pgsql_test_point", options)));
REQUIRE(0 == conn.get_count(with_schema("osm2pgsql_test_line", options)));
}

TEMPLATE_TEST_CASE("ways as linestrings and polygons", "", options_slim_default,
options_slim_expire)
options_slim_expire, options_slim_schema)
{
options_t options = TestType::options();

Expand All @@ -145,10 +188,11 @@ TEMPLATE_TEST_CASE("ways as linestrings and polygons", "", options_slim_default,

auto conn = db.db().connect();

REQUIRE(0 == conn.get_count("osm2pgsql_test_point"));
REQUIRE(0 == conn.get_count("osm2pgsql_test_line"));
REQUIRE(1 == conn.get_count("osm2pgsql_test_polygon"));
REQUIRE(1 == conn.get_count("osm2pgsql_test_polygon",
REQUIRE(0 == conn.get_count(with_schema("osm2pgsql_test_point", options)));
REQUIRE(0 == conn.get_count(with_schema("osm2pgsql_test_line", options)));
REQUIRE(1 ==
conn.get_count(with_schema("osm2pgsql_test_polygon", options)));
REQUIRE(1 == conn.get_count(with_schema("osm2pgsql_test_polygon", options),
"osm_id = 20 AND tags->'building' = 'yes' AND "
"ST_GeometryType(geom) = 'ST_Polygon'"));

Expand All @@ -157,48 +201,52 @@ TEMPLATE_TEST_CASE("ways as linestrings and polygons", "", options_slim_default,
REQUIRE_NOTHROW(db.run_import(
options, "w20 v2 dV Thighway=secondary Nn10,n11,n12,n13,n10\n"));

REQUIRE(0 == conn.get_count("osm2pgsql_test_point"));
REQUIRE(1 == conn.get_count("osm2pgsql_test_line"));
REQUIRE(0 == conn.get_count(with_schema("osm2pgsql_test_point", options)));
REQUIRE(1 == conn.get_count(with_schema("osm2pgsql_test_line", options)));
REQUIRE(1 ==
conn.get_count("osm2pgsql_test_line",
conn.get_count(with_schema("osm2pgsql_test_line", options),
"osm_id = 20 AND tags->'highway' = 'secondary' AND "
"ST_GeometryType(geom) = 'ST_LineString'"));
REQUIRE(0 == conn.get_count("osm2pgsql_test_polygon"));
REQUIRE(0 ==
conn.get_count(with_schema("osm2pgsql_test_polygon", options)));

// now remove a node from the way...
REQUIRE_NOTHROW(db.run_import(
options, "w20 v3 dV Thighway=secondary Nn10,n11,n12,n13\n"));

REQUIRE(0 == conn.get_count("osm2pgsql_test_point"));
REQUIRE(1 == conn.get_count("osm2pgsql_test_line"));
REQUIRE(0 == conn.get_count(with_schema("osm2pgsql_test_point", options)));
REQUIRE(1 == conn.get_count(with_schema("osm2pgsql_test_line", options)));
REQUIRE(1 ==
conn.get_count("osm2pgsql_test_line",
conn.get_count(with_schema("osm2pgsql_test_line", options),
"osm_id = 20 AND tags->'highway' = 'secondary' AND "
"ST_GeometryType(geom) = 'ST_LineString'"));
REQUIRE(0 == conn.get_count("osm2pgsql_test_polygon"));
REQUIRE(0 ==
conn.get_count(with_schema("osm2pgsql_test_polygon", options)));

// now change the tag back to an area tag (but the way is not closed)...
REQUIRE_NOTHROW(
db.run_import(options, "w20 v4 dV Tbuilding=yes Nn10,n11,n12,n13\n"));

REQUIRE(0 == conn.get_count("osm2pgsql_test_point"));
REQUIRE(0 == conn.get_count("osm2pgsql_test_line"));
REQUIRE(0 == conn.get_count("osm2pgsql_test_polygon"));
REQUIRE(0 == conn.get_count(with_schema("osm2pgsql_test_point", options)));
REQUIRE(0 == conn.get_count(with_schema("osm2pgsql_test_line", options)));
REQUIRE(0 ==
conn.get_count(with_schema("osm2pgsql_test_polygon", options)));

// now close the way again
REQUIRE_NOTHROW(db.run_import(
options, "w20 v5 dV Tbuilding=yes Nn10,n11,n12,n13,n10\n"));

REQUIRE(0 == conn.get_count("osm2pgsql_test_point"));
REQUIRE(0 == conn.get_count("osm2pgsql_test_line"));
REQUIRE(1 == conn.get_count("osm2pgsql_test_polygon"));
REQUIRE(1 == conn.get_count("osm2pgsql_test_polygon",
REQUIRE(0 == conn.get_count(with_schema("osm2pgsql_test_point", options)));
REQUIRE(0 == conn.get_count(with_schema("osm2pgsql_test_line", options)));
REQUIRE(1 ==
conn.get_count(with_schema("osm2pgsql_test_polygon", options)));
REQUIRE(1 == conn.get_count(with_schema("osm2pgsql_test_polygon", options),
"osm_id = 20 AND tags->'building' = 'yes' AND "
"ST_GeometryType(geom) = 'ST_Polygon'"));
}

TEMPLATE_TEST_CASE("multipolygons", "", options_slim_default,
options_slim_expire)
options_slim_expire, options_slim_schema)
{
options_t options = TestType::options();

Expand All @@ -213,10 +261,11 @@ TEMPLATE_TEST_CASE("multipolygons", "", options_slim_default,

auto conn = db.db().connect();

REQUIRE(0 == conn.get_count("osm2pgsql_test_point"));
REQUIRE(0 == conn.get_count("osm2pgsql_test_line"));
REQUIRE(1 == conn.get_count("osm2pgsql_test_polygon"));
REQUIRE(1 == conn.get_count("osm2pgsql_test_polygon",
REQUIRE(0 == conn.get_count(with_schema("osm2pgsql_test_point", options)));
REQUIRE(0 == conn.get_count(with_schema("osm2pgsql_test_line", options)));
REQUIRE(1 ==
conn.get_count(with_schema("osm2pgsql_test_polygon", options)));
REQUIRE(1 == conn.get_count(with_schema("osm2pgsql_test_polygon", options),
"osm_id = -30 AND tags->'building' = 'yes' AND "
"ST_GeometryType(geom) = 'ST_Polygon'"));

Expand All @@ -226,10 +275,11 @@ TEMPLATE_TEST_CASE("multipolygons", "", options_slim_default,
options,
"r30 v2 dV Ttype=multipolygon,building=yes,name=Shed Mw20@\n"));

REQUIRE(0 == conn.get_count("osm2pgsql_test_point"));
REQUIRE(0 == conn.get_count("osm2pgsql_test_line"));
REQUIRE(1 == conn.get_count("osm2pgsql_test_polygon"));
REQUIRE(1 == conn.get_count("osm2pgsql_test_polygon",
REQUIRE(0 == conn.get_count(with_schema("osm2pgsql_test_point", options)));
REQUIRE(0 == conn.get_count(with_schema("osm2pgsql_test_line", options)));
REQUIRE(1 ==
conn.get_count(with_schema("osm2pgsql_test_polygon", options)));
REQUIRE(1 == conn.get_count(with_schema("osm2pgsql_test_polygon", options),
"osm_id = -30 AND tags->'building' = 'yes' AND "
"ST_GeometryType(geom) = 'ST_Polygon'"));

Expand All @@ -244,7 +294,8 @@ TEMPLATE_TEST_CASE("multipolygons", "", options_slim_default,
options, "r30 v3 dV Tbuilding=yes,name=Shed Mw20@\n"));
}

REQUIRE(0 == conn.get_count("osm2pgsql_test_point"));
REQUIRE(0 == conn.get_count("osm2pgsql_test_line"));
REQUIRE(0 == conn.get_count("osm2pgsql_test_polygon"));
REQUIRE(0 == conn.get_count(with_schema("osm2pgsql_test_point", options)));
REQUIRE(0 == conn.get_count(with_schema("osm2pgsql_test_line", options)));
REQUIRE(0 ==
conn.get_count(with_schema("osm2pgsql_test_polygon", options)));
}
Loading