Find node dependencies faster by doing a single lookup

For each changed node osm2pgsql has to find all ways and all relations
referencing that node and make sure they are updated as well. This was
done by doing one SQL query per node which is quite inefficient. Instead
the new code collects all node ids and does only one query for the
parent ways and one for the parent relations after all nodes have been
read.

This implementation uses temporary tables. The changed nodes are written
to a temp table and from that temp tables are filled with the parent
nodes and ways. Strictly speaking the latter tables are not necessary,
we could read the data directly from the query. But maybe those tables
could be useful in the future for some additional processing in the
database. (They'd need to be converted to non-temp tables then, though.)

Author: joto

src/dependency-manager.cpp

@@ -15,14 +15,7 @@
 
 void full_dependency_manager_t::node_changed(osmid_t id)
 {
-    for (auto const way_id : m_object_store->get_ways_by_node(id)) {
-        way_changed(way_id);
-        m_ways_pending_tracker.set(way_id);
-    }
-
-    for (auto const rel_id : m_object_store->get_rels_by_node(id)) {
-        m_rels_pending_tracker.set(rel_id);
-    }
+    m_changed_nodes.set(id);
 }
 
 void full_dependency_manager_t::way_changed(osmid_t id)
@@ -36,6 +29,23 @@ void full_dependency_manager_t::way_changed(osmid_t id)
     }
 }
 
+void full_dependency_manager_t::after_nodes()
+{
+    if (m_changed_nodes.empty()) {
+        return;
+    }
+
+    m_object_store->get_node_parents(m_changed_nodes, &m_ways_pending_tracker,
+                                     &m_rels_pending_tracker);
+    m_changed_nodes.clear();
+
+    for (auto const way_id : m_ways_pending_tracker) {
+        for (auto const rel_id : m_object_store->get_rels_by_way(way_id)) {
+            m_rels_pending_tracker.set(rel_id);
+        }
+    }
+}
+
 bool full_dependency_manager_t::has_pending() const noexcept
 {
     return !m_ways_pending_tracker.empty() || !m_rels_pending_tracker.empty();

src/dependency-manager.hpp

@@ -52,6 +52,8 @@ class dependency_manager_t
      */
     virtual void way_changed(osmid_t) {}
 
+    virtual void after_nodes() {}
+
     /// Are there pending objects that need to be processed?
     virtual bool has_pending() const noexcept { return false; }
 
@@ -95,6 +97,8 @@ class full_dependency_manager_t : public dependency_manager_t
     void node_changed(osmid_t id) override;
     void way_changed(osmid_t id) override;
 
+    void after_nodes() override;
+
     bool has_pending() const noexcept override;
 
     idlist_t get_pending_way_ids() override
@@ -112,6 +116,16 @@ class full_dependency_manager_t : public dependency_manager_t
 
     std::shared_ptr<middle_t> m_object_store;
 
+    /**
+     * In append mode all new and changed nodes will be added to this. After
+     * all nodes are read this is used to figure out which parent ways and
+     * relations reference these nodes. Deleted nodes are not stored in here,
+     * because all ways and relations that referenced deleted nodes must be in
+     * the change file, too, and so we don't have to find out which ones they
+     * are.
+     */
+    osmium::index::IdSetSmall<osmid_t> m_changed_nodes;
+
     osmium::index::IdSetSmall<osmid_t> m_ways_pending_tracker;
     osmium::index::IdSetSmall<osmid_t> m_rels_pending_tracker;
 };

src/middle-pgsql.cpp

@@ -18,6 +18,7 @@
 #include <cassert>
 #include <cstdint>
 #include <cstdlib>
+#include <iterator>
 #include <memory>
 #include <stdexcept>
 #include <string>
@@ -41,6 +42,43 @@
 #include "pgsql-helper.hpp"
 #include "util.hpp"
 
+static bool check_bucket_index(pg_conn_t const *db_connection,
+                               std::string const &prefix)
+{
+    auto const res =
+        db_connection->exec("SELECT relname FROM pg_class"
+                            " WHERE relkind='i'"
+                            " AND relname = '{}_ways_nodes_bucket_idx'",
+                            prefix);
+    return res.num_tuples() > 0;
+}
+
+static void send_id_list(pg_conn_t const &db_connection,
+                         std::string const &table,
+                         osmium::index::IdSetSmall<osmid_t> const &ids)
+{
+    std::string data;
+    for (auto const id : ids) {
+        fmt::format_to(std::back_inserter(data), FMT_STRING("{}\n"), id);
+    }
+
+    auto const sql = fmt::format("COPY {} FROM STDIN", table);
+    db_connection.copy_start(sql.c_str());
+    db_connection.copy_send(data, table);
+    db_connection.copy_end(table);
+}
+
+static void load_id_list(pg_conn_t const &db_connection,
+                         std::string const &table,
+                         osmium::index::IdSetSmall<osmid_t> *ids)
+{
+    auto const res =
+        db_connection.exec(fmt::format("SELECT id FROM {} ORDER BY id", table));
+    for (int n = 0; n < res.num_tuples(); ++n) {
+        ids->set(osmium::string_to_object_id(res.get_value(n, 0)));
+    }
+}
+
 static std::string build_sql(options_t const &options, std::string const &templ)
 {
     std::string const using_tablespace{options.tblsslim_index.empty()
@@ -746,14 +784,79 @@ void middle_pgsql_t::node_delete(osmid_t osm_id)
     }
 }
 
-idlist_t middle_pgsql_t::get_ways_by_node(osmid_t osm_id)
+void middle_pgsql_t::get_node_parents(
+    osmium::index::IdSetSmall<osmid_t> const &changed_nodes,
+    osmium::index::IdSetSmall<osmid_t> *parent_ways,
+    osmium::index::IdSetSmall<osmid_t> *parent_relations) const
 {
-    return get_ids_from_db(&m_db_connection, "mark_ways_by_node", osm_id);
-}
+    util::timer_t timer;
 
-idlist_t middle_pgsql_t::get_rels_by_node(osmid_t osm_id)
-{
-    return get_ids_from_db(&m_db_connection, "mark_rels_by_node", osm_id);
+    m_db_connection.exec("BEGIN");
+    m_db_connection.exec("CREATE TEMP TABLE osm2pgsql_changed_nodes"
+                         " (id int8 NOT NULL) ON COMMIT DROP");
+    m_db_connection.exec("CREATE TEMP TABLE osm2pgsql_changed_ways"
+                         " (id int8 NOT NULL) ON COMMIT DROP");
+    m_db_connection.exec("CREATE TEMP TABLE osm2pgsql_changed_relations"
+                         " (id int8 NOT NULL) ON COMMIT DROP");
+
+    send_id_list(m_db_connection, "osm2pgsql_changed_nodes", changed_nodes);
+
+    m_db_connection.exec("ANALYZE osm2pgsql_changed_nodes");
+
+    bool const has_bucket_index =
+        check_bucket_index(&m_db_connection, m_options->prefix);
+
+    if (has_bucket_index) {
+        m_db_connection.exec(build_sql(*m_options, R"(
+WITH changed_buckets AS (
+  SELECT array_agg(id) AS node_ids, id >> {way_node_index_id_shift} AS bucket
+    FROM osm2pgsql_changed_nodes GROUP BY id >> {way_node_index_id_shift}
+)
+INSERT INTO osm2pgsql_changed_ways
+  SELECT DISTINCT w.id
+    FROM {schema}"{prefix}_ways" w, changed_buckets b
+    WHERE w.nodes && b.node_ids
+      AND {schema}"{prefix}_index_bucket"(w.nodes)
+       && ARRAY[b.bucket];
+        )"));
+    } else {
+        m_db_connection.exec(build_sql(*m_options, R"(
+INSERT INTO osm2pgsql_changed_ways
+  SELECT DISTINCT w.id
+    FROM {schema}"{prefix}_ways" w, osm2pgsql_changed_nodes n
+    WHERE w.nodes && ARRAY[n.id]
+        )"));
+    }
+
+    if (m_options->middle_database_format == 1) {
+        m_db_connection.exec(build_sql(*m_options, R"(
+INSERT INTO osm2pgsql_changed_relations
+  SELECT DISTINCT r.id
+    FROM {schema}"{prefix}_rels" r, osm2pgsql_changed_nodes n
+    WHERE r.parts && ARRAY[n.id]
+      AND r.parts[1:way_off] && ARRAY[n.id]
+        )"));
+    } else {
+        m_db_connection.exec(build_sql(*m_options, R"(
+INSERT INTO osm2pgsql_changed_relations
+  SELECT DISTINCT r.id
+    FROM {schema}"{prefix}_rels" r, osm2pgsql_changed_nodes c
+    WHERE {schema}"{prefix}_member_ids"(r.members, 'N'::char) && ARRAY[c.id];
+        )"));
+    }
+
+    load_id_list(m_db_connection, "osm2pgsql_changed_ways", parent_ways);
+    load_id_list(m_db_connection, "osm2pgsql_changed_relations",
+                 parent_relations);
+
+    m_db_connection.exec("COMMIT");
+
+    timer.stop();
+
+    log_debug("Found {} new/changed nodes in input.", changed_nodes.size());
+    log_debug("  Found in {} their {} parent ways and {} parent relations.",
+              std::chrono::duration_cast<std::chrono::seconds>(timer.elapsed()),
+              parent_ways->size(), parent_relations->size());
 }
 
 idlist_t middle_pgsql_t::get_rels_by_way(osmid_t osm_id)
@@ -1279,8 +1382,7 @@ static table_sql sql_for_nodes_format2(middle_pgsql_options const &options)
     return sql;
 }
 
-static table_sql sql_for_ways_format1(bool has_bucket_index,
-                                      uint8_t way_node_index_id_shift)
+static table_sql sql_for_ways_format1(uint8_t way_node_index_id_shift)
 {
     table_sql sql{};
 
@@ -1300,20 +1402,6 @@ static table_sql sql_for_ways_format1(bool has_bucket_index,
                         "    FROM {schema}\"{prefix}_ways\""
                         "      WHERE id = ANY($1::int8[]);\n";
 
-    if (has_bucket_index) {
-        sql.prepare_fw_dep_lookups =
-            "PREPARE mark_ways_by_node(int8) AS"
-            "  SELECT id FROM {schema}\"{prefix}_ways\" w"
-            "    WHERE $1 = ANY(nodes)"
-            "      AND {schema}\"{prefix}_index_bucket\"(w.nodes)"
-            "       && {schema}\"{prefix}_index_bucket\"(ARRAY[$1]);\n";
-    } else {
-        sql.prepare_fw_dep_lookups =
-            "PREPARE mark_ways_by_node(int8) AS"
-            "  SELECT id FROM {schema}\"{prefix}_ways\""
-            "    WHERE nodes && ARRAY[$1];\n";
-    }
-
     if (way_node_index_id_shift == 0) {
         sql.create_fw_dep_indexes =
             "CREATE INDEX ON {schema}\"{prefix}_ways\" USING GIN (nodes)"
@@ -1335,8 +1423,7 @@ static table_sql sql_for_ways_format1(bool has_bucket_index,
     return sql;
 }
 
-static table_sql sql_for_ways_format2(bool has_bucket_index,
-                                      middle_pgsql_options const &options)
+static table_sql sql_for_ways_format2(middle_pgsql_options const &options)
 {
     table_sql sql{};
 
@@ -1361,20 +1448,6 @@ static table_sql sql_for_ways_format2(bool has_bucket_index,
                         "  {users_table_access}"
                          " WHERE o.id = ANY($1::int8[]);\n";
 
-    if (has_bucket_index) {
-        sql.prepare_fw_dep_lookups =
-            "PREPARE mark_ways_by_node(int8) AS"
-            "  SELECT DISTINCT id FROM {schema}\"{prefix}_ways\" w"
-            "    WHERE $1 = ANY(nodes)"
-            "      AND {schema}\"{prefix}_index_bucket\"(w.nodes)"
-            "       && {schema}\"{prefix}_index_bucket\"(ARRAY[$1]);\n";
-    } else {
-        sql.prepare_fw_dep_lookups =
-            "PREPARE mark_ways_by_node(int8) AS"
-            "  SELECT DISTINCT id FROM {schema}\"{prefix}_ways\""
-            "    WHERE nodes && ARRAY[$1];\n";
-    }
-
     if (options.way_node_index_id_shift == 0) {
         sql.create_fw_dep_indexes =
             "CREATE INDEX ON {schema}\"{prefix}_ways\" USING GIN (nodes)"
@@ -1416,10 +1489,6 @@ static table_sql sql_for_relations_format1()
                         "    FROM {schema}\"{prefix}_rels\" WHERE id = $1;\n";
 
     sql.prepare_fw_dep_lookups =
-        "PREPARE mark_rels_by_node(int8) AS"
-        "  SELECT id FROM {schema}\"{prefix}_rels\""
-        "    WHERE parts && ARRAY[$1]"
-        "      AND parts[1:way_off] && ARRAY[$1];\n"
         "PREPARE mark_rels_by_way(int8) AS"
         "  SELECT id FROM {schema}\"{prefix}_rels\""
         "    WHERE parts && ARRAY[$1]"
@@ -1460,10 +1529,6 @@ static table_sql sql_for_relations_format2()
                         " WHERE o.id = $1;\n";
 
     sql.prepare_fw_dep_lookups =
-        "PREPARE mark_rels_by_node(int8) AS"
-        "  SELECT DISTINCT id FROM {schema}\"{prefix}_rels\""
-        "    WHERE {schema}\"{prefix}_member_ids\"(members, 'N'::char)"
-        "      @> ARRAY[$1::bigint];\n"
         "PREPARE mark_rels_by_way(int8) AS"
         "  SELECT DISTINCT id FROM {schema}\"{prefix}_rels\""
         "    WHERE {schema}\"{prefix}_member_ids\"(members, 'W'::char)"
@@ -1482,17 +1547,6 @@ static table_sql sql_for_relations_format2()
     return sql;
 }
 
-static bool check_bucket_index(pg_conn_t *db_connection,
-                               std::string const &prefix)
-{
-    auto const res =
-        db_connection->exec("SELECT relname FROM pg_class"
-                            " WHERE relkind='i'"
-                            " AND relname = '{}_ways_nodes_bucket_idx'",
-                            prefix);
-    return res.num_tuples() > 0;
-}
-
 middle_pgsql_t::middle_pgsql_t(std::shared_ptr<thread_pool_t> thread_pool,
                                options_t const *options)
 : middle_t(std::move(thread_pool)), m_options(options),
@@ -1537,17 +1591,16 @@ middle_pgsql_t::middle_pgsql_t(std::shared_ptr<thread_pool_t> thread_pool,
 
         m_tables.ways() = table_desc{
             *options,
-            sql_for_ways_format1(has_bucket_index,
-                                 m_store_options.way_node_index_id_shift)};
+            sql_for_ways_format1(m_store_options.way_node_index_id_shift)};
 
         m_tables.relations() =
             table_desc{*options, sql_for_relations_format1()};
     } else {
         m_tables.nodes() =
             table_desc{*options, sql_for_nodes_format2(m_store_options)};
 
-        m_tables.ways() = table_desc{
-            *options, sql_for_ways_format2(has_bucket_index, m_store_options)};
+        m_tables.ways() =
+            table_desc{*options, sql_for_ways_format2(m_store_options)};
 
         m_tables.relations() =
             table_desc{*options, sql_for_relations_format2()};

src/middle-pgsql.hpp

@@ -117,8 +117,11 @@ struct middle_pgsql_t : public middle_t
     void after_ways() override;
     void after_relations() override;
 
-    idlist_t get_ways_by_node(osmid_t osm_id) override;
-    idlist_t get_rels_by_node(osmid_t osm_id) override;
+    void get_node_parents(
+        osmium::index::IdSetSmall<osmid_t> const &changed_nodes,
+        osmium::index::IdSetSmall<osmid_t> *parent_ways,
+        osmium::index::IdSetSmall<osmid_t> *parent_relations) const override;
+
     idlist_t get_rels_by_way(osmid_t osm_id) override;
 
     class table_desc

src/middle.hpp

@@ -10,6 +10,7 @@
  * For a full list of authors see the git log.
  */
 
+#include <osmium/index/id_set.hpp>
 #include <osmium/memory/buffer.hpp>
 #include <osmium/osm/entity_bits.hpp>
 
@@ -148,8 +149,13 @@ class middle_t
 #endif
     }
 
-    virtual idlist_t get_ways_by_node(osmid_t) { return {}; }
-    virtual idlist_t get_rels_by_node(osmid_t) { return {}; }
+    virtual void get_node_parents(
+        osmium::index::IdSetSmall<osmid_t> const & /*changed_nodes*/,
+        osmium::index::IdSetSmall<osmid_t> * /*parent_ways*/,
+        osmium::index::IdSetSmall<osmid_t> * /*parent_relations*/) const
+    {
+    }
+
     virtual idlist_t get_rels_by_way(osmid_t) { return {}; }
 
     virtual std::shared_ptr<middle_query_t> get_query_instance() = 0;

src/osmdata.cpp

@@ -78,6 +78,9 @@ void osmdata_t::after_nodes()
 {
     m_mid->after_nodes();
     m_output->after_nodes();
+    if (m_append) {
+        m_dependency_manager->after_nodes();
+    }
 }
 
 void osmdata_t::way(osmium::Way &way)