Improve checks for data import

Author: rinigus

importer/src/hierarchy.cpp

@@ -114,43 +114,56 @@ void Hierarchy::finalize()
     }
 
   std::cout << "Hierarchy: active items: " << index - 1
-            << " / cleared items: " << m_items.size() - (index - 1) << "\n";
+            << " / cleared items: " << m_items.size() - (index - 1) << std::flush;
 }
 
-void Hierarchy::check_indexing()
+bool Hierarchy::check_indexing()
 {
+  std::cout << "Check whether all items are indexed\n";
+
+  m_index_check_failed.clear();
+  bool isok = true;
   for (auto itemp : m_items)
     {
       auto item = itemp.second;
-      if (item->keep() && !item->indexed())
+      if (item->keep(false) && !item->indexed())
         {
+          isok = false;
           std::cout << "\nItem is not included into hierarchy while it should be\n";
           item->print_item(0);
 
-          std::cout << "\nItem part of hierarchy (child -> parent):\n" << item->id() << " ";
-          auto i = item;
+          std::cout << "\nItem part of hierarchy (child -> parent):\n";
+
+          auto             i = item;
+          std::set<hindex> ids;
           for (auto parent = item->parent_id(); parent != 0; parent = i->parent_id())
             {
-              if (m_items.find(parent) != m_items.end())
+              if (m_items.find(parent) == m_items.end())
                 {
                   std::cout << "\nCannot find parent with ID " << parent << "\n";
                   break;
                 }
 
-              if (i->id() == item->id())
+              i = m_items[parent];
+              i->print_item(0);
+
+              if (ids.count(i->id()) > 0)
                 {
                   std::cout << "\nCyclic branch detected\n";
+                  m_index_check_failed.insert(ids.begin(), ids.end());
                   break;
                 }
-
-              i = m_items[parent];
-              std::cout << i->id() << " ";
+              ids.insert(i->id());
             }
           std::cout << "\n\n";
-
-          throw std::runtime_error("Item is not included into hierarchy while it should be");
         }
     }
+
+  if (isok)
+    std::cout << "Items indexing check passed\n";
+  else
+    std::cout << "Items indexing check FAILED\n";
+  return isok;
 }
 
 void Hierarchy::write(sqlite3pp::database &db) const

importer/src/hierarchy.h

@@ -23,7 +23,7 @@ class Hierarchy
   void set_country(const std::string &country, hindex id);
   void cleanup();
   void finalize();
-  void check_indexing();
+  bool check_indexing();
   void write(sqlite3pp::database &db) const;
 
   size_t get_missing_count() const { return m_root.size(); }
@@ -33,6 +33,8 @@ class Hierarchy
   hindex                get_next_nonzero_root_parent() const;
   std::set<std::string> get_root_countries() const;
 
+  std::set<hindex> get_failed_indexes() const { return m_index_check_failed; }
+
   void print(bool full = true) const;
   void print_root_with_parent_id(hindex parent_id) const;
 
@@ -43,6 +45,7 @@ class Hierarchy
   std::map<hindex, std::shared_ptr<HierarchyItem> >            m_items;
   std::map<hindex, std::set<std::shared_ptr<HierarchyItem> > > m_root;
   std::deque<std::shared_ptr<HierarchyItem> >                  m_root_finalized;
+  std::set<hindex>                                             m_index_check_failed;
 };
 
 #endif

importer/src/hierarchyitem.cpp

@@ -83,15 +83,26 @@ void HierarchyItem::load_skip_list(const std::string &fname)
   s_skip_types = load_list(fname);
 }
 
-bool HierarchyItem::keep() const
+void HierarchyItem::drop()
 {
+  m_dropped = true;
+}
+
+bool HierarchyItem::keep(bool verbose) const
+{
+  if (m_dropped)
+    return false;
+
   if (m_type.find_first_not_of(allowed_type_chars) != std::string::npos)
     {
-      std::cout << "Dropping " << m_type << "\n";
+      if (verbose)
+        std::cout << "Dropping " << m_type << "\n";
       return false;
     }
+
   if (s_skip_types.count(m_type) > 0)
     return false;
+
   return !m_name.empty() || s_priority_types.count(m_type) > 0;
 }
 
@@ -150,7 +161,7 @@ void HierarchyItem::set_parent(hindex parent, bool force)
   //     c->set_parent(m_id, force);
 }
 
-void HierarchyItem::cleanup_children(bool duplicate_only)
+void HierarchyItem::cleanup_children()
 {
   // as a result of this run, children that are supposed to be kept are staying in children
   // property. all disposed ones are still pointed to via Hierarchy map, but should not be accessed
@@ -193,10 +204,11 @@ void HierarchyItem::cleanup_children(bool duplicate_only)
                                   i->m_children.end());
           for (auto &i_children : i->m_children)
             i_children->set_parent(item->m_id, true);
+          i->drop();
         }
 
       if (had_duplicates)
-        item->cleanup_children(true);
+        item->cleanup_children();
 
       m_children = children;
     }
@@ -271,8 +283,7 @@ void HierarchyItem::print_item(unsigned int offset) const
   std::cout << std::string(offset, ' ') << "- " << m_id << " ";
   if (!m_housenumber.empty())
     std::cout << "house " << m_housenumber << " ";
-  for (const auto &i : m_data_name)
-    std::cout << i.first << ": " << i.second << " ";
+  std::cout << m_name << " ";
   std::cout << "(" << m_my_index << " " << m_last_child_index << ": "
             << m_last_child_index - m_my_index << ": " << m_parent_id << ", " << m_country
             << ", osmid=" << m_osm_id << ")\n";

importer/src/hierarchyitem.h

@@ -20,15 +20,18 @@ class HierarchyItem
   hindex             linked_id() const { return m_linked_id; }
   hindex             parent_id() const { return m_parent_id; }
   const std::string &country() const { return m_country; }
-  bool               keep() const;
+  bool               keep(bool verbose = true) const;
   bool               indexed() const { return m_my_index > 0; }
 
+  void drop();
+  bool dropped() const { return m_dropped; }
+
   const std::deque<std::shared_ptr<HierarchyItem> > &children() { return m_children; }
 
   void  add_child(std::shared_ptr<HierarchyItem> child);
   void  add_linked(std::shared_ptr<HierarchyItem> linked);
   void  set_parent(hindex parent, bool force = false);
-  void  cleanup_children(bool duplicate_only = false);
+  void  cleanup_children();
   sqlid index(sqlid idx, sqlid parent);
   void  write(sqlite3pp::database &db) const;
 
@@ -50,6 +53,7 @@ class HierarchyItem
   sqlid  m_my_index{ 0 };
   sqlid  m_parent_index{ 0 };
   sqlid  m_last_child_index{ 0 };
+  bool   m_dropped{ false };
 
   std::string m_type;
   float       m_latitude;

importer/src/main.cpp

@@ -212,27 +212,26 @@ int main(int argc, char *argv[])
 
   // find missing parents for root nodes
   std::cout << "Fill missing hierarchies. Root size: " << hierarchy.get_root_count() << "\n";
-  for (hindex parent = hierarchy.get_next_nonzero_root_parent(); parent;)
+  for (hindex parent = hierarchy.get_next_nonzero_root_parent(); parent;
+       parent        = hierarchy.get_next_nonzero_root_parent())
     {
-      pqxx::result r     = txn.exec_params(base_query + "where place_id=$1", parent);
-      bool         found = false;
-      for (auto row : r)
+             pqxx::result r     = txn.exec_params(base_query + "where place_id=$1", parent);
+             bool         found = false;
+             for (auto row : r)
         {
-          std::shared_ptr<HierarchyItem> item = std::make_shared<HierarchyItem>(row);
-          hierarchy.add_item(item);
-          found = true;
+                 std::shared_ptr<HierarchyItem> item = std::make_shared<HierarchyItem>(row);
+                 hierarchy.add_item(item);
+                 found = true;
         }
 
       if (!found)
         {
-          std::cerr << "Missing parent with ID " << parent << " . Stopping import\n";
-          hierarchy.print_root_with_parent_id(parent);
-          std::cerr << "\nSQL:\n" << base_query + "where place_id=" << parent << "\n";
+                 std::cerr << "Missing parent with ID " << parent << " . Stopping import\n";
+                 hierarchy.print_root_with_parent_id(parent);
+                 std::cerr << "\nSQL:\n" << base_query + "where place_id=" << parent << "\n";
 
-          return -1;
+                 return -1;
         }
-
-      parent = hierarchy.get_next_nonzero_root_parent();
     }
 
   // remove all items from hierarchy that are not supposed to be there
@@ -258,7 +257,18 @@ int main(int argc, char *argv[])
     }
 
   hierarchy.finalize();
-  hierarchy.check_indexing();
+  if (!hierarchy.check_indexing())
+    {
+      std::set<hindex> problem = hierarchy.get_failed_indexes();
+      for (hindex index : problem)
+        txn.exec_params0("update placex set indexed_status=2 where place_id=$1", index);
+
+      txn.commit();
+
+      std::cout << "Requested to reindex Nominatim database (run nominatim --index) for "
+                << problem.size() << " records\n";
+      return -3;
+    }
 
   txn.commit(); // finalize postgres transactions