Skip to content

Add callbacks for delete actions #2344

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Add callbacks for delete actions #2344

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1ee13a9
move special handling of untagged objects into pgsql output
lonvia Jun 5, 2025
b4e26fd
add callbacks for deletion to flex
lonvia Jun 5, 2025
a68ddd0
disable tags, nodes, members and geometries for deleted objecs
lonvia Jun 9, 2025
b9d3b28
add tests for delete callback
lonvia Jun 9, 2025
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
37 changes: 10 additions & 27 deletions src/osmdata.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,7 @@ osmdata_t::osmdata_t(std::shared_ptr<middle_t> 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);
Expand All @@ -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
Expand All @@ -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);
}
}
Expand All @@ -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
Expand All @@ -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);
}
}
Expand Down Expand Up @@ -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);
}
}
Expand Down
1 change: 0 additions & 1 deletion src/osmdata.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
109 changes: 76 additions & 33 deletions src/output-flex.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<osmium::Way const &>(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<osmium::Relation const &>(object);
luaX_add_table_array(
lua_state, "members", relation.members(),
[&](osmium::RelationMember const &member) {
lua_createtable(lua_state, 0, 3);
std::array<char, 2> 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<osmium::Way const &>(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<osmium::Relation const &>(object);
luaX_add_table_array(
lua_state, "members", relation.members(),
[&](osmium::RelationMember const &member) {
lua_createtable(lua_state, 0, 3);
std::array<char, 2> 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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is weird that the process callbacks get a "real Lua object", and the delete callbacks only get a "Lua table" as parameter. This might have all sorts of consequences later, no sure. It definitely prevents us from ever calling methods on deleted OSM objects.

lua_pushstring(lua_state, OSM2PGSQL_OSMOBJECT_CLASS);
lua_gettable(lua_state, LUA_REGISTRYINDEX);
lua_setmetatable(lua_state, -2);
}
}

/**
Expand Down Expand Up @@ -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;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are reusing an existing context. This is fine, but the comments in src/output-flex.hpp for the calling_context enum referring to the process_* callbacks should be changed, too. (We probably already should have changed them when we added the untagged callbacks.)

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();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deleted relations have no members, so there is no way of selecting any of them for two-stage processing. So this line can be removed.

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. */
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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};
Expand Down
14 changes: 11 additions & 3 deletions src/output-flex.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions src/output-null.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading