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

null not equal empty string (back compatibility issue) #1859

Closed
springmeyer opened this issue May 22, 2013 · 11 comments
Closed

null not equal empty string (back compatibility issue) #1859

springmeyer opened this issue May 22, 2013 · 11 comments
Milestone

Comments

@springmeyer
Copy link
Member

In Mapnik 2.2 you can now do [attr] != null to filter out true nulls after the fixes as part of #1642. But in older Mapnik you could not do [attr] != null and instead had to match null by doing [attr] != ''. Now [attr] != '' only rejects empty strings and actually matches nulls since null != ''. Whether null should be == to '' is a bigger question than this issue, but an interesting one to consider in relation to #796

This behavior causes a breaking change between Mapnik 2.1.x and 2.2.x and makes using stylesheets between them difficult since [attr] != null does not work in 2.1.x and older.

Reported downstream at http://support.mapbox.com/discussions/tilemill/6202-tilemill-bug-on-null-postgis-values-after-update-to-v0101-59 from @cquest

So, need to figure out if adding back(wards) compatibility with [attr] != '' matching nulls is needed.

@gravitystorm
Copy link

Yes, I found that with 2.2.0-pre - I needed to rewrite many of my stylesheets, since they relied on the foo != '' syntax to mean non-null values in postgis results. Now, foo != '' also matches both non-empty strings and also null values, which leads to features being unexpectedly symbolized.

At the least, there should be something in the release notes for 2.2.0 to say that stylesheets will need a minor reworking. Sorry for not reporting this when I first ran into it!

@springmeyer
Copy link
Member Author

@gravitystorm - thanks for the follow up. So, are you happy with the change and the ability to now filter out nulls by doing foo != null? I presume you fixed your stylesheets by doing foo != null and foo != '' instead of just foo != ''?

As far as API changes like this, I am collecting them at https://github.com/mapnik/mapnik/wiki/API-changes-between-v2.1-and-v2.2. I will add this one shortly.

@cquest
Copy link

cquest commented May 30, 2013

I'm not really happy with this kind of change. It breaks too many existing stylesheets from my point of view.

springmeyer pushed a commit that referenced this issue May 31, 2013
Make [attr] != '' match nulls: Followup to #1642, closes #1859
@springmeyer
Copy link
Member Author

Okay, @cquest I have added back compatibility to master/2.2.0 release. So, [attr]!= '' should now (again) like 2.1.x and older match both nulls and empty strings. This was done in the pull at #1880 and landed in master/2.2.0 in fdd9afd

@springmeyer
Copy link
Member Author

@cquest - what operating system are you on? (so I can get you a new TileMill dev build to test).

Also, @gravitystorm - I notice there are still a few usages of [attr]!= '' in the openstreetmap-carto stylesheets. This change should allow those stylesheets to work with both 2.1 and the upcoming 2.2. Let me know asap if you find that is not the case.

@cquest
Copy link

cquest commented Jun 1, 2013

Tanks a lot for the fix. I'm using TileMill on OSX and will be happy to test the new version.

@springmeyer
Copy link
Member Author

@cquest - please find an OS X dev build of TileMill (using mapnik latest @ c6da53a) at: http://tilemill.s3.amazonaws.com/dev/TileMill-v0.10.1-78-gbb8c6ce.zip

@springmeyer
Copy link
Member Author

fyi @gravitystorm and @cquest - based on the mis-rendering of the openstreets-dc stylesheet even with this build I presume mapbox/carto#247 is still somehow present.

@cquest
Copy link

cquest commented Jun 5, 2013

TileMill-v0.10.1-78-gbb8c6ce.zip looks fine with my OSM-FR tilemill project.

Thanks for the fix !

@springmeyer
Copy link
Member Author

Great. Thanks for testing!

On Jun 5, 2013, at 1:21 AM, cquest notifications@github.com wrote:

TileMill-v0.10.1-78-gbb8c6ce.zip looks fine with my OSM-FR tilemill project.

Thanks for the fix !


Reply to this email directly or view it on GitHub.

onepremise pushed a commit to onepremise/mapnik that referenced this issue Jun 14, 2013
onepremise pushed a commit to onepremise/mapnik that referenced this issue Jun 14, 2013
onepremise pushed a commit to onepremise/mapnik that referenced this issue Jun 14, 2013
This change adds back the ability to throw out both empty strings
and null values with the not equals statement like `[attr] != ''`

Note: this does not mean null == '' because the needed compatibility
fix only addresses the "not equals` operator to retain the historical
usage of not matching nulls.
onepremise pushed a commit to onepremise/mapnik that referenced this issue Jun 14, 2013
PetrDlouhy pushed a commit to PetrDlouhy/mapnik that referenced this issue Aug 22, 2013
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

3 participants