Add sql_type column field in flex output

The "type" field in a flex output column definition really does two
things:
1. It defines the way Lua values are transformed into PostgreSQL
   values.
2. It defines which PostgreSQL type should be used in a the CREATE
   TABLE statement for a particular column.

This commit adds a new "sql_type" field which takes over the second
meaning. If it is not specified, the default is still defined by
the "type" field. The default for the type field is "text".

The result is that in most cases you use only the "type" field as before
and it does the right thing. If you need a special SQL type, use the
"sql_type" field instead or in addition to the "type" field.

Note that this is a breaking change, but still allowed because the flex
output is still marked experimental.

Author: joto

flex-config/attributes.lua

@@ -15,7 +15,10 @@ tables.points = osm2pgsql.define_node_table('points', {
     { column = 'geom', type = 'point', projection = srid },
     { column = 'version', type = 'int' },
     { column = 'changeset', type = 'int' },
-    { column = 'created', type = 'timestamp' },
+    -- There is no built-in type for timestamps in osm2pgsql. So we use the
+    -- PostgreSQL type "timestamp" and then have to convert our timestamps
+    -- to a valid text representation for that type.
+    { column = 'created', sql_type = 'timestamp' },
     { column = 'uid', type = 'int' },
     { column = 'user', type = 'text' },
 })
@@ -25,7 +28,7 @@ tables.lines = osm2pgsql.define_way_table('lines', {
     { column = 'geom', type = 'linestring', projection = srid },
     { column = 'version', type = 'int' },
     { column = 'changeset', type = 'int' },
-    { column = 'created', type = 'timestamp' },
+    { column = 'created', sql_type = 'timestamp' },
     { column = 'uid', type = 'int' },
     { column = 'user', type = 'text' },
 })
@@ -34,7 +37,7 @@ tables.relations = osm2pgsql.define_relation_table('relations', {
     { column = 'tags', type = 'jsonb' },
     { column = 'version', type = 'int' },
     { column = 'changeset', type = 'int' },
-    { column = 'created', type = 'timestamp' },
+    { column = 'created', sql_type = 'timestamp' },
     { column = 'uid', type = 'int' },
     { column = 'user', type = 'text' },
 })

flex-config/data-types.lua

@@ -11,7 +11,7 @@ local highways = osm2pgsql.define_way_table('highways', {
 
     -- Add a SERIAL column and tell osm2pgsql not to fill it (PostgreSQL will
     -- do that for us)
-    { column = 'id',       type = 'serial', create_only = true },
+    { column = 'id',       sql_type = 'serial', create_only = true },
 
     -- type "direction" is special, see below
     { column = 'oneway',   type = 'direction' },
@@ -21,8 +21,10 @@ local highways = osm2pgsql.define_way_table('highways', {
     { column = 'lit',      type = 'bool' },
     { column = 'tags',     type = 'jsonb' }, -- also available: 'json', 'hstore'
 
-    -- an PostgreSQL array type, not specially handled by osm2pgsql, see below
-    { column = 'nodes',    type = 'int8[]' },
+    -- osm2pgsql doesn't know about PostgreSQL arrays, so we define the SQL
+    -- type of this column and then have to convert our array data into a
+    -- valid text representation for that type, see below.
+    { column = 'nodes',    sql_type = 'int8[]' },
     { column = 'geom',     type = 'linestring' },
 })
 

flex-config/route-relations.lua

@@ -14,7 +14,7 @@ local tables = {}
 tables.highways = osm2pgsql.define_way_table('highways', {
     { column = 'tags',     type = 'jsonb' },
     { column = 'rel_refs', type = 'text' }, -- for the refs from the relations
-    { column = 'rel_ids',  type = 'int8[]' }, -- array with integers (for relation IDs)
+    { column = 'rel_ids',  sql_type = 'int8[]' }, -- array with integers (for relation IDs)
     { column = 'geom',     type = 'linestring' },
 })
 

src/flex-table-column.cpp

@@ -22,9 +22,8 @@ struct column_type_lookup
     table_column_type type;
 };
 
-static std::array<column_type_lookup, 26> const column_types = {
-    {{"sql", table_column_type::sql},
-     {"text", table_column_type::text},
+static std::array<column_type_lookup, 25> const column_types = {
+    {{"text", table_column_type::text},
      {"boolean", table_column_type::boolean},
      {"bool", table_column_type::boolean},
      {"int2", table_column_type::int2},
@@ -51,21 +50,19 @@ static std::array<column_type_lookup, 26> const column_types = {
      {"id_num", table_column_type::id_num}}};
 
 static table_column_type
-get_column_type_from_string(std::string const &type) noexcept
+get_column_type_from_string(std::string const &type)
 {
     auto const it =
         std::find_if(std::begin(column_types), std::end(column_types),
                      [&type](column_type_lookup name_type) {
                          return type == name_type.name;
                      });
 
-    if (it != std::end(column_types)) {
-        return it->type;
+    if (it == std::end(column_types)) {
+        throw std::runtime_error{"Unknown column type '{}'."_format(type)};
     }
 
-    // If we don't recognize the column type, we just assume its a valid SQL
-    // type and use it "as is".
-    return table_column_type::sql;
+    return it->type;
 }
 
 static std::string lowercase(std::string const &str)
@@ -81,8 +78,10 @@ static std::string lowercase(std::string const &str)
 }
 
 flex_table_column_t::flex_table_column_t(std::string name,
-                                         std::string const &type)
+                                         std::string const &type,
+                                         std::string const &sql_type)
 : m_name(std::move(name)), m_type_name(lowercase(type)),
+  m_sql_type(sql_type),
   m_type(get_column_type_from_string(m_type_name))
 {}
 
@@ -115,9 +114,11 @@ void flex_table_column_t::set_projection(char const *projection)
 
 std::string flex_table_column_t::sql_type_name() const
 {
+    if (!m_sql_type.empty()) {
+        return m_sql_type;
+    }
+
     switch (m_type) {
-    case table_column_type::sql:
-        return m_type_name;
     case table_column_type::text:
         return "text";
     case table_column_type::boolean:

src/flex-table-column.hpp

@@ -18,8 +18,6 @@
 
 enum class table_column_type : uint8_t
 {
-    sql,
-
     text,
 
     boolean,
@@ -56,7 +54,8 @@ enum class table_column_type : uint8_t
 class flex_table_column_t
 {
 public:
-    flex_table_column_t(std::string name, std::string const &type);
+    flex_table_column_t(std::string name, std::string const &type,
+                        std::string const &sql_type);
 
     std::string const &name() const noexcept { return m_name; }
 
@@ -122,14 +121,18 @@ class flex_table_column_t
     std::string m_name;
 
     /**
-     * The type name of the database table column. Either a name we recognize
-     * or just an SQL snippet.
+     * The type name of the column.
      */
     std::string m_type_name;
 
     /**
-     * The type of database column. Use table_column_type::sql as fallback
-     * in which case m_type_name is the SQL type used in the database.
+     * The SQL type of the database table column. If this is not set, use
+     * one generated from the m_type.
+     */
+    std::string m_sql_type;
+
+    /**
+     * The type of column.
      */
     table_column_type m_type;
 

src/flex-table.cpp

@@ -65,15 +65,16 @@ std::string flex_table_t::full_tmp_name() const
 }
 
 flex_table_column_t &flex_table_t::add_column(std::string const &name,
-                                              std::string const &type)
+                                              std::string const &type,
+                                              std::string const &sql_type)
 {
     // id_type (optional) and id_num must always be the first columns
     assert(type != "id_type" || m_columns.empty());
     assert(type != "id_num" || m_columns.empty() ||
            (m_columns.size() == 1 &&
             m_columns[0].type() == table_column_type::id_type));
 
-    m_columns.emplace_back(name, type);
+    m_columns.emplace_back(name, type, sql_type);
     auto &column = m_columns.back();
 
     if (column.is_geometry_column()) {

src/flex-table.hpp

@@ -183,7 +183,8 @@ class flex_table_t
     }
 
     flex_table_column_t &add_column(std::string const &name,
-                                    std::string const &type);
+                                    std::string const &type,
+                                    std::string const &sql_type);
 
     bool has_multicolumn_id_index() const noexcept;
     std::string id_column_names() const;

src/lua-utils.cpp

@@ -125,6 +125,24 @@ char const *luaX_get_table_string(lua_State *lua_state, char const *key,
     return lua_tostring(lua_state, -1);
 }
 
+char const *luaX_get_table_string(lua_State *lua_state, char const *key,
+                                  int table_index, char const *error_msg,
+                                  char const *default_value)
+{
+    assert(lua_state);
+    assert(key);
+    lua_getfield(lua_state, table_index, key);
+    int const ltype = lua_type(lua_state, -1);
+    if (ltype == LUA_TNIL) {
+        return default_value;
+    }
+    if (ltype != LUA_TSTRING) {
+        throw std::runtime_error{
+            "{} must contain a '{}' string field."_format(error_msg, key)};
+    }
+    return lua_tostring(lua_state, -1);
+}
+
 bool luaX_get_table_bool(lua_State *lua_state, char const *key, int table_index,
                          char const *error_msg, bool default_value)
 {

src/lua-utils.hpp

@@ -55,6 +55,10 @@ void luaX_add_table_array(lua_State *lua_state, char const *key,
 char const *luaX_get_table_string(lua_State *lua_state, char const *key,
                                   int table_index, char const *error_msg);
 
+char const *luaX_get_table_string(lua_State *lua_state, char const *key,
+                                  int table_index, char const *error_msg,
+                                  char const *default_value);
+
 bool luaX_get_table_bool(lua_State *lua_state, char const *key, int table_index,
                          char const *error_msg, bool default_value);
 

src/output-flex.cpp

@@ -491,8 +491,7 @@ void output_flex_t::write_column(
         return;
     }
 
-    if ((column.type() == table_column_type::sql) ||
-        (column.type() == table_column_type::text)) {
+    if (column.type() == table_column_type::text) {
         auto const *const str = lua_tolstring(lua_state(), -1, nullptr);
         if (!str) {
             throw std::runtime_error{
@@ -845,7 +844,7 @@ void output_flex_t::setup_id_columns(flex_table_t *table)
             std::string const column_name =
                 lua_tolstring(lua_state(), -1, nullptr);
             check_name(column_name, "column");
-            auto &column = table->add_column(column_name, "id_type");
+            auto &column = table->add_column(column_name, "id_type", "");
             column.set_not_null();
         } else if (!lua_isnil(lua_state(), -1)) {
             throw std::runtime_error{"type_column must be a string or nil."};
@@ -859,7 +858,7 @@ void output_flex_t::setup_id_columns(flex_table_t *table)
         luaX_get_table_string(lua_state(), "id_column", -2, "The ids field");
     check_name(name, "column");
 
-    auto &column = table->add_column(name, "id_num");
+    auto &column = table->add_column(name, "id_num", "");
     column.set_not_null();
     lua_pop(lua_state(), 3); // id_column, type, ids
 }
@@ -885,21 +884,23 @@ void output_flex_t::setup_flex_table_columns(flex_table_t *table)
                 "The entries in the 'columns' array must be tables."};
         }
 
-        char const *const type =
-            luaX_get_table_string(lua_state(), "type", -1, "Column entry");
+        char const *const type = luaX_get_table_string(lua_state(), "type", -1,
+                                                       "Column entry", "text");
         char const *const name =
             luaX_get_table_string(lua_state(), "column", -2, "Column entry");
         check_name(name, "column");
+        char const *const sql_type = luaX_get_table_string(
+            lua_state(), "sql_type", -3, "Column entry", "");
 
-        auto &column = table->add_column(name, type);
+        auto &column = table->add_column(name, type, sql_type);
 
-        column.set_not_null(luaX_get_table_bool(lua_state(), "not_null", -3,
+        column.set_not_null(luaX_get_table_bool(lua_state(), "not_null", -4,
                                                 "Entry 'not_null'", false));
 
         column.set_create_only(luaX_get_table_bool(
-            lua_state(), "create_only", -4, "Entry 'create_only'", false));
+            lua_state(), "create_only", -5, "Entry 'create_only'", false));
 
-        lua_getfield(lua_state(), -5, "projection");
+        lua_getfield(lua_state(), -6, "projection");
         if (!lua_isnil(lua_state(), -1)) {
             if (column.is_geometry_column() ||
                 column.type() == table_column_type::area) {
@@ -910,8 +911,9 @@ void output_flex_t::setup_flex_table_columns(flex_table_t *table)
             }
         }
 
-        // stack has: projection, create_only, not_null, column, type, table
-        lua_pop(lua_state(), 6);
+        // stack has: projection, create_only, not_null, sql_type, column,
+        //            type, table
+        lua_pop(lua_state(), 7);
         ++num_columns;
     }