Innefective vars in ST_AsMVT queries

[jgoizueta]

When handling a query to be passed to ST_AsMVT we inject Mapnik tokens (bbox, scale-denominator, pixel_width, pixel_height) and also three additional values: var_zoom, var_x, var_y.

This is ineffective because SubstitutionToken only supports the mapnik tokens.

If the additional var variables are desirable we should add them to SubstitutionToken, otherwise remove them. I find them very convenient except that they can only be used with queries to be sent to ST_AsMVT (not Mapnik).

@rafatower : I guess you introduced these variables, do you remember what was the intention?

cc @dgaubert

[rafatower]

That is dependent on some mapnik & friends changes:

• Pass variables to replace tokens in query mapbox/mapnik-vector-tile#248
• Pass variables to replace tokens in query mapnik/node-mapnik#809

[rafatower]

Sorry about the partial and inaccurate response above. I'll try to elaborate it a little further:

Mapnik variables in general are used like this:
http://mapnik.org/documentation/node-mapnik/3.6/#Map.render

options.variables
Object
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 passedvtile.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)

and now we have full support for them in mapnik and node-mapnik starting with Windshaft v4.3.1: https://github.com/CartoDB/Windshaft/blob/master/NEWS.md#version-431

Here's an example from the node-mapnik tests: https://github.com/CartoDB/node-mapnik/pull/10/files#diff-34a62dfe0f38cdda7b5022e3a70eb9e6R11

But you were talking specifically about pg-mvt renderer. As you saw, var_zoom, var_x and var_y are not being used nor replaced there. It would make sense to do so if the query passed (through layer.options.sql) contained any of them, as it was the case in the 1000x branch, where that code comes from:

https://github.com/CartoDB/Windshaft/blob/1000x/lib/windshaft/utils/substitution_tokens.js
https://github.com/CartoDB/Windshaft/blob/1000x/lib/windshaft/backends/attributes.js#L84-L87
https://github.com/CartoDB/Windshaft/blob/1000x/lib/windshaft/renderers/mapnik/geojson_renderer.js#L87-L90

Now, this should open up the possibility of the datasources refactor you had in mind in 1000x in case you still want to carry on.

To sum up: I'd add them to SubstitutionToken in order to make both mapnik and pg-mvt renderers functionally equivalent, and similar to geojson. It simply does not hurt.

Hope this sheds some light on the matter. Hope this can be somehow useful for the aggregation stuff.

[rafatower]

Related: CartoDB/mapnik#29