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

Apply patch to enable the use of the @ character in SQL queries #29

Closed
enekid opened this issue Jan 26, 2018 · 6 comments
Closed

Apply patch to enable the use of the @ character in SQL queries #29

enekid opened this issue Jan 26, 2018 · 6 comments
Assignees

Comments

@enekid
Copy link

enekid commented Jan 26, 2018

This issue documents a bug related to the use of the @ character in the SQL queries sent to mapnik. It has being patched in the future 3.1 mapnik release, but not in v3.0.15 (the version used currently in this fork), because it has breaking changes.

Mapnik 3.0.X has a bug with the @ character in the SQL queries. Whenever it finds a @ it treats it as a render-time variable. Therefore the use of the @ character corrupts the SQL query (eg. email addresses: tesla@baddogs.com).

#1136 could be fixed applying the patch, but at the same time, all uses of render-time variables in windshaft should be surrounded by ! characters.

@Algunenano Algunenano self-assigned this Jan 29, 2018
@Algunenano
Copy link

Initial branch with the commits modified to be compatible with our 3.0.15 build in https://github.com/Algunenano/mapnik/tree/v3.0.15-carto_postgis_quoting

Pending reviewing Windshaft/Windshaft-cartodb for render-time-variables usages in queries.

@Algunenano
Copy link

I have it working locally with the branch above:
image

I haven't seen any usage of render-time variables but I have to do a deeper search.

@Algunenano
Copy link

I haven't found any usage of this kind of variables in Windshaft. @enekid did you see anything while you were investigating the issue?

@enekid
Copy link
Author

enekid commented Jan 30, 2018

Nothing. I was warned by @rafatower about this. Maybe he can guide us.

@Algunenano
Copy link

I've only seen it used in the 1000x branch (https://github.com/CartoDB/Windshaft-cartodb/blob/1000x-tt/lib/cartodb/utils/substitution-tokens.js) so I'm guessing we should be ok.

@Algunenano
Copy link

Merging. To be tested in next Mapnik release.

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

No branches or pull requests

2 participants