Merge branch 'postgis-quoting' of https://github.com/lightmare/mapnik into lightmare-postgis-quoting

include/mapnik/sql_utils.hpp

@@ -33,12 +33,77 @@
 #pragma GCC diagnostic pop
 
 // stl
-#include <sstream>
+#include <iosfwd>
-#include <vector>
+#include <regex>
 #include <string>
 
 namespace mapnik { namespace sql_utils {
 
+    struct quoted_string
+    {
+        std::string const* operator-> () const { return &str; }
+        std::string const& str;
+        char const quot;
+    };
+
+    inline quoted_string identifier(std::string const& str)
+    {
+        return { str, '"' };
+    }
+
+    inline quoted_string literal(std::string const& str)
+    {
+        return { str, '\'' };
+    }
+
+    inline std::ostream& operator << (std::ostream& os, quoted_string qs)
+    {
+        std::size_t pos = 0, next;
+
+        os.put(qs.quot);
+        while ((next = qs->find(qs.quot, pos)) != std::string::npos)
+        {
+            os.write(qs->data() + pos, next - pos + 1);
+            os.put(qs.quot);
+            pos = next + 1;
+        }
+        if ((next = qs->size()) > pos)
+        {
+            os.write(qs->data() + pos, next - pos);
+        }
+        return os.put(qs.quot);
+    }
+
+    // Does nothing if `str` doesn't start with `quot`.
+    // Otherwise erases the opening quote, collapses inner quote pairs,
+    // and erases everything from the closing quote to the end of the
+    // string. The closing quote is the first non-paired quote after the
+    // opening one. For a well-formed quoted string, it is also the last
+    // character, so nothing gets lost.
+    inline void unquote(char quot, std::string & str)
+    {
+        if (!str.empty() && str.front() == quot)
+        {
+            std::size_t di = 0;
+            for (std::size_t si = 1; si < str.size(); ++si)
+            {
+                char c = str[si];
+                if (c == quot && (++si >= str.size() || str[si] != quot))
+                    break;
+                str[di++] = c;
+            }
+            str.erase(di);
+        }
+    }
+
+    inline std::string unquote_copy(char quot, std::string const& str)
+    {
+        std::string tmp(str);
+        sql_utils::unquote(quot, tmp);
+        return tmp;
+    }
+
+    [[deprecated("flawed")]]
     inline std::string unquote_double(std::string const& sql)
     {
         std::string table_name = sql;
@@ -46,6 +111,7 @@ namespace mapnik { namespace sql_utils {
         return table_name;
     }
 
+    [[deprecated("flawed")]]
     inline std::string unquote(std::string const& sql)
     {
         std::string table_name = sql;
@@ -53,11 +119,75 @@ namespace mapnik { namespace sql_utils {
         return table_name;
     }
 
+    [[deprecated("flawed")]]
     inline void quote_attr(std::ostringstream & s, std::string const& field)
     {
         s << ",\"" << field << "\"";
     }
 
+    const std::regex re_from{
+        "\\bFROM\\b"
+        , std::regex::icase
+    };
+
+    const std::regex re_table_name{
+        "\\s*(\\w+|(\"[^\"]*\")+)"      // $1 = schema
+        "(\\.(\\w+|(\"[^\"]*\")+))?"    // $4 = table
+        "\\s*"
+    };
+
+    inline bool table_from_sql(std::string const& sql,
+                               std::string & schema,
+                               std::string & table)
+    {
+        std::smatch m;
+        auto start = sql.begin();
+        auto end = sql.end();
+        auto flags = std::regex_constants::match_default;
+        auto found = std::regex_match(start, end, m, re_table_name);
+        auto extract_matched_parts = [&]()
+        {
+            if (m[4].matched)
+            {
+                table.assign(m[4].first, m[4].second);
+                schema.assign(m[1].first, m[1].second);
+            }
+            else
+            {
+                table.assign(m[1].first, m[1].second);
+                schema.clear();
+            }
+        };
+
+        if (found)
+        {
+            // input is not subquery, just "[schema.]table"
+            extract_matched_parts();
+        }
+        else
+        {
+            // search "FROM [schema.]table" in subquery
+            while (std::regex_search(start, end, m, re_from, flags))
+            {
+                start = m[0].second;
+                if (std::regex_search(start, end, m, re_table_name,
+                                      std::regex_constants::match_continuous))
+                {
+                    extract_matched_parts();
+                    found = true;
+                    start = m[0].second;
+                }
+                flags = std::regex_constants::match_prev_avail;
+            }
+        }
+        if (found)
+        {
+            sql_utils::unquote('"', schema);
+            sql_utils::unquote('"', table);
+        }
+        return found;
+    }
+
     inline std::string table_from_sql(std::string const& sql)
     {
         std::string table_name = sql;

plugins/input/pgraster/pgraster_datasource.hpp

@@ -42,6 +42,7 @@
 #include <boost/optional.hpp>
 
 // stl
+#include <regex>
 #include <vector>
 #include <string>
 #include <memory>
@@ -90,24 +91,26 @@ public:
 
 private:
     std::string sql_bbox(box2d<double> const& env) const;
-    std::string populate_tokens(std::string const& sql, double scale_denom, box2d<double> const& env, double pixel_width, double pixel_height) const;
+    std::string populate_tokens(std::string const& sql, double scale_denom,
+                                box2d<double> const& env,
+                                double pixel_width, double pixel_height,
+                                mapnik::attributes const& vars,
+                                bool intersect = true) const;
     std::string populate_tokens(std::string const& sql) const;
     std::shared_ptr<IResultSet> get_resultset(std::shared_ptr<Connection> &conn, std::string const& sql, CnxPool_ptr const& pool, processor_context_ptr ctx= processor_context_ptr()) const;
     static const std::string RASTER_COLUMNS;
     static const std::string RASTER_OVERVIEWS;
     static const std::string SPATIAL_REF_SYS;
-    static const double FMAX;
 
     const std::string uri_;
     const std::string username_;
     const std::string password_;
     // table name (schema qualified or not) or subquery
     const std::string table_;
-    // schema name (possibly extracted from table_)
+    const std::string raster_table_; // possibly schema-qualified
-    std::string schema_;
-    // table name (possibly extracted from table_)
-    std::string raster_table_;
     const std::string raster_field_;
+    std::string parsed_schema_; // extracted from raster_table_ or table_
+    std::string parsed_table_; // extracted from raster_table_ or table_
     std::string key_field_;
     mapnik::value_integer cursor_fetch_size_;
     mapnik::value_integer row_limit_;
@@ -129,10 +132,7 @@ private:
     bool clip_rasters_;
     layer_descriptor desc_;
     ConnectionCreator<Connection> creator_;
-    const std::string bbox_token_;
+    std::regex re_tokens_;
-    const std::string scale_denom_token_;
-    const std::string pixel_width_token_;
-    const std::string pixel_height_token_;
     int pool_max_size_;
     bool persist_connection_;
     bool extent_from_subquery_;

plugins/input/postgis/build.py

@@ -52,7 +52,6 @@ libraries = copy(plugin_env['LIBS'])
 
 if env['PLUGIN_LINKING'] == 'shared':
     libraries.append('boost_system%s' % env['BOOST_APPEND'])
-    libraries.append('boost_regex%s' % env['BOOST_APPEND'])
     libraries.insert(0,env['MAPNIK_NAME'])
     libraries.append(env['ICU_LIB_NAME'])
 

plugins/input/postgis/postgis_datasource.cpp

@@ -38,11 +38,10 @@
 #pragma GCC diagnostic push
 #include <mapnik/warning_ignore.hpp>
 #include <boost/algorithm/string.hpp>
-#include <boost/tokenizer.hpp>
-#include <boost/regex.hpp>
 #pragma GCC diagnostic pop
 
 // stl
+#include <cfloat> // FLT_MAX
 #include <memory>
 #include <string>
 #include <algorithm>
@@ -52,17 +51,17 @@
 
 DATASOURCE_PLUGIN(postgis_datasource)
 
-const double postgis_datasource::FMAX = std::numeric_limits<float>::max();
 const std::string postgis_datasource::GEOMETRY_COLUMNS = "geometry_columns";
 const std::string postgis_datasource::SPATIAL_REF_SYS = "spatial_ref_system";
 
 using std::shared_ptr;
 using mapnik::attribute_descriptor;
+using mapnik::sql_utils::identifier;
+using mapnik::sql_utils::literal;
 
 postgis_datasource::postgis_datasource(parameters const& params)
     : datasource(params),
       table_(*params.get<std::string>("table", "")),
-      schema_(""),
       geometry_table_(*params.get<std::string>("geometry_table", "")),
       geometry_field_(*params.get<std::string>("geometry_field", "")),
       key_field_(*params.get<std::string>("key_field", "")),
@@ -79,10 +78,6 @@ postgis_datasource::postgis_datasource(parameters const& params)
              params.get<std::string>("user"),
              params.get<std::string>("password"),
              params.get<std::string>("connect_timeout", "4")),
-      bbox_token_("!bbox!"),
-      scale_denom_token_("!scale_denominator!"),
-      pixel_width_token_("!pixel_width!"),
-      pixel_height_token_("!pixel_height!"),
       pool_max_size_(*params_.get<mapnik::value_integer>("max_size", 10)),
       persist_connection_(*params.get<mapnik::boolean_type>("persist_connection", true)),
       extent_from_subquery_(*params.get<mapnik::boolean_type>("extent_from_subquery", false)),
@@ -99,8 +94,7 @@ postgis_datasource::postgis_datasource(parameters const& params)
       simplify_prefilter_(*params_.get<mapnik::value_double>("simplify_prefilter", 0.0)),
       simplify_dp_preserve_(false),
       simplify_clip_resolution_(*params_.get<mapnik::value_double>("simplify_clip_resolution", 0.0)),
-      // TODO - use for known tokens too: "(@\\w+|!\\w+!)"
+      re_tokens_("!(@?\\w+)!"), // matches  !mapnik_var!  or  !@user_var!
-      pattern_(boost::regex("(@\\w+)",boost::regex::normal | boost::regbase::icase)),
       // params below are for testing purposes only and may be removed at any time
       intersect_min_scale_(*params.get<mapnik::value_integer>("intersect_min_scale", 0)),
       intersect_max_scale_(*params.get<mapnik::value_integer>("intersect_max_scale", 0)),
@@ -159,21 +153,13 @@ postgis_datasource::postgis_datasource(parameters const& params)
 
             desc_.set_encoding(conn->client_encoding());
 
-            if (geometry_table_.empty())
+            mapnik::sql_utils::table_from_sql(
-            {
+                    geometry_table_.empty() ? table_ : geometry_table_,
-                geometry_table_ = mapnik::sql_utils::table_from_sql(table_);
+                    parsed_schema_, parsed_table_);
-            }
 
-            std::string::size_type idx = geometry_table_.find_last_of('.');
+            // NOTE: parsed_table_ now should ideally be a table name, but
-            if (idx != std::string::npos)
-            {
-                schema_ = geometry_table_.substr(0, idx);
-                geometry_table_ = geometry_table_.substr(idx + 1);
-            }
-
-            // NOTE: geometry_table_ how should ideally be a table name, but
             // there are known edge cases where this will break down and
-            // geometry_table_ may even be empty: https://github.com/mapnik/mapnik/issues/2718
+            // it may even be empty: https://github.com/mapnik/mapnik/issues/2718
 
             // If we do not know both the geometry_field and the srid
             // then first attempt to fetch the geometry name from a geometry_columns entry.
@@ -181,8 +167,8 @@ postgis_datasource::postgis_datasource(parameters const& params)
             // from the simplistic table parsing in table_from_sql() or if
             // the table parameter references a table, view, or subselect not
             // registered in the geometry columns.
-            geometryColumn_ = geometry_field_;
+            geometryColumn_ = mapnik::sql_utils::unquote_copy('"', geometry_field_);
-            if (!geometry_table_.empty() && (geometryColumn_.empty() || srid_ == 0))
+            if (!parsed_table_.empty() && (geometryColumn_.empty() || srid_ == 0))
             {
 #ifdef MAPNIK_STATS
                 mapnik::progress_timer __stats2__(std::clog, "postgis_datasource::init(get_srid_and_geometry_column)");
@@ -191,21 +177,15 @@ postgis_datasource::postgis_datasource(parameters const& params)
 
                 try
                 {
-                    s << "SELECT f_geometry_column, srid FROM "
+                    s << "SELECT f_geometry_column, srid FROM " << GEOMETRY_COLUMNS
-                      << GEOMETRY_COLUMNS <<" WHERE f_table_name='"
+                      << " WHERE f_table_name=" << literal(parsed_table_);
-                      << mapnik::sql_utils::unquote_double(geometry_table_)
+                    if (!parsed_schema_.empty())
-                      << "'";
-                    if (! schema_.empty())
                     {
-                        s << " AND f_table_schema='"
+                        s << " AND f_table_schema=" << literal(parsed_schema_);
-                          << mapnik::sql_utils::unquote_double(schema_)
-                          << "'";
                     }
-                    if (! geometry_field_.empty())
+                    if (!geometryColumn_.empty())
                     {
-                        s << " AND f_geometry_column='"
+                        s << " AND f_geometry_column=" << literal(geometryColumn_);
-                          << mapnik::sql_utils::unquote_double(geometry_field_)
-                          << "'";
                     }
                     shared_ptr<ResultSet> rs = conn->executeQuery(s.str());
                     if (rs->next())
@@ -237,26 +217,24 @@ postgis_datasource::postgis_datasource(parameters const& params)
             }
 
             // If we still do not know the srid then we can try to fetch
-            // it from the 'geometry_table_' parameter, which should work even if it is
+            // it from the 'parsed_table_' parameter, which should work even if it is
             // a subselect as long as we know the geometry_field to query
             if (!geometryColumn_.empty() && srid_ <= 0)
             {
                 std::ostringstream s;
 
-                s << "SELECT ST_SRID(\"" << geometryColumn_ << "\") AS srid FROM ";
+                s << "SELECT ST_SRID(" << identifier(geometryColumn_)
-                if (!geometry_table_.empty())
+                  << ") AS srid FROM ";
+                if (!parsed_table_.empty())
                 {
-                    if (!schema_.empty())
+                    append_geometry_table(s);
-                    {
-                        s << schema_ << '.';
-                    }
-                    s << geometry_table_;
                 }
                 else
                 {
                     s << populate_tokens(table_);
                 }
-                s << " WHERE \"" << geometryColumn_ << "\" IS NOT NULL LIMIT 1;";
+                s << " WHERE " << identifier(geometryColumn_)
+                  << " IS NOT NULL LIMIT 1";
 
                 shared_ptr<ResultSet> rs = conn->executeQuery(s.str());
                 if (rs->next())
@@ -288,18 +266,16 @@ postgis_datasource::postgis_datasource(parameters const& params)
                     "WHERE a.attnum > 0 AND a.attrelid = c.oid "
                     "AND a.atttypid = t.oid AND c.relnamespace = n.oid "
                     "AND c.oid = i.indrelid AND i.indisprimary = 't' "
-                    "AND t.typname !~ '^geom' AND c.relname ="
+                    "AND t.typname !~ '^geom' AND c.relname = "
-                  << " '" << mapnik::sql_utils::unquote_double(geometry_table_) << "' "
+                  << literal(parsed_table_) << " "
                     //"AND a.attnum = ANY (i.indkey) " // postgres >= 8.1
                   << "AND (i.indkey[0]=a.attnum OR i.indkey[1]=a.attnum OR i.indkey[2]=a.attnum "
                     "OR i.indkey[3]=a.attnum OR i.indkey[4]=a.attnum OR i.indkey[5]=a.attnum "
                     "OR i.indkey[6]=a.attnum OR i.indkey[7]=a.attnum OR i.indkey[8]=a.attnum "
                     "OR i.indkey[9]=a.attnum) ";
-                if (! schema_.empty())
+                if (!parsed_schema_.empty())
                 {
-                    s << "AND n.nspname='"
+                    s << "AND n.nspname=" << literal(parsed_schema_) << ' ';
-                      << mapnik::sql_utils::unquote_double(schema_)
-                      << "' ";
                 }
                 s << "ORDER BY a.attnum";
 
@@ -318,7 +294,7 @@ postgis_datasource::postgis_datasource(parameters const& params)
                                 key_field_ = std::string(key_field_string);
 
                                 MAPNIK_LOG_DEBUG(postgis) << "postgis_datasource: auto-detected key field of '"
-                                                          << key_field_ << "' on table '" << geometry_table_ << "'";
+                                                          << key_field_ << "' on table '" << parsed_table_ << "'";
                             }
                         }
                         else
@@ -329,7 +305,7 @@ postgis_datasource::postgis_datasource(parameters const& params)
                             err << "PostGIS Plugin: Error: '"
                                 << rs_key->getValue(0)
                                 << "' on table '"
-                                << geometry_table_
+                                << parsed_table_
                                 << "' is not a valid integer primary key field\n";
                             throw mapnik::datasource_exception(err.str());
                         }
@@ -349,9 +325,11 @@ postgis_datasource::postgis_datasource(parameters const& params)
             // but still not known at this point, then throw
             if (*autodetect_key_field && key_field_.empty())
             {
-                throw mapnik::datasource_exception(std::string("PostGIS Plugin: Error: primary key required")
+                throw mapnik::datasource_exception(
-                                                   + " but could not be detected for table '" +
+                        "PostGIS Plugin: Error: primary key required"
-                                                   geometry_table_ + "', please supply 'key_field' option to specify field to use for primary key");
+                        " but could not be detected for table '"
+                        + parsed_table_ + "', please supply 'key_field'"
+                        " option to specify field to use for primary key");
             }
 
             if (srid_ == 0)
@@ -551,43 +529,9 @@ std::string postgis_datasource::sql_bbox(box2d<double> const& env) const
 
 std::string postgis_datasource::populate_tokens(std::string const& sql) const
 {
-    std::string populated_sql = sql;
+    return populate_tokens(sql, FLT_MAX,
-
+                           box2d<double>(-FLT_MAX, -FLT_MAX, FLT_MAX, FLT_MAX),
-    if (boost::algorithm::icontains(sql, bbox_token_))
+                           0, 0, mapnik::attributes{}, false);
-    {
-        box2d<double> max_env(-1.0 * FMAX, -1.0 * FMAX, FMAX, FMAX);
-        const std::string max_box = sql_bbox(max_env);
-        boost::algorithm::replace_all(populated_sql, bbox_token_, max_box);
-    }
-
-    if (boost::algorithm::icontains(sql, scale_denom_token_))
-    {
-        std::ostringstream ss;
-        ss << FMAX;
-        boost::algorithm::replace_all(populated_sql, scale_denom_token_, ss.str());
-    }
-
-    if (boost::algorithm::icontains(sql, pixel_width_token_))
-    {
-        boost::algorithm::replace_all(populated_sql, pixel_width_token_, "0");
-    }
-
-    if (boost::algorithm::icontains(sql, pixel_height_token_))
-    {
-        boost::algorithm::replace_all(populated_sql, pixel_height_token_, "0");
-    }
-
-    std::string copy2 = populated_sql;
-    std::list<std::string> l;
-    boost::regex_split(std::back_inserter(l), copy2, pattern_);
-    if (!l.empty())
-    {
-        for (auto const & token: l)
-        {
-            boost::algorithm::replace_all(populated_sql, token, "null");
-        }
-    }
-    return populated_sql;
 }
 
 std::string postgis_datasource::populate_tokens(
@@ -596,43 +540,66 @@ std::string postgis_datasource::populate_tokens(
                                 box2d<double> const& env,
                                 double pixel_width,
                                 double pixel_height,
-                                mapnik::attributes const& vars) const
+                                mapnik::attributes const& vars,
+                                bool intersect) const
 {
-    std::string populated_sql = sql;
+    std::ostringstream populated_sql;
-    std::string box = sql_bbox(env);
+    std::cmatch m;
+    char const* start = sql.data();
+    char const* end = start + sql.size();
 
-    if (boost::algorithm::icontains(populated_sql, scale_denom_token_))
+    while (std::regex_search(start, end, m, re_tokens_))
     {
-        std::ostringstream ss;
+        populated_sql.write(start, m[0].first - start);
-        ss << scale_denom;
+        start = m[0].second;
-        boost::algorithm::replace_all(populated_sql, scale_denom_token_, ss.str());
+
+        auto m1 = boost::make_iterator_range(m[1].first, m[1].second);
+        if (m1.front() == '@')
+        {
+            std::string var_name(m1.begin() + 1, m1.end());
+            auto itr = vars.find(var_name);
+            if (itr != vars.end())
+            {
+                auto var_value = itr->second.to_string();
+                populated_sql << literal(var_value);
+            }
+            else
+            {
+                populated_sql << "NULL"; // undefined @variable
+            }
+        }
+        else if (boost::algorithm::equals(m1, "bbox"))
+        {
+            populated_sql << sql_bbox(env);
+            intersect = false;
+        }
+        else if (boost::algorithm::equals(m1, "pixel_height"))
+        {
+            populated_sql << pixel_height;
+        }
+        else if (boost::algorithm::equals(m1, "pixel_width"))
+        {
+            populated_sql << pixel_width;
+        }
+        else if (boost::algorithm::equals(m1, "scale_denominator"))
+        {
+            populated_sql << scale_denom;
+        }
+        else
+        {
+            populated_sql << "NULL"; // unrecognized !token!
+        }
     }
 
-    if (boost::algorithm::icontains(sql, pixel_width_token_))
+    populated_sql.write(start, end - start);
-    {
-        std::ostringstream ss;
-        ss << pixel_width;
-        boost::algorithm::replace_all(populated_sql, pixel_width_token_, ss.str());
-    }
 
-    if (boost::algorithm::icontains(sql, pixel_height_token_))
+    if (intersect)
     {
-        std::ostringstream ss;
-        ss << pixel_height;
-        boost::algorithm::replace_all(populated_sql, pixel_height_token_, ss.str());
-    }
-
-    if (boost::algorithm::icontains(populated_sql, bbox_token_))
-    {
-        boost::algorithm::replace_all(populated_sql, bbox_token_, box);
-    }
-    else
-    {
-        std::ostringstream s;
-
         if (intersect_min_scale_ > 0 && (scale_denom <= intersect_min_scale_))
         {
-            s << " WHERE ST_Intersects(\"" << geometryColumn_ << "\"," << box << ")";
+            populated_sql << " WHERE ST_Intersects("
+                          << identifier(geometryColumn_) << ", "
+                          << sql_bbox(env) << ")";
         }
         else if (intersect_max_scale_ > 0 && (scale_denom >= intersect_max_scale_))
         {
@@ -640,31 +607,34 @@ std::string postgis_datasource::populate_tokens(
         }
         else
         {
-            s << " WHERE \"" << geometryColumn_ << "\" && " << box;
+            populated_sql << " WHERE "
-        }
+                          << identifier(geometryColumn_) << " && "
-        populated_sql += s.str();
+                          << sql_bbox(env);
-    }
-    std::string copy2 = populated_sql;
-    std::list<std::string> l;
-    boost::regex_split(std::back_inserter(l), copy2, pattern_);
-    if (!l.empty())
-    {
-        for (auto const & token: l)
-        {
-            auto itr = vars.find(token.substr(1,std::string::npos));
-            if (itr != vars.end())
-            {
-                boost::algorithm::replace_all(populated_sql, token, itr->second.to_string());
-            }
-            else
-            {
-                boost::algorithm::replace_all(populated_sql, token, "null");
-            }
         }
     }
-    return populated_sql;
+
+    return populated_sql.str();
 }
 
+void postgis_datasource::append_geometry_table(std::ostream & os) const
+{
+    if (!geometry_table_.empty())
+    {
+        os << geometry_table_; // assume datasource parameter is valid SQL
+    }
+    else if (!parsed_schema_.empty())
+    {
+        os << identifier(parsed_schema_) << '.' << identifier(parsed_table_);
+    }
+    else if (!parsed_table_.empty())
+    {
+        os << identifier(parsed_table_);
+    }
+    else
+    {
+        os << table_; // assume datasource parameter is valid SQL
+    }
+}
 
 std::shared_ptr<IResultSet> postgis_datasource::get_resultset(std::shared_ptr<Connection> &conn, std::string const& sql, CnxPool_ptr const& pool, processor_context_ptr ctx) const
 {
@@ -789,20 +759,11 @@ featureset_ptr postgis_datasource::features_with_context(query const& q,processo
         {
             std::ostringstream s_error;
             s_error << "PostGIS: geometry name lookup failed for table '";
-
+            append_geometry_table(s_error);
-            if (! schema_.empty())
+            s_error << "'. Please manually provide the 'geometry_field' parameter or add an entry "
-            {
-                s_error << schema_ << ".";
-            }
-            s_error << geometry_table_
-                    << "'. Please manually provide the 'geometry_field' parameter or add an entry "
                     << "in the geometry_columns for '";
-
+            append_geometry_table(s_error);
-            if (! schema_.empty())
+            s_error << "'.";
-            {
-                s_error << schema_ << ".";
-            }
-            s_error << geometry_table_ << "'.";
 
             throw mapnik::datasource_exception(s_error.str());
         }
@@ -833,7 +794,7 @@ featureset_ptr postgis_datasource::features_with_context(query const& q,processo
             {
                 s << "ST_ClipByBox2D(";
             }
-            s << "\"" << geometryColumn_ << "\"";
+            s << identifier(geometryColumn_);
 
             // ! ST_ClipByBox2D()
             if (simplify_clip_resolution_ > 0.0 && simplify_clip_resolution_ > px_sz)
@@ -865,7 +826,7 @@ featureset_ptr postgis_datasource::features_with_context(query const& q,processo
             }
 
             // Geometry column!
-            s << "\"" << geometryColumn_ << "\"";
+            s << identifier(geometryColumn_);
 
             // ! ST_SnapToGrid()
             if (simplify_geometries_ && simplify_snap_ratio_ > 0.0)
@@ -903,7 +864,7 @@ featureset_ptr postgis_datasource::features_with_context(query const& q,processo
 
         if (! key_field_.empty())
         {
-            mapnik::sql_utils::quote_attr(s, key_field_);
+            s << ',' << identifier(key_field_);
             if (key_field_as_attribute_)
             {
                 ctx->push(key_field_);
@@ -913,7 +874,7 @@ featureset_ptr postgis_datasource::features_with_context(query const& q,processo
             {
                 if (*pos != key_field_)
                 {
-                    mapnik::sql_utils::quote_attr(s, *pos);
+                    s << ',' << identifier(*pos);
                     ctx->push(*pos);
                 }
             }
@@ -922,7 +883,7 @@ featureset_ptr postgis_datasource::features_with_context(query const& q,processo
         {
             for (; pos != end; ++pos)
             {
-                mapnik::sql_utils::quote_attr(s, *pos);
+                s << ',' << identifier(*pos);
                 ctx->push(*pos);
             }
         }
@@ -963,33 +924,25 @@ featureset_ptr postgis_datasource::features_at_point(coord2d const& pt, double t
             {
                 std::ostringstream s_error;
                 s_error << "PostGIS: geometry name lookup failed for table '";
-
+                append_geometry_table(s_error);
-                if (! schema_.empty())
+                s_error << "'. Please manually provide the 'geometry_field' parameter or add an entry "
-                {
-                    s_error << schema_ << ".";
-                }
-                s_error << geometry_table_
-                        << "'. Please manually provide the 'geometry_field' parameter or add an entry "
                         << "in the geometry_columns for '";
-
+                append_geometry_table(s_error);
-                if (! schema_.empty())
+                s_error << "'.";
-                {
-                    s_error << schema_ << ".";
-                }
-                s_error << geometry_table_ << "'.";
 
                 throw mapnik::datasource_exception(s_error.str());
             }
 
             std::ostringstream s;
-            s << "SELECT ST_AsBinary(\"" << geometryColumn_ << "\") AS geom";
+            s << "SELECT ST_AsBinary(" << identifier(geometryColumn_)
+              << ") AS geom";
 
             mapnik::context_ptr ctx = std::make_shared<mapnik::context_type>();
             auto const& desc = desc_.get_descriptors();
 
             if (!key_field_.empty())
             {
-                mapnik::sql_utils::quote_attr(s, key_field_);
+                s << ',' << identifier(key_field_);
                 if (key_field_as_attribute_)
                 {
                     ctx->push(key_field_);
@@ -999,7 +952,7 @@ featureset_ptr postgis_datasource::features_at_point(coord2d const& pt, double t
                     std::string const& name = attr_info.get_name();
                     if (name != key_field_)
                     {
-                        mapnik::sql_utils::quote_attr(s, name);
+                        s << ',' << identifier(name);
                         ctx->push(name);
                     }
                 }
@@ -1009,13 +962,14 @@ featureset_ptr postgis_datasource::features_at_point(coord2d const& pt, double t
                 for (auto const& attr_info : desc)
                 {
                     std::string const& name = attr_info.get_name();
-                    mapnik::sql_utils::quote_attr(s, name);
+                    s << ',' << identifier(name);
                     ctx->push(name);
                 }
             }
 
             box2d<double> box(pt.x - tol, pt.y - tol, pt.x + tol, pt.y + tol);
-            std::string table_with_bbox = populate_tokens(table_, FMAX, box, 0, 0, mapnik::attributes());
+            std::string table_with_bbox = populate_tokens(table_, FLT_MAX, box, 0, 0,
+                                                          mapnik::attributes{});
 
             s << " FROM " << table_with_bbox;
 
@@ -1053,12 +1007,8 @@ box2d<double> postgis_datasource::envelope() const
             {
                 std::ostringstream s_error;
                 s_error << "PostGIS: unable to query the layer extent of table '";
-
+                append_geometry_table(s_error);
-                if (! schema_.empty())
+                s_error << "' because we cannot determine the geometry field name."
-                {
-                    s_error << schema_ << ".";
-                }
-                s_error << geometry_table_ << "' because we cannot determine the geometry field name."
                         << "\nPlease provide either an 'extent' parameter to skip this query, "
                         << "a 'geometry_field' and/or 'geometry_table' parameter, or add a "
                         << "record to the 'geometry_columns' for your table.";
@@ -1070,19 +1020,19 @@ box2d<double> postgis_datasource::envelope() const
             {
                 s << "SELECT ST_XMin(ext),ST_YMin(ext),ST_XMax(ext),ST_YMax(ext)"
                   << " FROM (SELECT ST_EstimatedExtent('";
-
+                if (!parsed_schema_.empty())
-                if (! schema_.empty())
                 {
-                    s << mapnik::sql_utils::unquote_double(schema_) << "','";
+                    s << literal(parsed_schema_) << ',';
                 }
 
-                s << mapnik::sql_utils::unquote_double(geometry_table_) << "','"
+                s << literal(parsed_table_) << ','
-                  << mapnik::sql_utils::unquote_double(geometryColumn_) << "') as ext) as tmp";
+                  << literal(geometryColumn_) << ") as ext) as tmp";
             }
             else
             {
                 s << "SELECT ST_XMin(ext),ST_YMin(ext),ST_XMax(ext),ST_YMax(ext)"
-                  << " FROM (SELECT ST_Extent(" <<geometryColumn_<< ") as ext from ";
+                  << " FROM (SELECT ST_Extent("
+                  << identifier(geometryColumn_) << ") as ext from ";
 
                 if (extent_from_subquery_)
                 {
@@ -1092,14 +1042,10 @@ box2d<double> postgis_datasource::envelope() const
                 }
                 else
                 {
-                    if (! schema_.empty())
-                    {
-                        s << schema_ << ".";
-                    }
-
                     // but if the subquery does not limit records then querying the
                     // actual table will be faster as indexes can be used
-                    s << geometry_table_ << ") as tmp";
+                    append_geometry_table(s);
+                    s << ") as tmp";
                 }
             }
 
@@ -1142,21 +1088,15 @@ boost::optional<mapnik::datasource_geometry_t> postgis_datasource::get_geometry_
             std::string g_type;
             try
             {
-                s << "SELECT lower(type) as type FROM "
+                s << "SELECT lower(type) as type FROM " << GEOMETRY_COLUMNS
-                  << GEOMETRY_COLUMNS <<" WHERE f_table_name='"
+                  << " WHERE f_table_name=" << literal(parsed_table_);
-                  << mapnik::sql_utils::unquote_double(geometry_table_)
+                if (!parsed_schema_.empty())
-                  << "'";
-                if (! schema_.empty())
                 {
-                    s << " AND f_table_schema='"
+                    s << " AND f_table_schema=" << literal(parsed_schema_);
-                      << mapnik::sql_utils::unquote_double(schema_)
-                      << "'";
                 }
-                if (! geometry_field_.empty())
+                if (!geometryColumn_.empty())
                 {
-                    s << " AND f_geometry_column='"
+                    s << " AND f_geometry_column=" << literal(geometryColumn_);
-                      << mapnik::sql_utils::unquote_double(geometry_field_)
-                      << "'";
                 }
                 shared_ptr<ResultSet> rs = conn->executeQuery(s.str());
                 if (rs->next())
@@ -1195,8 +1135,8 @@ boost::optional<mapnik::datasource_geometry_t> postgis_datasource::get_geometry_
 
                 std::string prev_type("");
 
-                s << "SELECT ST_GeometryType(\""
+                s << "SELECT ST_GeometryType("
-                  << geometryColumn_ << "\") AS geom"
+                  << identifier(geometryColumn_) << ") AS geom"
                   << " FROM " << populate_tokens(table_);
 
                 if (row_limit_ > 0 && row_limit_ < 5)

plugins/input/postgis/postgis_datasource.hpp

@@ -37,10 +37,10 @@
 
 // boost
 #include <boost/optional.hpp>
-#include <boost/regex.hpp>
 
 // stl
 #include <memory>
+#include <regex>
 #include <vector>
 #include <string>
 
@@ -84,20 +84,22 @@ private:
                                 box2d<double> const& env,
                                 double pixel_width,
                                 double pixel_height,
-                                mapnik::attributes const& vars) const;
+                                mapnik::attributes const& vars,
+                                bool intersect = true) const;
     std::string populate_tokens(std::string const& sql) const;
+    void append_geometry_table(std::ostream & os) const;
     std::shared_ptr<IResultSet> get_resultset(std::shared_ptr<Connection> &conn, std::string const& sql, CnxPool_ptr const& pool, processor_context_ptr ctx= processor_context_ptr()) const;
     static const std::string GEOMETRY_COLUMNS;
     static const std::string SPATIAL_REF_SYS;
-    static const double FMAX;
 
     const std::string uri_;
     const std::string username_;
     const std::string password_;
     const std::string table_;
-    std::string schema_;
+    const std::string geometry_table_;
-    std::string geometry_table_;
     const std::string geometry_field_;
+    std::string parsed_schema_;
+    std::string parsed_table_;
     std::string key_field_;
     mapnik::value_integer cursor_fetch_size_;
     mapnik::value_integer row_limit_;
@@ -109,10 +111,6 @@ private:
     bool simplify_geometries_;
     layer_descriptor desc_;
     ConnectionCreator<Connection> creator_;
-    const std::string bbox_token_;
-    const std::string scale_denom_token_;
-    const std::string pixel_width_token_;
-    const std::string pixel_height_token_;
     int pool_max_size_;
     bool persist_connection_;
     bool extent_from_subquery_;
@@ -126,7 +124,7 @@ private:
     mapnik::value_double simplify_prefilter_;
     bool simplify_dp_preserve_;
     mapnik::value_double simplify_clip_resolution_;
-    boost::regex pattern_;
+    std::regex re_tokens_;
     int intersect_min_scale_;
     int intersect_max_scale_;
     bool key_field_as_attribute_;

test/unit/datasource/postgis.cpp

@@ -26,6 +26,7 @@
 #include <mapnik/datasource.hpp>
 #include <mapnik/datasource_cache.hpp>
 #include <mapnik/geometry/geometry_type.hpp>
+#include <mapnik/unicode.hpp>
 #include <mapnik/util/fs.hpp>
 
 /*
@@ -185,6 +186,24 @@ TEST_CASE("postgis") {
             CHECK(ds->get_geometry_type() == mapnik::datasource_geometry_t::Point);
         }
 
+        SECTION("Postgis properly escapes names with single quotes")
+        {
+            mapnik::parameters params(base_params);
+            params["table"] = "\"test'single'quotes\"";
+            auto ds = mapnik::datasource_cache::instance().create(params);
+            REQUIRE(ds != nullptr);
+            CHECK(ds->get_geometry_type() == mapnik::datasource_geometry_t::Point);
+        }
+
+        SECTION("Postgis properly escapes names with double quotes")
+        {
+            mapnik::parameters params(base_params);
+            params["table"] = "\"test\"\"double\"\"quotes\"";
+            auto ds = mapnik::datasource_cache::instance().create(params);
+            REQUIRE(ds != nullptr);
+            CHECK(ds->get_geometry_type() == mapnik::datasource_geometry_t::Point);
+        }
+
         SECTION("Postgis query field names")
         {
             mapnik::parameters params(base_params);
@@ -270,6 +289,55 @@ TEST_CASE("postgis") {
             REQUIRE(ext.maxy() == 4);
         }
 
+        SECTION("Postgis doesn't interpret @domain in email address as @variable")
+        {
+            mapnik::parameters params(base_params);
+            params["table"] = "(SELECT gid, geom, 'fake@mail.ru' as email"
+                              " FROM public.test LIMIT 1) AS data";
+            auto ds = mapnik::datasource_cache::instance().create(params);
+            REQUIRE(ds != nullptr);
+            auto featureset = all_features(ds);
+            auto feature = featureset->next();
+            CHECKED_IF(feature != nullptr)
+            {
+                CHECK(feature->get("email").to_string() == "fake@mail.ru");
+            }
+        }
+
+        SECTION("Postgis interpolates !@uservar! tokens in query")
+        {
+            mapnik::parameters params(base_params);
+            params["table"] = "(SELECT * FROM public.test"
+                              " WHERE GeometryType(geom) = !@wantedGeomType!"
+                              " LIMIT 1) AS data";
+            auto ds = mapnik::datasource_cache::instance().create(params);
+            REQUIRE(ds != nullptr);
+
+            mapnik::transcoder tr("utf8");
+            mapnik::query qry(ds->envelope());
+            qry.set_variables({{"wantedGeomType", tr.transcode("POINT")}});
+            CHECK(qry.variables().count("wantedGeomType") == 1);
+
+            auto featureset = ds->features(qry);
+            auto feature = featureset->next();
+            CHECKED_IF(feature != nullptr)
+            {
+                auto const& geom = feature->get_geometry();
+                CHECK(mapnik::geometry::geometry_type(geom) == mapnik::geometry::Point);
+            }
+
+            qry.set_variables({{"wantedGeomType", tr.transcode("POLYGON")}});
+            CHECK(qry.variables().count("wantedGeomType") == 1);
+
+            featureset = ds->features(qry);
+            feature = featureset->next();
+            CHECKED_IF(feature != nullptr)
+            {
+                auto const& geom = feature->get_geometry();
+                CHECK(mapnik::geometry::geometry_type(geom) == mapnik::geometry::Polygon);
+            }
+        }
+
         SECTION("Postgis query extent: full dataset")
         {
             //include schema to increase coverage