[Tile-serving] [osm2pgsql-dev/osm2pgsql] Add callbacks for delete actions (PR #2344)

[Jochen Topf]

@joto requested changes on this pull request.



> @@ -458,6 +470,8 @@ output_pgsql_t::output_pgsql_t(std::shared_ptr<middle_query_t> const &mid,
 
     m_enable_way_area = read_style_file(options.style, &exlist);
 
+    m_ignore_untagged_objects = !options.extra_attributes;
+

Should be initialized with the other members above.

> +
+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();

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.

> @@ -1084,6 +1084,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;

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.)

> +                    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

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.

> @@ -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{

Use `local`

> @@ -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)

Confusing that this function is called `process` like the process callbacks. Also should be `local`.

> @@ -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' }

`type = 'int'` should work fine.

> +            """
+        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|

*mode superpicky on* Spaces at the end missing in this and following lines.

> +            | --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' }

see above

-- 
Reply to this email directly or view it on GitHub:
https://github.com/osm2pgsql-dev/osm2pgsql/pull/2344#pullrequestreview-2912611398
You are receiving this because you are subscribed to this thread.

Message ID: <osm2pgsql-dev/osm2pgsql/pull/2344/review/2912611398 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/tile-serving/attachments/20250610/374848bf/attachment.htm>