[Tile-serving] [openstreetmap/osm2pgsql] Fix deletion on update in gazetteer (#1008)

[Jochen Topf]

joto requested changes on this pull request.



>  {
-    assert(!m_inflight);
-
-    std::string sql = "DELETE FROM ";
-    sql.reserve(buffer->target->name.size() + buffer->deletables.size() * 15 +
-                30);
-    sql += buffer->target->name;
-    sql += " WHERE ";
-    sql += buffer->target->id;
-    sql += " IN (";
-    for (auto id : buffer->deletables) {
-        sql += std::to_string(id);
+    std::string sql = "DELETE FROM {} WHERE {} IN ("_format(table, column);
+    sql.reserve(m_deletables.size() * 15 + sql.size());

Unexplained magic value 15 / unexplained calculation.

> -    assert(!m_inflight);
-
-    std::string sql = "DELETE FROM ";
-    sql.reserve(buffer->target->name.size() + buffer->deletables.size() * 15 +
-                30);
-    sql += buffer->target->name;
-    sql += " WHERE ";
-    sql += buffer->target->id;
-    sql += " IN (";
-    for (auto id : buffer->deletables) {
-        sql += std::to_string(id);
+    std::string sql = "DELETE FROM {} WHERE {} IN ("_format(table, column);
+    sql.reserve(m_deletables.size() * 15 + sql.size());
+
+    for (auto id : m_deletables) {
+        sql += fmt::to_string(id);

This will create a string for each id and then append it. Consider using `fmt::basic_memory_buffer` or `fmt::format_to` with `back_inserter`.

>  
-            return true;
+    for (auto const &m : m_main) {
+        bool do_include = true;
+        if (std::get<2>(m) & SF_MAIN_NAMED) {

Why the `do_include`? What's up with the `XXX`?

>  
-        return false;
-    });
+        if (do_include) {
+            l += "'{}',"_format(std::get<0>(m));
+        }
+    }
+
+    if (!l.empty())

{}

>      int process_node(osmium::Node const &node);
     int process_way(osmium::Way *way);
     int process_relation(osmium::Relation const &rel);
     void connect();
 
-    void prepare_query_conn() const;
-
     void delete_unused_full(char const *osm_type, osmid_t osm_id)
     {

Why is the parameter here a `char const *`, but `delete_object()` uses a `char`?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/osm2pgsql/pull/1008#pullrequestreview-324923722
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/tile-serving/attachments/20191130/e9b3b4e2/attachment.html>