Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tokens in Mapnik postgis sql sub-selects not replaced using options.variables for VectorTiles #679

Closed
oleksii-leonov opened this issue Aug 17, 2016 · 12 comments · Fixed by #809
Milestone

Comments

@oleksii-leonov
Copy link

Accordingly to documentation, mapnik.Map render method have options attribute that could contain variables object.

http://mapnik.org/documentation/node-mapnik/3.5/#Map.render

options.variables

Mapnik 3.x ONLY: A javascript object containing key value pairs that should be passed into Mapnik as variables for rendering and for datasource queries. For example if you passed vtile.render(map,image,{ variables : {zoom:1} },cb) then the @zoom variable would be usable in Mapnik symbolizers like line-width:"@zoom" and as a token in Mapnik postgis sql sub-selects like (select * from table where some_field > @zoom) as tmp (used when rendering a vector tile)

Also, example from mapbox/tilelive-bridge code:
https://github.com/mapbox/tilelive-bridge/blob/master/index.js#L281

var opts = {};
opts.variables = { "zoom_level": z };
map.render(vtile, opts, function(err, vtile) {
  ...
});

Test case

I tried to pass variables to mapnik PostGIS SQL query accordingly to docs (id = @fieldid).

var mapnik = require('mapnik');
var fs = require('fs');

mapnik.register_default_input_plugins();
var map = new mapnik.Map(256, 256);
map.loadSync('./map_conf.xml');

var opts = {};
opts.variables = { "fieldid": 1 };

map.render(new mapnik.VectorTile(0, 0, 0), opts, function(err, vtile) {
  vtile.getData(function(err, pbf) {
      console.log(pbf);
  });
});

map_conf.xml file:

<?xml version="1.0" encoding="utf-8"?>
<Map srs="+init=epsg:3857">
  <Layer name="field_shapes" status="on" srs="+init=epsg:4326">
    <Datasource>
      <Parameter name="type">postgis</Parameter>
      <Parameter name="host">localhost</Parameter>
      <Parameter name="dbname">db_development</Parameter>
      <Parameter name="user">db_user</Parameter>
      <Parameter name="password">db_pass</Parameter>
      <Parameter name="srid">4326</Parameter>
      <Parameter name="geometry_field">shape</Parameter>
      <Parameter name="table">
  (SELECT id, shape FROM fields WHERE id = @fieldid) as field_shapes
      </Parameter>
      <Parameter name="estimate_extent">false</Parameter>
      <Parameter name="extent">-150,-85,150,85</Parameter>
    </Datasource>
  </Layer>
</Map>

Problem

id = @fieldid must be replaced with id = 1, but I always get id = null.

From log of SQL queries:

SELECT ST_AsBinary("shape") AS geom,"id" FROM
  (SELECT id, shape FROM fields WHERE id = null) as field_shapes
WHERE "shape" && ST_SetSRID('BOX3D(-150 -85,150 85)'::box3d, 4326)

Mapnik version

> mapnik
...
version: '3.5.13',
versions: 
   { node: '4.4.3',
     v8: '4.5.103.37',
     boost: '1.59.0',
     boost_number: 105900,
     mapnik: '3.0.11',
     mapnik_number: 300011,
     mapnik_git_describe: 'v3.0.11',
     cairo: '1.14.4' },
...
@oleksii-leonov
Copy link
Author

Same example, but written in C++ (using C++ mapnik libraries) works fine and variables replaced properly (tested with mapnik 3.0.9 and 3.0.12).

So, problem definitely in node-mapnik bindings. Trying to debug it now :)

@oleksii-leonov
Copy link
Author

This problem appears only with Vector tiles. When rendering to raster image, tokens replaced properly.

var opts = {};
opts.variables = { "fieldid": 1 };

// tokens from `opts.variables` replaced properly
map.render(new mapnik.Image(256, 256), opts, function(err, im) {
  ...
}

// tokens not replaced
map.render(new mapnik.VectorTile(0, 0, 0), opts, function(err, vtile) {
  ...
}

@oleksii-leonov
Copy link
Author

oleksii-leonov commented Aug 24, 2016

Based on public methods, there no ability to set opts.variables to mapnik::vector_tile_impl::processor (https://github.com/mapbox/mapnik-vector-tile/blob/master/src/vector_tile_processor.hpp#L50).

So, would be good to remove options.variables option from http://mapnik.org/documentation/node-mapnik/3.5/#VectorTile.render.

@oleksii-leonov oleksii-leonov changed the title .render options.variables didn't replace tokens in Mapnik postgis sql sub-selects tokens in Mapnik postgis sql sub-selects not replaced using options.variables for VectorTiles Aug 25, 2016
@springmeyer
Copy link
Member

This can be solved once mapbox/mapnik-vector-tile#248 from @rafatower is pulled in.

@springmeyer springmeyer added this to the v3.6.3 milestone Sep 10, 2017
@springmeyer
Copy link
Member

@aleksejleonov the patch needed is CartoDB@5182b71. Would you be able to create a PR with that patch + testcases?

@oleksii-leonov
Copy link
Author

@springmeyer
Could try on weekends. But I didn't use node-mapnik for long time (need some free time to setup dev environment and make testcases).

Maybe @rafatower have plans to merge his branch CartoDB@5182b71 into node-mapnik master? Would be nice to have response from @rafatower to avoid work duplication.

@springmeyer
Copy link
Member

Sounds good, we can wait from word on plans from @rafatower

@rafatower
Copy link
Contributor

I'll take a stab at writing some tests for my patch. Thanks!

@Cadfael31
Copy link

Hello,

I saw that this topic is treated in many tickets and I am a bit lost. I hope being in the right one to ask my question.

Currently I use node-mapnik 3.7.2 and this issue (options.variables are not taken into account for vector tiles) persists. In the tickets I saw that the versions 3.6.3 and 3.6.4 are mentionned but there are not released. Is the fix planned for the versions 3.7.x?

Thank you for your answer.

@Cadfael31
Copy link

Nobody else? :-(

@springmeyer
Copy link
Member

@Cadfael31 next steps at #809 (comment). Maybe you can help get this landed?

@Cadfael31
Copy link

@springmeyer , thank you for your answer, I didn't read the previous comments carefully enough. I'm sorry. About helping you, sure I would like, but I'm not sure to be good enough in development.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants