From e7964003771d10874d98b5a22347db26e2eda27e Mon Sep 17 00:00:00 2001 From: Erica Fischer Date: Fri, 8 Mar 2024 13:01:12 -0800 Subject: [PATCH] Fix null behavior in in/ni expressions (#214) * Fix null behavior in in/ni expressions * Add test for null attributes in "ni" filters * Remove unidecode from "in" expression evaluation again --- CHANGELOG.md | 5 +++ Makefile | 10 +++++ evaluator.cpp | 52 ++++++++++++++---------- mvt.cpp | 2 +- tests/pbf/12-2145-1391-filter1.pbf.json | 15 +++++++ tests/pbf/12-2145-1391-filter2.pbf.json | 5 +++ tests/pbf/12-2145-1391.pbf | Bin 0 -> 284 bytes version.hpp | 2 +- 8 files changed, 67 insertions(+), 24 deletions(-) create mode 100644 tests/pbf/12-2145-1391-filter1.pbf.json create mode 100644 tests/pbf/12-2145-1391-filter2.pbf.json create mode 100644 tests/pbf/12-2145-1391.pbf diff --git a/CHANGELOG.md b/CHANGELOG.md index 6296a2af..024ab468 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +# 2.51.0 + +* Fix null behavior in "in" expressions +* But they are back to not using unidecode + # 2.50.0 * FSL-style "in" expressions use unidecode again diff --git a/Makefile b/Makefile index 1384daad..a39ac049 100644 --- a/Makefile +++ b/Makefile @@ -335,6 +335,16 @@ overzoom-test: tippecanoe-overzoom ./tippecanoe-decode tests/muni/out/out.dir/000.pbf 0 0 0 > tests/muni/out/out.dir/overzoomed.json cmp tests/muni/out/out.dir/overzoomed.json tests/muni/out/out.dir/direct.json rm -rf tests/muni/out/out.dir tests/muni/out/out.mbtiles tests/muni/out/out.dir/overzoomed.json tests/muni/out/out.dir/direct.json + # Test filter with null attribute + ./tippecanoe-overzoom -j '{"*":["name","ni",[1,5,6,9]]}' -o tests/pbf/12-2145-1391-filter1.pbf tests/pbf/12-2145-1391.pbf 12/2145/1391 12/2145/1391 + ./tippecanoe-decode tests/pbf/12-2145-1391-filter1.pbf 12 2145 1391 > tests/pbf/12-2145-1391-filter1.pbf.json.check + cmp tests/pbf/12-2145-1391-filter1.pbf.json.check tests/pbf/12-2145-1391-filter1.pbf.json + rm tests/pbf/12-2145-1391-filter1.pbf.json.check tests/pbf/12-2145-1391-filter1.pbf + # Test filter with null attribute in "ni" list + ./tippecanoe-overzoom -j '{"*":["name","ni",[1,5,6,9,null]]}' -o tests/pbf/12-2145-1391-filter2.pbf tests/pbf/12-2145-1391.pbf 12/2145/1391 12/2145/1391 + ./tippecanoe-decode tests/pbf/12-2145-1391-filter2.pbf 12 2145 1391 > tests/pbf/12-2145-1391-filter2.pbf.json.check + cmp tests/pbf/12-2145-1391-filter2.pbf.json.check tests/pbf/12-2145-1391-filter2.pbf.json + rm tests/pbf/12-2145-1391-filter2.pbf.json.check tests/pbf/12-2145-1391-filter2.pbf join-test: tippecanoe tippecanoe-decode tile-join ./tippecanoe -q -f -z12 -o tests/join-population/tabblock_06001420.mbtiles -YALAND10:'Land area' -L'{"file": "tests/join-population/tabblock_06001420.json", "description": "population"}' diff --git a/evaluator.cpp b/evaluator.cpp index 2aa2e726..1f528226 100644 --- a/evaluator.cpp +++ b/evaluator.cpp @@ -343,11 +343,9 @@ static int eval(std::function feature, json_obje strcmp(f->value.array.array[1]->value.string.string, "is") == 0 || strcmp(f->value.array.array[1]->value.string.string, "isnt") == 0 || false)) { - mvt_value lhs; - lhs.type = mvt_null; // attributes that aren't found are nulls - mvt_value ff = feature(std::string(f->value.array.array[0]->value.string.string)); - if (ff.type != mvt_no_such_key) { - lhs = ff; + mvt_value lhs = feature(std::string(f->value.array.array[0]->value.string.string)); + if (lhs.type == mvt_no_such_key) { + lhs.type = mvt_null; } if (f->value.array.array[2]->type == JSON_NULL && strcmp(f->value.array.array[1]->value.string.string, "is") == 0) { @@ -357,15 +355,15 @@ static int eval(std::function feature, json_obje return lhs.type != mvt_null; // null isnt null => false, anything isnt null => true } - if (lhs.type == mvt_null) { - return -1; // null op anything => null - } - bool fail = false; if (f->value.array.array[2]->type == JSON_STRING && (strcmp(f->value.array.array[1]->value.string.string, "cn") == 0 || strcmp(f->value.array.array[1]->value.string.string, "nc") == 0)) { + if (lhs.type == mvt_null) { + return -1; // null cn/nc anything => null + } + std::string s = mvt_value_to_string(lhs, fail, unidecode_data); if (fail) { return -1; // null cn anything => false @@ -387,22 +385,28 @@ static int eval(std::function feature, json_obje if (f->value.array.array[2]->type == JSON_ARRAY && (strcmp(f->value.array.array[1]->value.string.string, "in") == 0 || strcmp(f->value.array.array[1]->value.string.string, "ni") == 0)) { - std::string s = mvt_value_to_string(lhs, fail, unidecode_data); - if (fail) { - return -1; // null in anything => false - } + // no null check here, since null can be in or not in bool contains = false; for (size_t i = 0; i < f->value.array.array[2]->value.array.length; i++) { - fail = false; - int cmp = compare_fsl(ff, f->value.array.array[2]->value.array.array[i], fail, unidecode_data); - if (fail) { - continue; // null - } + if (lhs.type == mvt_null) { + if (f->value.array.array[2]->value.array.array[i]->type == JSON_NULL) { + contains = true; // null is null + break; + } else { + contains = false; // null isnt non-null + } + } else { + fail = false; + static std::vector no_unidecode_data; + int cmp = compare_fsl(lhs, f->value.array.array[2]->value.array.array[i], fail, no_unidecode_data); - if (cmp == 0) { - contains = true; - break; + if (fail) { + contains = false; // non-null isnt null + } else if (cmp == 0) { + contains = true; + break; + } } } @@ -413,7 +417,11 @@ static int eval(std::function feature, json_obje } } - int cmp = compare_fsl(ff, f->value.array.array[2], fail, unidecode_data); + if (lhs.type == mvt_null) { + return -1; // null compared to anything => null + } + + int cmp = compare_fsl(lhs, f->value.array.array[2], fail, unidecode_data); if (fail) { return -1; // null } diff --git a/mvt.cpp b/mvt.cpp index 04c1ef9d..1628fe57 100644 --- a/mvt.cpp +++ b/mvt.cpp @@ -601,7 +601,7 @@ std::string mvt_value::toString() const { case mvt_null: return "null"; default: - return "unknown"; + return "unknown " + std::to_string(type); } } diff --git a/tests/pbf/12-2145-1391-filter1.pbf.json b/tests/pbf/12-2145-1391-filter1.pbf.json new file mode 100644 index 00000000..59ea6601 --- /dev/null +++ b/tests/pbf/12-2145-1391-filter1.pbf.json @@ -0,0 +1,15 @@ +{ "type": "FeatureCollection", "properties": { "zoom": 12, "x": 2145, "y": 1391 }, "features": [ +{ "type": "FeatureCollection", "properties": { "layer": "parsed", "version": 2, "extent": 4096 }, "features": [ +{ "type": "Feature", "id": 1, "properties": { "osm_id": "192162", "felt:cluster_size": 1 }, "geometry": { "type": "Point", "coordinates": [ 8.585773, 49.852166 ] } } +, +{ "type": "Feature", "id": 516, "properties": { "osm_id": "317282344", "felt:cluster_size": 1 }, "geometry": { "type": "Point", "coordinates": [ 8.591459, 49.856800 ] } } +, +{ "type": "Feature", "id": 545, "properties": { "osm_id": "330843617", "felt:cluster_size": 1 }, "geometry": { "type": "Point", "coordinates": [ 8.591051, 49.857063 ] } } +, +{ "type": "Feature", "id": 546, "properties": { "osm_id": "330843827", "felt:cluster_size": 1 }, "geometry": { "type": "Point", "coordinates": [ 8.592553, 49.851875 ] } } +, +{ "type": "Feature", "id": 547, "properties": { "osm_id": "330844309", "felt:cluster_size": 1 }, "geometry": { "type": "Point", "coordinates": [ 8.581738, 49.854047 ] } } +, +{ "type": "Feature", "id": 41908, "properties": { "name": "August-Euler-Flugplatz", "wikipedia": "de:August-Euler-Flugplatz", "ele": "102", "osm_way_id": "23872501", "felt:cluster_size": 1 }, "geometry": { "type": "Point", "coordinates": [ 8.586588, 49.853687 ] } } +] } +] } diff --git a/tests/pbf/12-2145-1391-filter2.pbf.json b/tests/pbf/12-2145-1391-filter2.pbf.json new file mode 100644 index 00000000..531951af --- /dev/null +++ b/tests/pbf/12-2145-1391-filter2.pbf.json @@ -0,0 +1,5 @@ +{ "type": "FeatureCollection", "properties": { "zoom": 12, "x": 2145, "y": 1391 }, "features": [ +{ "type": "FeatureCollection", "properties": { "layer": "parsed", "version": 2, "extent": 4096 }, "features": [ +{ "type": "Feature", "id": 41908, "properties": { "name": "August-Euler-Flugplatz", "wikipedia": "de:August-Euler-Flugplatz", "ele": "102", "osm_way_id": "23872501", "felt:cluster_size": 1 }, "geometry": { "type": "Point", "coordinates": [ 8.586588, 49.853687 ] } } +] } +] } diff --git a/tests/pbf/12-2145-1391.pbf b/tests/pbf/12-2145-1391.pbf new file mode 100644 index 0000000000000000000000000000000000000000..52f631b220fd5fef1e3da3ee04811a417998696b GIT binary patch literal 284 zcmV+%0ptE3iwFP!000001CzSKRKdi>R*+a!oSLH1pdiJTUz{7CnIa{amYP#ym7G&r zT#{N8Uz}N$D#enQn42oaS)Q4lS&*8NnJC4anv*KU1=3KSSP9gm#LC5NXketo!Nq21 zX=G?-q{PL=VPtG!Ze(g;sKm|1X>4e2WMO1%VglwE8(5ean;DuTI2J|-j)}2>rIG}f zm}6-=&<(n-r8%iZx^6k8=><87B~?n&T#_lNRyZV?6c~jBB^ZTR7#JBjIT)2#Isa(? z(-#r~3o!$QT3CQ$eFhJ-pkgdQv4tS97y7$kVyr;1MIf;+1}hApVr)RM#UQa3&3C#& il0Y#oCU#~HpkgMV%$DO!K&cZti}V1nZjFXl0RRAkV|YXW literal 0 HcmV?d00001 diff --git a/version.hpp b/version.hpp index 52ff0912..1929bc14 100644 --- a/version.hpp +++ b/version.hpp @@ -1,6 +1,6 @@ #ifndef VERSION_HPP #define VERSION_HPP -#define VERSION "v2.50.0" +#define VERSION "v2.51.0" #endif