Merge pull request #1504 from joto/cluster-flex-optional

Make clustering by geometry optional in flex output

Author: lonvia

src/flex-table.cpp

@@ -199,7 +199,7 @@ void table_connection_t::stop(bool updateable, bool append)
         return;
     }
 
-    if (table().has_geom_column()) {
+    if (table().cluster_by_geom()) {
         if (table().geom_column().needs_isvalid()) {
             drop_geom_check_trigger(m_db_connection.get(), table().schema(),
                                     table().name());
@@ -246,6 +246,14 @@ 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 (table().has_geom_column()) {
         log_info("Creating geometry index on table '{}'...", table().name());
 
         // Use fillfactor 100 for un-updateable imports
@@ -258,13 +266,6 @@ void table_connection_t::stop(bool updateable, bool append)
 
     if (updateable && table().has_id_column()) {
         create_id_index();
-
-        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());
-        }
     }
 
     log_info("Analyzing table '{}'...", table().name());

src/flex-table.hpp

@@ -48,6 +48,11 @@ class flex_table_t
 
     std::string const &schema() const noexcept { return m_schema; }
 
+    bool cluster_by_geom() const noexcept
+    {
+        return has_geom_column() && m_cluster_by_geom;
+    }
+
     std::string const &data_tablespace() const noexcept
     {
         return m_data_tablespace;
@@ -60,6 +65,11 @@ class flex_table_t
 
     void set_schema(std::string const &schema) noexcept { m_schema = schema; }
 
+    void set_cluster_by_geom(bool cluster) noexcept
+    {
+        m_cluster_by_geom = cluster;
+    }
+
     void set_data_tablespace(std::string const &tablespace) noexcept
     {
         m_data_tablespace = tablespace;
@@ -211,6 +221,9 @@ class flex_table_t
      */
     osmium::item_type m_id_type = osmium::item_type::undefined;
 
+    /// Cluster the table by geometry.
+    bool m_cluster_by_geom = true;
+
 }; // class flex_table_t
 
 class table_connection_t

src/output-flex.cpp

@@ -69,6 +69,7 @@ TRAMPOLINE(app_define_table, define_table)
 TRAMPOLINE(app_get_bbox, get_bbox)
 TRAMPOLINE(table_name, name)
 TRAMPOLINE(table_schema, schema)
+TRAMPOLINE(table_cluster, cluster)
 TRAMPOLINE(table_add_row, add_row)
 TRAMPOLINE(table_columns, columns)
 TRAMPOLINE(table_tostring, __tostring)
@@ -771,6 +772,28 @@ flex_table_t &output_flex_t::create_flex_table()
     }
     lua_pop(lua_state(), 1);
 
+    // optional "cluster" field
+    lua_getfield(lua_state(), -1, "cluster");
+    int const cluster_type = lua_type(lua_state(), -1);
+    if (cluster_type == LUA_TSTRING) {
+        std::string const cluster = lua_tostring(lua_state(), -1);
+        if (cluster == "auto") {
+            new_table.set_cluster_by_geom(true);
+        } else if (cluster == "no") {
+            new_table.set_cluster_by_geom(false);
+        } else {
+            throw std::runtime_error{
+                "Unknown value '{}' for 'cluster' table option"
+                " (use 'auto' or 'no')."_format(cluster)};
+        }
+    } else if (cluster_type == LUA_TNIL) {
+        // ignore
+    } else {
+        throw std::runtime_error{
+            "Unknown value for 'cluster' table option: Must be string."};
+    }
+    lua_pop(lua_state(), 1);
+
     // optional "data_tablespace" field
     lua_getfield(lua_state(), -1, "data_tablespace");
     if (lua_isstring(lua_state(), -1)) {
@@ -1058,6 +1081,13 @@ int output_flex_t::table_schema()
     return 1;
 }
 
+int output_flex_t::table_cluster()
+{
+    auto const &table = get_table_from_param();
+    lua_pushboolean(lua_state(), table.cluster_by_geom());
+    return 1;
+}
+
 static std::unique_ptr<geom_transform_t>
 get_transform(lua_State *lua_state, flex_table_column_t const &column)
 {
@@ -1615,6 +1645,7 @@ void output_flex_t::init_lua(std::string const &filename)
     luaX_add_table_func(lua_state(), "add_row", lua_trampoline_table_add_row);
     luaX_add_table_func(lua_state(), "name", lua_trampoline_table_name);
     luaX_add_table_func(lua_state(), "schema", lua_trampoline_table_schema);
+    luaX_add_table_func(lua_state(), "cluster", lua_trampoline_table_cluster);
     luaX_add_table_func(lua_state(), "columns", lua_trampoline_table_columns);
 
     // Clean up stack

src/output-flex.hpp

@@ -159,6 +159,7 @@ class output_flex_t : public output_t
     int table_add_row();
     int table_name();
     int table_schema();
+    int table_cluster();
     int table_columns();
 
 private:

tests/CMakeLists.txt

@@ -73,6 +73,7 @@ if (HAVE_LUA)
     set_test(test-output-flex-area)
     set_test(test-output-flex-attr)
     set_test(test-output-flex-bbox)
+    set_test(test-output-flex-cluster)
     set_test(test-output-flex-invalid-geom)
     set_test(test-output-flex-line)
     set_test(test-output-flex-lua-fail)

tests/data/test_output_flex.lua

@@ -13,7 +13,8 @@ tables.line = osm2pgsql.define_table{
         { column = 'tags', type = 'hstore' },
         { column = 'name', type = 'text' },
         { column = 'geom', type = 'linestring' },
-    }
+    },
+    cluster = 'auto'
 }
 
 tables.polygon = osm2pgsql.define_table{

tests/data/test_output_flex_nocluster.lua

@@ -0,0 +1,29 @@
+
+local dtable = osm2pgsql.define_node_table('osm2pgsql_test_point', {
+    { column = 'tags', type = 'hstore' },
+    { column = 'geom', type = 'point' },
+}, { cluster = 'no' })
+
+function is_empty(some_table)
+    return next(some_table) == nil
+end
+
+function clean_tags(tags)
+    tags.odbl = nil
+    tags.created_by = nil
+    tags.source = nil
+    tags['source:ref'] = nil
+    tags['source:name'] = nil
+end
+
+function osm2pgsql.process_node(data)
+    clean_tags(data.tags)
+    if is_empty(data.tags) then
+        return
+    end
+
+    dtable:add_row({
+        tags = data.tags
+    })
+end
+

tests/test-output-flex-cluster.cpp

@@ -0,0 +1,28 @@
+#include <catch.hpp>
+
+#include "common-import.hpp"
+#include "common-options.hpp"
+
+static testing::db::import_t db;
+
+static char const *const conf_file = "test_output_flex_nocluster.lua";
+static char const *const data_file = "liechtenstein-2013-08-03.osm.pbf";
+
+TEST_CASE("non-slim without clustering")
+{
+    REQUIRE_NOTHROW(db.run_file(testing::opt_t().flex(conf_file), data_file));
+
+    auto conn = db.db().connect();
+
+    CHECK(1362 == conn.get_count("osm2pgsql_test_point"));
+}
+
+TEST_CASE("slim without clustering")
+{
+    REQUIRE_NOTHROW(
+        db.run_file(testing::opt_t().slim().flex(conf_file), data_file));
+
+    auto conn = db.db().connect();
+
+    CHECK(1362 == conn.get_count("osm2pgsql_test_point"));
+}