Merge pull request #1746 from joto/geom-several-columns

Make the check trigger work with multiple geom columns

Author: lonvia

src/flex-table.cpp

@@ -73,15 +73,14 @@ flex_table_column_t &flex_table_t::add_column(std::string const &name,
            (m_columns.size() == 1 &&
             m_columns[0].type() == table_column_type::id_type));
 
-    m_columns.emplace_back(name, type, sql_type);
-    auto &column = m_columns.back();
+    auto &column = m_columns.emplace_back(name, type, sql_type);
 
     if (column.is_geometry_column()) {
-        if (m_geom_column != std::numeric_limits<std::size_t>::max()) {
+        if (m_geom_column == std::numeric_limits<std::size_t>::max()) {
+            m_geom_column = m_columns.size() - 1;
+        } else {
             m_has_multiple_geom_columns = true;
         }
-        m_geom_column = m_columns.size() - 1;
-        column.set_not_null();
     }
 
     return column;
@@ -162,6 +161,30 @@ void table_connection_t::connect(std::string const &conninfo)
     m_db_connection->exec("SET synchronous_commit = off");
 }
 
+static void
+enable_check_trigger(pg_conn_t *db_connection, flex_table_t const &table)
+{
+    std::string checks;
+
+    for (auto const &column : table) {
+        if (column.is_geometry_column() && column.needs_isvalid()) {
+            checks.append(
+                R"((NEW."{0}" IS NULL OR ST_IsValid(NEW."{0}")) AND )"_format(
+                    column.name()));
+        }
+    }
+
+    if (checks.empty()) {
+        return;
+    }
+
+    // remove last " AND "
+    checks.resize(checks.size() - 5);
+
+    create_geom_check_trigger(db_connection, table.schema(), table.name(),
+                              checks);
+}
+
 void table_connection_t::start(bool append)
 {
     assert(m_db_connection);
@@ -184,12 +207,7 @@ void table_connection_t::start(bool append)
                                       : flex_table_t::table_type::permanent,
             table().full_name()));
 
-        if (table().has_geom_column() &&
-            table().geom_column().needs_isvalid()) {
-            create_geom_check_trigger(m_db_connection.get(), table().schema(),
-                                      table().name(),
-                                      table().geom_column().name());
-        }
+        enable_check_trigger(m_db_connection.get(), table());
     }
 
     prepare();
@@ -254,10 +272,8 @@ void table_connection_t::stop(bool updateable, bool append)
             table().full_tmp_name(), table().name()));
         m_id_index_created = false;
 
-        if (updateable && table().geom_column().needs_isvalid()) {
-            create_geom_check_trigger(m_db_connection.get(), table().schema(),
-                                      table().name(),
-                                      table().geom_column().name());
+        if (updateable) {
+            enable_check_trigger(m_db_connection.get(), table());
         }
     }
 

src/flex-table.hpp

@@ -110,7 +110,7 @@ class flex_table_t
         return m_geom_column != std::numeric_limits<std::size_t>::max();
     }
 
-    // XXX should we allow several geometry columns?
+    /// Get the (first, if there are multiple) geometry column.
     flex_table_column_t const &geom_column() const noexcept
     {
         assert(has_geom_column());
@@ -218,7 +218,10 @@ class flex_table_t
      */
     std::vector<flex_table_column_t> m_columns;
 
-    /// Index of the geometry column in m_columns. Default means no geometry.
+    /**
+     * Index of the (first) geometry column in m_columns. Default means no
+     * geometry column.
+     */
     std::size_t m_geom_column = std::numeric_limits<std::size_t>::max();
 
     /**

src/pgsql-helper.cpp

@@ -35,20 +35,20 @@ idlist_t get_ids_from_db(pg_conn_t const *db_connection, char const *stmt,
 void create_geom_check_trigger(pg_conn_t *db_connection,
                                std::string const &schema,
                                std::string const &table,
-                               std::string const &geom_column)
+                               std::string const &condition)
 {
     std::string func_name = qualified_name(schema, table + "_osm2pgsql_valid");
 
     db_connection->exec(
         "CREATE OR REPLACE FUNCTION {}()\n"
         "RETURNS TRIGGER AS $$\n"
         "BEGIN\n"
-        "  IF ST_IsValid(NEW.{}) THEN \n"
+        "  IF {} THEN \n"
         "    RETURN NEW;\n"
         "  END IF;\n"
         "  RETURN NULL;\n"
         "END;"
-        "$$ LANGUAGE plpgsql;"_format(func_name, geom_column));
+        "$$ LANGUAGE plpgsql;"_format(func_name, condition));
 
     db_connection->exec(
         "CREATE TRIGGER \"{}\""

src/pgsql-helper.hpp

@@ -33,7 +33,7 @@ idlist_t get_ids_from_db(pg_conn_t const *db_connection, char const *stmt,
 void create_geom_check_trigger(pg_conn_t *db_connection,
                                std::string const &schema,
                                std::string const &table,
-                               std::string const &geom_column);
+                               std::string const &condition);
 
 void drop_geom_check_trigger(pg_conn_t *db_connection,
                              std::string const &schema,

src/table.cpp

@@ -134,7 +134,7 @@ void table_t::start(std::string const &conninfo, std::string const &table_space)
 
         if (m_srid != "4326") {
             create_geom_check_trigger(m_sql_conn.get(), m_target->schema,
-                                      m_target->name, "way");
+                                      m_target->name, "ST_IsValid(NEW.way)");
         }
     }
 
@@ -243,7 +243,8 @@ void table_t::stop(bool updateable, bool enable_hstore_index,
                     qual_name, tablespace_clause(table_space_index)));
             if (m_srid != "4326") {
                 create_geom_check_trigger(m_sql_conn.get(), m_target->schema,
-                                          m_target->name, "way");
+                                          m_target->name,
+                                          "ST_IsValid(NEW.way)");
             }
         }