Merge pull request #909 from lonvia/split-table-t-constructor

Split table_t constructor

Author: lonvia

output-multi.cpp

@@ -24,12 +24,10 @@ output_multi_t::output_multi_t(std::string const &name,
   m_osm_type(m_processor->interests(geometry_processor::interest_node)
                  ? osmium::item_type::node
                  : osmium::item_type::way),
-  m_table(new table_t(
-      m_options.database_options.conninfo(), name, m_processor->column_type(),
-      export_list.normal_columns(m_osm_type), m_options.hstore_columns,
-      m_processor->srid(), m_options.append, m_options.slim, m_options.droptemp,
-      m_options.hstore_mode, m_options.enable_hstore_index,
-      m_options.tblsmain_data, m_options.tblsmain_index)),
+  m_table(new table_t(name, m_processor->column_type(),
+                      export_list.normal_columns(m_osm_type),
+                      m_options.hstore_columns, m_processor->srid(),
+                      m_options.append, m_options.hstore_mode)),
   ways_done_tracker(new id_tracker()),
   m_expire(m_options.expire_tiles_zoom, m_options.expire_tiles_max_bbox,
            m_options.projection),
@@ -66,7 +64,8 @@ output_multi_t::clone(std::shared_ptr<middle_query_t> const &mid) const
 }
 
 int output_multi_t::start() {
-    m_table->start();
+    m_table->start(m_options.database_options.conninfo(),
+                   m_options.tblsmain_data);
     return 0;
 }
 
@@ -178,7 +177,10 @@ int output_multi_t::pending_relation(osmid_t id, int exists) {
 
 void output_multi_t::stop(osmium::thread::Pool *pool)
 {
-    pool->submit(std::bind(&table_t::stop, m_table.get()));
+    pool->submit([this]() {
+        m_table->stop(m_options.slim & !m_options.droptemp,
+                      m_options.enable_hstore_index, m_options.tblsmain_index);
+    });
     if (m_options.expire_tiles_zoom_min > 0) {
         m_expire.output_and_destroy(m_options.expire_tiles_filename.c_str(),
                                     m_options.expire_tiles_zoom_min);

output-pgsql.cpp

@@ -22,7 +22,6 @@
 #include <unistd.h>
 
 #include <boost/algorithm/string/predicate.hpp>
-#include <boost/bind.hpp>
 #include <boost/exception_ptr.hpp>
 #include <boost/format.hpp>
 
@@ -242,7 +241,10 @@ void output_pgsql_t::stop(osmium::thread::Pool *pool)
 {
     // attempt to stop tables in parallel
     for (auto &t : m_tables) {
-        pool->submit(std::bind(&table_t::stop, t.get()));
+        pool->submit([&]() {
+            t->stop(m_options.slim & !m_options.droptemp,
+                    m_options.enable_hstore_index, m_options.tblsmain_index);
+        });
     }
 
     if (m_options.expire_tiles_zoom_min > 0) {
@@ -490,7 +492,8 @@ int output_pgsql_t::start()
 {
     for (auto &t : m_tables) {
         //setup the table in postgres
-        t->start();
+        t->start(m_options.database_options.conninfo(),
+                 m_options.tblsmain_data);
     }
 
     return 0;
@@ -517,7 +520,7 @@ output_pgsql_t::output_pgsql_t(std::shared_ptr<middle_query_t> const &mid,
     m_tagtransform = tagtransform_t::make_tagtransform(&m_options, exlist);
 
     //for each table
-    for (int i = 0; i < t_MAX; i++) {
+    for (size_t i = 0; i < t_MAX; i++) {
 
         //figure out the columns this table needs
         columns_t columns = exlist.normal_columns(
@@ -549,15 +552,10 @@ output_pgsql_t::output_pgsql_t(std::shared_ptr<middle_query_t> const &mid,
                 util::exit_nicely();
         }
 
-        //tremble in awe of this massive constructor! seriously we are trying to avoid passing an
-        //options object because we want to make use of the table_t in output_mutli_t which could
-        //have a different tablespace/hstores/etc per table
-        m_tables[i].reset(new table_t(
-            m_options.database_options.conninfo(), name, type, columns,
-            m_options.hstore_columns, m_options.projection->target_srs(),
-            m_options.append, m_options.slim, m_options.droptemp,
-            m_options.hstore_mode, m_options.enable_hstore_index,
-            m_options.tblsmain_data, m_options.tblsmain_index));
+        m_tables[i].reset(new table_t(name, type, columns,
+                                      m_options.hstore_columns,
+                                      m_options.projection->target_srs(),
+                                      m_options.append, m_options.hstore_mode));
     }
 }
 

output.cpp

@@ -9,7 +9,6 @@
 #include <stdexcept>
 
 #include <boost/format.hpp>
-#include <boost/functional/hash.hpp>
 #include <boost/property_tree/ptree.hpp>
 #include <boost/property_tree/json_parser.hpp>
 

table.cpp

@@ -16,13 +16,12 @@ typedef boost::format fmt;
 
 #define BUFFER_SEND_SIZE 1024
 
-
-table_t::table_t(const string& conninfo, const string& name, const string& type, const columns_t& columns, const hstores_t& hstore_columns,
-    const int srid, const bool append, const bool slim, const bool drop_temp, const int hstore_mode,
-    const bool enable_hstore_index, const boost::optional<string>& table_space, const boost::optional<string>& table_space_index) :
-    conninfo(conninfo), name(name), type(type), sql_conn(nullptr), copyMode(false), srid((fmt("%1%") % srid).str()),
-    append(append), slim(slim), drop_temp(drop_temp), hstore_mode(hstore_mode), enable_hstore_index(enable_hstore_index),
-    columns(columns), hstore_columns(hstore_columns), table_space(table_space), table_space_index(table_space_index)
+table_t::table_t(string const &name, string const &type,
+                 columns_t const &columns, hstores_t const &hstore_columns,
+                 int const srid, bool const append, int const hstore_mode)
+: name(name), type(type), sql_conn(nullptr), copyMode(false),
+  srid((fmt("%1%") % srid).str()), append(append), hstore_mode(hstore_mode),
+  columns(columns), hstore_columns(hstore_columns)
 {
     //if we dont have any columns
     if(columns.size() == 0 && hstore_mode != HSTORE_ALL)
@@ -36,11 +35,13 @@ table_t::table_t(const string& conninfo, const string& name, const string& type,
     del_fmt = fmt("DELETE FROM %1% WHERE osm_id = %2%");
 }
 
-table_t::table_t(const table_t& other):
-    conninfo(other.conninfo), name(other.name), type(other.type), sql_conn(nullptr), copyMode(false), buffer(), srid(other.srid),
-    append(other.append), slim(other.slim), drop_temp(other.drop_temp), hstore_mode(other.hstore_mode), enable_hstore_index(other.enable_hstore_index),
-    columns(other.columns), hstore_columns(other.hstore_columns), copystr(other.copystr), table_space(other.table_space),
-    table_space_index(other.table_space_index), single_fmt(other.single_fmt), del_fmt(other.del_fmt)
+table_t::table_t(table_t const &other)
+: m_conninfo(other.m_conninfo), name(other.name), type(other.type),
+  sql_conn(nullptr), copyMode(false), srid(other.srid), append(other.append),
+  hstore_mode(other.hstore_mode), columns(other.columns),
+  hstore_columns(other.hstore_columns), copystr(other.copystr),
+  m_table_space(other.m_table_space), single_fmt(other.single_fmt),
+  del_fmt(other.del_fmt)
 {
     // if the other table has already started, then we want to execute
     // the same stuff to get into the same state. but if it hasn't, then
@@ -89,19 +90,23 @@ void table_t::commit()
 void table_t::connect()
 {
     //connect
-    PGconn* _conn = PQconnectdb(conninfo.c_str());
+    PGconn *_conn = PQconnectdb(m_conninfo.c_str());
     if (PQstatus(_conn) != CONNECTION_OK)
         throw std::runtime_error((fmt("Connection to database failed: %1%\n") % PQerrorMessage(_conn)).str());
     sql_conn = _conn;
     //let commits happen faster by delaying when they actually occur
     pgsql_exec_simple(sql_conn, PGRES_COMMAND_OK, "SET synchronous_commit TO off;");
 }
 
-void table_t::start()
+void table_t::start(std::string const &conninfo,
+                    boost::optional<std::string> const &table_space)
 {
     if(sql_conn)
         throw std::runtime_error(name + " cannot start, its already started");
 
+    m_conninfo = conninfo;
+    m_table_space = table_space ? " TABLESPACE " + table_space.get() : "";
+
     connect();
     fprintf(stderr, "Setting up table: %s\n", name.c_str());
     pgsql_exec_simple(sql_conn, PGRES_COMMAND_OK, "SET client_min_messages = WARNING");
@@ -144,20 +149,10 @@ void table_t::start()
         // doesn't need to be RESET on these tables
         sql += " WITH ( autovacuum_enabled = FALSE )";
         //add the main table space
-        if (table_space)
-            sql += " TABLESPACE " + table_space.get();
+        sql += m_table_space;
 
         //create the table
         pgsql_exec_simple(sql_conn, PGRES_COMMAND_OK, sql);
-
-        //slim mode needs this to be able to delete from tables in pending
-        if (slim && !drop_temp) {
-            sql = (fmt("CREATE INDEX ON %1% USING BTREE (osm_id)") % name).str();
-            if (table_space_index)
-                sql += " TABLESPACE " + table_space_index.get();
-            pgsql_exec_simple(sql_conn, PGRES_COMMAND_OK, sql);
-        }
-
     }//appending
     else {
         //check the columns against those in the existing table
@@ -208,7 +203,8 @@ void table_t::start()
     copyMode = true;
 }
 
-void table_t::stop()
+void table_t::stop(bool updateable, bool enable_hstore_index,
+                   boost::optional<std::string> const &table_space_index)
 {
     stop_copy();
     if (!append)
@@ -221,14 +217,13 @@ void table_t::stop()
         if (srid == "4326") {
             /* libosmium assures validity of geometries in 4326, so the WHERE can be skipped.
                Because we know the geom is already in 4326, no reprojection is needed for GeoHashing */
-            pgsql_exec_simple(
-                sql_conn, PGRES_COMMAND_OK,
-                (fmt("CREATE TABLE %1%_tmp %2% AS\n"
-                     "  SELECT * FROM %1%\n"
-                     "    ORDER BY ST_GeoHash(way,10)\n"
-                     "    COLLATE \"C\"") %
-                 name % (table_space ? "TABLESPACE " + table_space.get() : ""))
-                    .str());
+            pgsql_exec_simple(sql_conn, PGRES_COMMAND_OK,
+                              (fmt("CREATE TABLE %1%_tmp %2% AS\n"
+                                   "  SELECT * FROM %1%\n"
+                                   "    ORDER BY ST_GeoHash(way,10)\n"
+                                   "    COLLATE \"C\"") %
+                               name % m_table_space)
+                                  .str());
         } else {
             /* osm2pgsql's transformation from 4326 to another projection could make a geometry invalid,
                and these need to be filtered. Also, a transformation is needed for geohashing.
@@ -244,7 +239,7 @@ void table_t::stop()
                      "    ORDER BY ST_GeoHash(ST_Transform(ST_Envelope(way),4326),10)\n"
                      // clang-format on
                      "    COLLATE \"C\"") %
-                 name % (table_space ? "TABLESPACE " + table_space.get() : ""))
+                 name % m_table_space)
                     .str());
             pgsql_exec_simple(sql_conn, PGRES_COMMAND_OK, "RESET client_min_messages");
         }
@@ -253,17 +248,23 @@ void table_t::stop()
         fprintf(stderr, "Copying %s to cluster by geometry finished\n", name.c_str());
         fprintf(stderr, "Creating geometry index on %s\n", name.c_str());
 
+        std::string tblspc_sql =
+            table_space_index ? "TABLESPACE " + table_space_index.get() : "";
         // Use fillfactor 100 for un-updatable imports
-        pgsql_exec_simple(sql_conn, PGRES_COMMAND_OK, (fmt("CREATE INDEX ON %1% USING GIST (way) %2% %3%") % name %
-            (slim && !drop_temp ? "" : "WITH (FILLFACTOR=100)") %
-            (table_space_index ? "TABLESPACE " + table_space_index.get() : "")).str());
+        pgsql_exec_simple(sql_conn, PGRES_COMMAND_OK,
+                          (fmt("CREATE INDEX ON %1% USING GIST (way) %2% %3%") %
+                           name % (updateable ? "" : "WITH (FILLFACTOR=100)") %
+                           tblspc_sql)
+                              .str());
 
         /* slim mode needs this to be able to apply diffs */
-        if (slim && !drop_temp)
-        {
+        if (updateable) {
             fprintf(stderr, "Creating osm_id index on %s\n", name.c_str());
-            pgsql_exec_simple(sql_conn, PGRES_COMMAND_OK, (fmt("CREATE INDEX ON %1% USING BTREE (osm_id) %2%") % name %
-                (table_space_index ? "TABLESPACE " + table_space_index.get() : "")).str());
+            pgsql_exec_simple(
+                sql_conn, PGRES_COMMAND_OK,
+                (fmt("CREATE INDEX ON %1% USING BTREE (osm_id) %2%") % name %
+                 tblspc_sql)
+                    .str());
             if (srid != "4326") {
                 pgsql_exec_simple(
                     sql_conn, PGRES_COMMAND_OK,

table.hpp

@@ -19,14 +19,16 @@ typedef std::vector<std::string> hstores_t;
 class table_t
 {
     public:
-        table_t(const std::string& conninfo, const std::string& name, const std::string& type, const columns_t& columns, const hstores_t& hstore_columns, const int srid,
-                const bool append, const bool slim, const bool droptemp, const int hstore_mode, const bool enable_hstore_index,
-                const boost::optional<std::string>& table_space, const boost::optional<std::string>& table_space_index);
+        table_t(std::string const &name, std::string const &type,
+                columns_t const &columns, hstores_t const &hstore_columns,
+                const int srid, const bool append, const int hstore_mode);
         table_t(const table_t& other);
         ~table_t();
 
-        void start();
-        void stop();
+        void start(std::string const &conninfo,
+                   boost::optional<std::string> const &table_space);
+        void stop(bool updateable, bool enable_hstore_index,
+                  boost::optional<std::string> const &table_space_index);
 
         void begin();
         void commit();
@@ -81,23 +83,19 @@ class table_t
         void escape4hstore(const char *src, std::string& dst);
         void escape_type(const std::string &value, ColumnType flags, std::string& dst);
 
-        std::string conninfo;
+        std::string m_conninfo;
         std::string name;
         std::string type;
         pg_conn *sql_conn;
         bool copyMode;
         std::string buffer;
         std::string srid;
         bool append;
-        bool slim;
-        bool drop_temp;
         int hstore_mode;
-        bool enable_hstore_index;
         columns_t columns;
         hstores_t hstore_columns;
         std::string copystr;
-        boost::optional<std::string> table_space;
-        boost::optional<std::string> table_space_index;
+        std::string m_table_space;
 
         boost::format single_fmt, del_fmt;
 };