diff --git a/src/osmdata.cpp b/src/osmdata.cpp index 82dc262db..1bfb556a2 100644 --- a/src/osmdata.cpp +++ b/src/osmdata.cpp @@ -31,9 +31,7 @@ osmdata_t::osmdata_t(std::shared_ptr mid, : m_mid(std::move(mid)), m_output(std::move(output)), m_connection_params(options.connection_params), 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 || - options.output_backend == "flex") + m_droptemp(options.droptemp) { assert(m_mid); assert(m_output); @@ -56,17 +54,12 @@ void osmdata_t::node(osmium::Node const &node) m_mid->node(node); if (node.deleted()) { - m_output->node_delete(node.id()); + m_output->node_delete(node); return; } - bool const has_tags_or_attrs = m_with_extra_attrs || !node.tags().empty(); if (m_append) { - if (has_tags_or_attrs) { - m_output->node_modify(node); - } else { - m_output->node_delete(node.id()); - } + m_output->node_modify(node); // Version 1 means this is a new node, so there can't be an existing // way or relation referencing it, so we don't have to add that node @@ -75,7 +68,7 @@ void osmdata_t::node(osmium::Node const &node) if (node.version() != 1) { m_changed_nodes.push_back(node.id()); } - } else if (has_tags_or_attrs) { + } else { m_output->node_add(node); } } @@ -101,17 +94,12 @@ void osmdata_t::way(osmium::Way &way) m_mid->way(way); if (way.deleted()) { - m_output->way_delete(way.id()); + m_output->way_delete(&way); return; } - bool const has_tags_or_attrs = m_with_extra_attrs || !way.tags().empty(); if (m_append) { - if (has_tags_or_attrs) { - m_output->way_modify(&way); - } else { - m_output->way_delete(way.id()); - } + m_output->way_modify(&way); // Version 1 means this is a new way, so there can't be an existing // relation referencing it, so we don't have to add that way to the @@ -120,7 +108,7 @@ void osmdata_t::way(osmium::Way &way) if (way.version() != 1) { m_changed_ways.push_back(way.id()); } - } else if (has_tags_or_attrs) { + } else { m_output->way_add(&way); } } @@ -173,19 +161,14 @@ void osmdata_t::relation(osmium::Relation const &rel) m_mid->relation(rel); if (rel.deleted()) { - m_output->relation_delete(rel.id()); + m_output->relation_delete(rel); return; } - bool const has_tags_or_attrs = m_with_extra_attrs || !rel.tags().empty(); if (m_append) { - if (has_tags_or_attrs) { - m_output->relation_modify(rel); - } else { - m_output->relation_delete(rel.id()); - } + m_output->relation_modify(rel); m_changed_relations.push_back(rel.id()); - } else if (has_tags_or_attrs) { + } else { m_output->relation_add(rel); } } diff --git a/src/osmdata.hpp b/src/osmdata.hpp index dcbe74b12..4054222b0 100644 --- a/src/osmdata.hpp +++ b/src/osmdata.hpp @@ -117,7 +117,6 @@ class osmdata_t : public osmium::handler::Handler unsigned int m_num_procs; bool m_append; bool m_droptemp; - bool m_with_extra_attrs; }; #endif // OSM2PGSQL_OSMDATA_HPP diff --git a/src/output-flex.cpp b/src/output-flex.cpp index bd12a6248..fd3b10dae 100644 --- a/src/output-flex.cpp +++ b/src/output-flex.cpp @@ -158,39 +158,41 @@ void push_osm_object_to_lua_stack(lua_State *lua_state, luaX_add_table_str(lua_state, "user", object.user()); } - if (object.type() == osmium::item_type::way) { - auto const &way = static_cast(object); - luaX_add_table_bool(lua_state, "is_closed", - !way.nodes().empty() && way.is_closed()); - luaX_add_table_array(lua_state, "nodes", way.nodes(), - [&](osmium::NodeRef const &wn) { - lua_pushinteger(lua_state, wn.ref()); - }); - } else if (object.type() == osmium::item_type::relation) { - auto const &relation = static_cast(object); - luaX_add_table_array( - lua_state, "members", relation.members(), - [&](osmium::RelationMember const &member) { - lua_createtable(lua_state, 0, 3); - std::array tmp{"x"}; - tmp[0] = osmium::item_type_to_char(member.type()); - luaX_add_table_str(lua_state, "type", tmp.data()); - luaX_add_table_int(lua_state, "ref", member.ref()); - luaX_add_table_str(lua_state, "role", member.role()); - }); - } - - lua_pushliteral(lua_state, "tags"); - lua_createtable(lua_state, 0, (int)object.tags().size()); - for (auto const &tag : object.tags()) { - luaX_add_table_str(lua_state, tag.key(), tag.value()); - } - lua_rawset(lua_state, -3); - - // Set the metatable of this object - lua_pushstring(lua_state, OSM2PGSQL_OSMOBJECT_CLASS); - lua_gettable(lua_state, LUA_REGISTRYINDEX); - lua_setmetatable(lua_state, -2); + if (!object.deleted()) { + if (object.type() == osmium::item_type::way) { + auto const &way = static_cast(object); + luaX_add_table_bool(lua_state, "is_closed", + !way.nodes().empty() && way.is_closed()); + luaX_add_table_array(lua_state, "nodes", way.nodes(), + [&](osmium::NodeRef const &wn) { + lua_pushinteger(lua_state, wn.ref()); + }); + } else if (object.type() == osmium::item_type::relation) { + auto const &relation = static_cast(object); + luaX_add_table_array( + lua_state, "members", relation.members(), + [&](osmium::RelationMember const &member) { + lua_createtable(lua_state, 0, 3); + std::array tmp{"x"}; + tmp[0] = osmium::item_type_to_char(member.type()); + luaX_add_table_str(lua_state, "type", tmp.data()); + luaX_add_table_int(lua_state, "ref", member.ref()); + luaX_add_table_str(lua_state, "role", member.role()); + }); + } + + lua_pushliteral(lua_state, "tags"); + lua_createtable(lua_state, 0, (int)object.tags().size()); + for (auto const &tag : object.tags()) { + luaX_add_table_str(lua_state, tag.key(), tag.value()); + } + lua_rawset(lua_state, -3); + + // Set the metatable of this object + lua_pushstring(lua_state, OSM2PGSQL_OSMOBJECT_CLASS); + lua_gettable(lua_state, LUA_REGISTRYINDEX); + lua_setmetatable(lua_state, -2); + } } /** @@ -1084,6 +1086,38 @@ void output_flex_t::delete_from_tables(osmium::item_type type, osmid_t osm_id) } } +void output_flex_t::node_delete(osmium::Node const &node) +{ + if (m_delete_node) { + m_context_node = &node; + get_mutex_and_call_lua_function(m_delete_node, node); + m_context_node = nullptr; + } + + node_delete(node.id()); +} + +void output_flex_t::way_delete(osmium::Way *way) +{ + if (m_delete_way) { + m_way_cache.init(way); + get_mutex_and_call_lua_function(m_delete_way, m_way_cache.get()); + } + + way_delete(way->id()); +} + +void output_flex_t::relation_delete(osmium::Relation const &rel) +{ + if (m_delete_relation) { + m_relation_cache.init(rel); + select_relation_members(); + get_mutex_and_call_lua_function(m_delete_relation, rel); + } + + relation_delete(rel.id()); +} + /* Delete is easy, just remove all traces of this object. We don't need to * worry about finding objects that depend on it, since the same diff must * contain the change for that also. */ @@ -1146,6 +1180,8 @@ output_flex_t::output_flex_t(output_flex_t const *other, m_process_untagged_node(other->m_process_untagged_node), m_process_untagged_way(other->m_process_untagged_way), m_process_untagged_relation(other->m_process_untagged_relation), + m_delete_node(other->m_delete_node), m_delete_way(other->m_delete_way), + m_delete_relation(other->m_delete_relation), m_select_relation_members(other->m_select_relation_members), m_after_nodes(other->m_after_nodes), m_after_ways(other->m_after_ways), m_after_relations(other->m_after_relations) @@ -1330,6 +1366,13 @@ void output_flex_t::init_lua(std::string const &filename, prepared_lua_function_t{lua_state(), calling_context::process_relation, "process_untagged_relation"}; + m_delete_node = prepared_lua_function_t{ + lua_state(), calling_context::process_node, "delete_node"}; + m_delete_way = prepared_lua_function_t{ + lua_state(), calling_context::process_way, "delete_way"}; + m_delete_relation = prepared_lua_function_t{ + lua_state(), calling_context::process_relation, "delete_relation"}; + m_select_relation_members = prepared_lua_function_t{ lua_state(), calling_context::select_relation_members, "select_relation_members", 1}; diff --git a/src/output-flex.hpp b/src/output-flex.hpp index 23dceab08..c819c4b6d 100644 --- a/src/output-flex.hpp +++ b/src/output-flex.hpp @@ -147,9 +147,9 @@ class output_flex_t : public output_t void way_modify(osmium::Way *way) override; void relation_modify(osmium::Relation const &rel) override; - void node_delete(osmid_t id) override; - void way_delete(osmid_t id) override; - void relation_delete(osmid_t id) override; + void node_delete(osmium::Node const &node) override; + void way_delete(osmium::Way *way) override; + void relation_delete(osmium::Relation const &rel) override; void merge_expire_trees(output_t *other) override; @@ -204,6 +204,10 @@ class output_flex_t : public output_t osmium::OSMObject const & check_and_get_context_object(flex_table_t const &table); + void node_delete(osmid_t id); + void way_delete(osmid_t id); + void relation_delete(osmid_t id); + void delete_from_table(table_connection_t *table_connection, pg_conn_t const &db_connection, osmium::item_type type, osmid_t osm_id); @@ -304,6 +308,10 @@ class output_flex_t : public output_t prepared_lua_function_t m_process_untagged_way; prepared_lua_function_t m_process_untagged_relation; + prepared_lua_function_t m_delete_node; + prepared_lua_function_t m_delete_way; + prepared_lua_function_t m_delete_relation; + prepared_lua_function_t m_select_relation_members; prepared_lua_function_t m_after_nodes; diff --git a/src/output-null.hpp b/src/output-null.hpp index 75b93425c..a7ad82b50 100644 --- a/src/output-null.hpp +++ b/src/output-null.hpp @@ -50,9 +50,9 @@ class output_null_t : public output_t void way_modify(osmium::Way * /*way*/) override {} void relation_modify(osmium::Relation const & /*rel*/) override {} - void node_delete(osmid_t /*id*/) override {} - void way_delete(osmid_t /*id*/) override {} - void relation_delete(osmid_t /*id*/) override {} + void node_delete(osmium::Node const & /*node*/) override {} + void way_delete(osmium::Way * /*way*/) override {} + void relation_delete(osmium::Relation const & /*rel*/) override {} }; #endif // OSM2PGSQL_OUTPUT_NULL_HPP diff --git a/src/output-pgsql.cpp b/src/output-pgsql.cpp index e7c1023eb..46885d72e 100644 --- a/src/output-pgsql.cpp +++ b/src/output-pgsql.cpp @@ -206,6 +206,10 @@ void output_pgsql_t::wait() void output_pgsql_t::node_add(osmium::Node const &node) { + if (m_ignore_untagged_objects && node.tags().empty()) { + return; + } + taglist_t outtags; if (m_tagtransform->filter_tags(node, nullptr, nullptr, &outtags)) { return; @@ -219,6 +223,10 @@ void output_pgsql_t::node_add(osmium::Node const &node) void output_pgsql_t::way_add(osmium::Way *way) { + if (m_ignore_untagged_objects && way->tags().empty()) { + return; + } + bool polygon = false; bool roads = false; taglist_t outtags; @@ -317,6 +325,10 @@ void output_pgsql_t::pgsql_process_relation(osmium::Relation const &rel) void output_pgsql_t::relation_add(osmium::Relation const &rel) { + if (m_ignore_untagged_objects && rel.tags().empty()) { + return; + } + char const *const type = rel.tags()["type"]; /* Must have a type field or we ignore it */ @@ -337,15 +349,15 @@ void output_pgsql_t::relation_add(osmium::Relation const &rel) /* Delete is easy, just remove all traces of this object. We don't need to * worry about finding objects that depend on it, since the same diff must * contain the change for that also. */ -void output_pgsql_t::node_delete(osmid_t osm_id) +void output_pgsql_t::node_delete(osmium::Node const &node) { if (m_expire.enabled()) { - auto const results = m_tables[t_point]->get_wkb(osm_id); + auto const results = m_tables[t_point]->get_wkb(node.id()); if (expire_from_result(&m_expire, results, m_expire_config) != 0) { - m_tables[t_point]->delete_row(osm_id); + m_tables[t_point]->delete_row(node.id()); } } else { - m_tables[t_point]->delete_row(osm_id); + m_tables[t_point]->delete_row(node.id()); } } @@ -380,9 +392,9 @@ void output_pgsql_t::pgsql_delete_way_from_output(osmid_t osm_id) delete_from_output_and_expire(osm_id); } -void output_pgsql_t::way_delete(osmid_t osm_id) +void output_pgsql_t::way_delete(osmium::Way *way) { - pgsql_delete_way_from_output(osm_id); + pgsql_delete_way_from_output(way->id()); } /* Relations are identified by using negative IDs */ @@ -391,9 +403,9 @@ void output_pgsql_t::pgsql_delete_relation_from_output(osmid_t osm_id) delete_from_output_and_expire(-osm_id); } -void output_pgsql_t::relation_delete(osmid_t osm_id) +void output_pgsql_t::relation_delete(osmium::Relation const &rel) { - pgsql_delete_relation_from_output(osm_id); + pgsql_delete_relation_from_output(rel.id()); } /* Modify is slightly trickier. The basic idea is we simply delete the @@ -401,19 +413,19 @@ void output_pgsql_t::relation_delete(osmid_t osm_id) * objects that depend on this one */ void output_pgsql_t::node_modify(osmium::Node const &node) { - node_delete(node.id()); + node_delete(node); node_add(node); } void output_pgsql_t::way_modify(osmium::Way *way) { - way_delete(way->id()); + way_delete(way); way_add(way); } void output_pgsql_t::relation_modify(osmium::Relation const &rel) { - relation_delete(rel.id()); + relation_delete(rel); relation_add(rel); } @@ -458,6 +470,8 @@ output_pgsql_t::output_pgsql_t(std::shared_ptr const &mid, m_enable_way_area = read_style_file(options.style, &exlist); + m_ignore_untagged_objects = !options.extra_attributes; + m_tagtransform = tagtransform_t::make_tagtransform(&options, exlist); auto copy_thread = @@ -507,6 +521,7 @@ output_pgsql_t::output_pgsql_t( std::shared_ptr const ©_thread) : output_t(other, mid), m_tagtransform(other->m_tagtransform->clone()), m_enable_way_area(other->m_enable_way_area), + m_ignore_untagged_objects(other->m_ignore_untagged_objects), m_proj(get_options()->projection), m_expire_config(other->m_expire_config), m_expire(get_options()->expire_tiles_zoom, get_options()->projection), m_buffer(1024, osmium::memory::Buffer::auto_grow::yes), diff --git a/src/output-pgsql.hpp b/src/output-pgsql.hpp index b99b64eca..82aab120e 100644 --- a/src/output-pgsql.hpp +++ b/src/output-pgsql.hpp @@ -76,9 +76,9 @@ class output_pgsql_t : public output_t void way_modify(osmium::Way *way) override; void relation_modify(osmium::Relation const &rel) override; - void node_delete(osmid_t id) override; - void way_delete(osmid_t id) override; - void relation_delete(osmid_t id) override; + void node_delete(osmium::Node const &node) override; + void way_delete(osmium::Way *way) override; + void relation_delete(osmium::Relation const &rel) override; void merge_expire_trees(output_t *other) override; @@ -100,6 +100,8 @@ class output_pgsql_t : public output_t //enable output of a generated way_area tag to either hstore or its own column bool m_enable_way_area; + // handle objects without tags as if they were not there + bool m_ignore_untagged_objects; std::array, t_MAX> m_tables; diff --git a/src/output.hpp b/src/output.hpp index e6b75b402..3993a7585 100644 --- a/src/output.hpp +++ b/src/output.hpp @@ -98,9 +98,9 @@ class output_t virtual void way_modify(osmium::Way *way) = 0; virtual void relation_modify(osmium::Relation const &rel) = 0; - virtual void node_delete(osmid_t id) = 0; - virtual void way_delete(osmid_t id) = 0; - virtual void relation_delete(osmid_t id) = 0; + virtual void node_delete(osmium::Node const &node) = 0; + virtual void way_delete(osmium::Way *way) = 0; + virtual void relation_delete(osmium::Relation const &rel) = 0; virtual void merge_expire_trees(output_t * /*other*/) {} diff --git a/tests/bdd/flex/delete-callbacks.feature b/tests/bdd/flex/delete-callbacks.feature new file mode 100644 index 000000000..2121a4e33 --- /dev/null +++ b/tests/bdd/flex/delete-callbacks.feature @@ -0,0 +1,112 @@ +Feature: Test for delete callbacks + + Background: + Given the input file 'liechtenstein-2013-08-03.osm.pbf' + And the lua style + """ + change = osm2pgsql.define_table{ + name = 'change', + ids = {type = 'any', id_column = 'osm_id', type_column = 'osm_type'}, + columns = { + { column = 'extra', sql_type = 'int' } + }} + + function process(object) + change:insert{extra = object.version} + end + + osm2pgsql.delete_node = process + osm2pgsql.delete_way = process + osm2pgsql.delete_relation = process + """ + When running osm2pgsql flex with parameters + | --slim | + + Then table change contains exactly + | osm_type | osm_id | + + Given the input file '008-ch.osc.gz' + + Scenario: Delete callbacks are called + When running osm2pgsql flex with parameters + | --slim | -a | + + Then SELECT osm_type, count(*), sum(extra) FROM change GROUP BY osm_type + | osm_type | count | sum | + | N | 16773 | 16779| + | W | 4 | 9| + | R | 1 | 3| + + When running osm2pgsql flex with parameters + | --slim | -a | + + Then SELECT osm_type, count(*), sum(osm_id) FROM change GROUP BY osm_type + | osm_type | count | sum | + | N | 16773 | 37856781001834 | + | W | 4 | 350933407 | + | R | 1 | 2871571 | + + Scenario: No object payload is available + Given the lua style + """ + change = osm2pgsql.define_table{ + name = 'change', + ids = {type = 'any', id_column = 'osm_id', type_column = 'osm_type'}, + columns = { + { column = 'extra', sql_type = 'int' } + }} + + function osm2pgsql.delete_node(object) + change:insert{extra = object.tags == nil and 1 or 0} + end + + function osm2pgsql.delete_way(object) + change:insert{extra = object.nodes == nil and 1 or 0} + end + + function osm2pgsql.delete_relation(object) + change:insert{extra = object.members == nil and 1 or 0} + end + + + """ + When running osm2pgsql flex with parameters + | --slim | -a | + Then SELECT osm_type, count(*), sum(extra) FROM change GROUP BY osm_type + | osm_type | count | sum | + | N | 16773 | 16773 | + | W | 4 | 4 | + | R | 1 | 1 | + + + Scenario: Geometries are not valid for deleted objects + Given the lua style + """ + change = osm2pgsql.define_table{ + name = 'change', + ids = {type = 'any', id_column = 'osm_id', type_column = 'osm_type'}, + columns = { + { column = 'extra', sql_type = 'int' } + }} + + function osm2pgsql.delete_node(object) + change:insert{extra = object.as_point == nil and 1 or 0} + end + + function osm2pgsql.delete_way(object) + change:insert{extra = object.as_linestring == nil and 1 or 0} + end + + function osm2pgsql.delete_relation(object) + change:insert{extra = object.as_geometrycollection == nil and 1 or 0} + end + + + """ + When running osm2pgsql flex with parameters + | --slim | -a | + Then SELECT osm_type, count(*), sum(extra) FROM change GROUP BY osm_type + | osm_type | count | sum | + | N | 16773 | 16773| + | W | 4 | 4| + | R | 1 | 1| diff --git a/tests/test-osm-file-parsing.cpp b/tests/test-osm-file-parsing.cpp index 838b7fc80..712e95986 100644 --- a/tests/test-osm-file-parsing.cpp +++ b/tests/test-osm-file-parsing.cpp @@ -130,11 +130,11 @@ struct counting_output_t : public output_null_t ++relation.modified; } - void node_delete(osmid_t) override { ++node.deleted; } + void node_delete(osmium::Node const &) override { ++node.deleted; } - void way_delete(osmid_t) override { ++way.deleted; } + void way_delete(osmium::Way *) override { ++way.deleted; } - void relation_delete(osmid_t) override { ++relation.deleted; } + void relation_delete(osmium::Relation const &) override { ++relation.deleted; } type_stats_t node, way, relation; uint64_t sum_ids = 0; @@ -154,13 +154,13 @@ TEST_CASE("parse xml file") testing::parse_file(options, middle, output, "test_multipolygon.osm", false); - REQUIRE(output->sum_ids == 4728); - REQUIRE(output->sum_nds == 186); + REQUIRE(output->sum_ids == 73514); + REQUIRE(output->sum_nds == 495); REQUIRE(output->sum_members == 146); - REQUIRE(output->node.added == 0); + REQUIRE(output->node.added == 353); REQUIRE(output->node.modified == 0); REQUIRE(output->node.deleted == 0); - REQUIRE(output->way.added == 48); + REQUIRE(output->way.added == 140); REQUIRE(output->way.modified == 0); REQUIRE(output->way.deleted == 0); REQUIRE(output->relation.added == 40); @@ -188,8 +188,8 @@ TEST_CASE("parse diff file") testing::parse_file(options, middle, output, "008-ch.osc.gz", false); REQUIRE(output->node.added == 0); - REQUIRE(output->node.modified == 153); - REQUIRE(output->node.deleted == 17796); + REQUIRE(output->node.modified == 1176); + REQUIRE(output->node.deleted == 16773); REQUIRE(output->way.added == 0); REQUIRE(output->way.modified == 161); REQUIRE(output->way.deleted == 4);