From c80fcda4c30e88fe37f5cba2e909c6840c1ab80c Mon Sep 17 00:00:00 2001 From: eyal rozen Date: Mon, 14 Sep 2026 19:00:50 +0300 Subject: [PATCH 1/2] refactor: compute polygon-interior tile rows through coords_to_tile from_polygon_area() computed its tile row range with inline Web Mercator arithmetic on the raw envelope, bypassing the projection's tile conversion that every other expiry path uses. For Web Mercator the conversion is the identity, so this is a no-op; for other projections it stops both bounds collapsing to the same row, which silently skipped polygon interiors. Refs: MAPCO-11657 --- src/expire-tiles.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/expire-tiles.cpp b/src/expire-tiles.cpp index d9d2b81ec..ad8b9aa5e 100644 --- a/src/expire-tiles.cpp +++ b/src/expire-tiles.cpp @@ -169,10 +169,10 @@ void expire_tiles_t::from_polygon_area(geom::polygon_t const &geom, // Coordinates are numbered from bottom to top, tiles are numbered from top // to bottom, so "min" and "max" are switched here. - auto const max_tile_y = static_cast( - m_map_width * (0.5 - box.min().y() / tile_t::EARTH_CIRCUMFERENCE)); - auto const min_tile_y = static_cast( - m_map_width * (0.5 - box.max().y() / tile_t::EARTH_CIRCUMFERENCE)); + auto const max_tile_y = + static_cast(coords_to_tile(box.min()).y()); + auto const min_tile_y = + static_cast(coords_to_tile(box.max()).y()); std::vector tile_x_list; From c59e78f2fb19c84427ad8ed6411df31ad6b0e040 Mon Sep 17 00:00:00 2001 From: eyal rozen Date: Mon, 14 Sep 2026 19:11:54 +0300 Subject: [PATCH 2/2] feat: expire tiles for geometry columns in any single projection The command-line expire options only picked up tables whose geometry column is Web Mercator, so a flex config storing EPSG:4326 produced an empty expire list. Expiry now takes the SRID from the geometry columns being expired, builds the tile conversion with osm2pgsql's own reprojection for it, and refuses to start if those columns disagree. The expire output stays in the Web Mercator tile scheme. The SRID is stored on the expire output, so the main output and its per-worker clones build their projection from the same value, at setup and never per row. Lua-defined expire outputs keep the Web Mercator default. Refs: MAPCO-11657 --- src/expire-output.hpp | 6 ++ src/flex-table-column.cpp | 2 +- src/output-flex.cpp | 35 +++++--- tests/bdd/flex/expire-non-mercator.feature | 68 ++++++++++++++++ tests/test-expire-from-geometry.cpp | 95 ++++++++++++++++++++++ 5 files changed, 194 insertions(+), 12 deletions(-) create mode 100644 tests/bdd/flex/expire-non-mercator.feature diff --git a/src/expire-output.hpp b/src/expire-output.hpp index 312ed7de4..6fb681c49 100644 --- a/src/expire-output.hpp +++ b/src/expire-output.hpp @@ -10,6 +10,7 @@ * For a full list of authors see the git log. */ +#include "projection.hpp" #include "tile.hpp" #include @@ -59,6 +60,9 @@ class expire_output_t uint32_t maxzoom() const noexcept { return m_maxzoom; } void set_maxzoom(uint32_t maxzoom) noexcept { m_maxzoom = maxzoom; } + int srid() const noexcept { return m_srid; } + void set_srid(int srid) noexcept { m_srid = srid; } + std::size_t max_tiles_geometry() const noexcept { return m_max_tiles_geometry; @@ -142,6 +146,8 @@ class expire_output_t /// Zoom level we capture tiles on uint32_t m_maxzoom = 0; + int m_srid = PROJ_SPHERE_MERC; + /** * The following two settings are for protecting osm2pgsql from overload as * well as downstream tile expiry mechanisms in case of large changes to diff --git a/src/flex-table-column.cpp b/src/flex-table-column.cpp index 98a9cd4e1..33f9c1181 100644 --- a/src/flex-table-column.cpp +++ b/src/flex-table-column.cpp @@ -209,7 +209,7 @@ std::string flex_table_column_t::sql_create() const void flex_table_column_t::add_expire(expire_config_t const &config) { assert(is_geometry_column()); - assert(srid() == PROJ_SPHERE_MERC); + assert(srid() > 0); m_expires.push_back(config); } diff --git a/src/output-flex.cpp b/src/output-flex.cpp index 379e111d6..dc9fcf896 100644 --- a/src/output-flex.cpp +++ b/src/output-flex.cpp @@ -1315,7 +1315,7 @@ output_flex_t::output_flex_t(output_flex_t const *other, for (auto &expire_output : *m_expire_outputs) { m_expire_tiles.emplace_back( expire_output.maxzoom(), - reprojection_t::create_projection(PROJ_SPHERE_MERC), + reprojection_t::create_projection(expire_output.srid()), expire_output.max_tiles_geometry()); } } @@ -1358,17 +1358,30 @@ output_flex_t::output_flex_t(std::shared_ptr const &mid, eo.set_minzoom(options.expire_tiles_zoom_min); eo.set_maxzoom(options.expire_tiles_zoom); + flex_table_t const *srid_table = nullptr; for (auto &table : *m_tables) { - if (table.has_geom_column() && - table.geom_column().srid() == PROJ_SPHERE_MERC) { - expire_config_t config{}; - config.expire_output = m_expire_outputs->size() - 1; - if (options.expire_tiles_max_bbox > 0.0) { - config.mode = expire_mode::hybrid; - config.full_area_limit = options.expire_tiles_max_bbox; - } - table.geom_column().add_expire(config); + if (!table.has_geom_column()) { + continue; + } + auto const srid = table.geom_column().srid(); + if (!srid_table) { + srid_table = &table; + eo.set_srid(srid); + } else if (srid != eo.srid()) { + throw fmt_error( + "Tile expiry needs all tables with a geometry column to use" + " the same projection, but table '{}' uses SRID {} and" + " table '{}' uses SRID {}.", + srid_table->name(), eo.srid(), table.name(), srid); + } + + expire_config_t config{}; + config.expire_output = m_expire_outputs->size() - 1; + if (options.expire_tiles_max_bbox > 0.0) { + config.mode = expire_mode::hybrid; + config.full_area_limit = options.expire_tiles_max_bbox; } + table.geom_column().add_expire(config); } } @@ -1382,7 +1395,7 @@ output_flex_t::output_flex_t(std::shared_ptr const &mid, for (auto const &expire_output : *m_expire_outputs) { m_expire_tiles.emplace_back( expire_output.maxzoom(), - reprojection_t::create_projection(PROJ_SPHERE_MERC), + reprojection_t::create_projection(expire_output.srid()), expire_output.max_tiles_geometry()); } diff --git a/tests/bdd/flex/expire-non-mercator.feature b/tests/bdd/flex/expire-non-mercator.feature new file mode 100644 index 000000000..f4e4a7cf4 --- /dev/null +++ b/tests/bdd/flex/expire-non-mercator.feature @@ -0,0 +1,68 @@ +Feature: Expire with command line options on geometries not in Web Mercator + + Scenario: Tables in EPSG:4326 are expired + Given the lua style + """ + local points = osm2pgsql.define_node_table('osm2pgsql_test_points', { + { column = 'geom', type = 'point', projection = 4326 }, + }) + + function osm2pgsql.process_node(object) + points:insert({ geom = object:as_point() }) + end + """ + And the OSM data + """ + n10 v1 dV Tamenity=restaurant x10.0 y10.0 + """ + When running osm2pgsql flex with parameters + | --slim | -c | + Then execution is successful + + Given the OSM data + """ + n10 v2 dV Tamenity=restaurant x10.5 y10.5 + """ + When running osm2pgsql flex with parameters + | --slim | -a | --expire-tiles=12 | --expire-output=expire.list | + Then execution is successful + And the error output contains + """ + entries to expire output [0]. + """ + + Scenario: Tables in different projections can not share command line expire + Given the lua style + """ + local merc = osm2pgsql.define_node_table('osm2pgsql_test_merc', { + { column = 'geom', type = 'point', projection = 3857 }, + }) + + local latlon = osm2pgsql.define_node_table('osm2pgsql_test_latlon', { + { column = 'geom', type = 'point', projection = 4326 }, + }) + + function osm2pgsql.process_node(object) + merc:insert({ geom = object:as_point() }) + latlon:insert({ geom = object:as_point() }) + end + """ + And the OSM data + """ + n10 v1 dV Tamenity=restaurant x10.0 y10.0 + """ + When running osm2pgsql flex with parameters + | --slim | -c | + Then execution is successful + + Given the OSM data + """ + n10 v2 dV Tamenity=restaurant x10.5 y10.5 + """ + When running osm2pgsql flex with parameters + | --slim | -a | --expire-tiles=12 | + Then execution fails + And the error output contains + """ + Tile expiry needs all tables with a geometry column to use the same projection + """ diff --git a/tests/test-expire-from-geometry.cpp b/tests/test-expire-from-geometry.cpp index 49ff65b40..5e1f26a61 100644 --- a/tests/test-expire-from-geometry.cpp +++ b/tests/test-expire-from-geometry.cpp @@ -9,6 +9,9 @@ #include +#include + +#include #include #include #include @@ -24,10 +27,39 @@ namespace { std::shared_ptr defproj{ reprojection_t::create_projection(PROJ_SPHERE_MERC)}; +std::shared_ptr latlonproj{ + reprojection_t::create_projection(PROJ_LATLONG)}; + // We are using zoom level 12 here, because at that level a tile is about // 10,000 units wide/high which gives us easy numbers to work with. constexpr uint32_t ZOOM = 12; +geom::point_t merc_to_latlon(geom::point_t const &point) +{ + auto const c = osmium::geom::mercator_to_lonlat( + osmium::geom::Coordinates{point.x(), point.y()}); + return {c.x, c.y}; +} + +template +TLIST merc_to_latlon(TLIST const &list) +{ + TLIST result; + for (auto const &point : list) { + result.push_back(merc_to_latlon(point)); + } + return result; +} + +template +quadkey_list_t expire(std::shared_ptr const &projection, + TGEOM const &geom, expire_config_t const &expire_config) +{ + expire_tiles_t et{ZOOM, projection}; + et.from_geometry(geom, expire_config); + return et.get_tiles(); +} + } // anonymous namespace TEST_CASE("expire null geometry does nothing", "[NoDB]") @@ -506,3 +538,66 @@ TEST_CASE("expire doesn't do anything if not in 3857", "[NoDB]") auto const tiles = et.get_tiles(); REQUIRE(tiles.empty()); } + +TEST_CASE("expire point in 4326 matches web mercator", "[NoDB]") +{ + expire_config_t const expire_config; + geom::point_t const pt{5000.0, 5000.0}; + + auto const merc_tiles = expire(defproj, pt, expire_config); + REQUIRE(merc_tiles.size() == 1); + REQUIRE(expire(latlonproj, merc_to_latlon(pt), expire_config) == + merc_tiles); +} + +TEST_CASE("expire linestring in 4326 matches web mercator", "[NoDB]") +{ + expire_config_t const expire_config; + geom::linestring_t const line{{5000.0, 5000.0}, {5000.0, 15000.0}}; + + auto const merc_tiles = expire(defproj, line, expire_config); + REQUIRE(merc_tiles.size() == 2); + REQUIRE(expire(latlonproj, merc_to_latlon(line), expire_config) == + merc_tiles); +} + +TEST_CASE("expire polygon boundary in 4326 matches web mercator", "[NoDB]") +{ + expire_config_t expire_config; + expire_config.mode = expire_mode::boundary_only; + geom::ring_t const ring{{5000.0, 5000.0}, + {25000.0, 5000.0}, + {25000.0, 25000.0}, + {5000.0, 25000.0}, + {5000.0, 5000.0}}; + + auto const merc_tiles = + expire(defproj, geom::polygon_t{geom::ring_t{ring}}, expire_config); + REQUIRE(merc_tiles.size() == 8); + REQUIRE(expire(latlonproj, geom::polygon_t{merc_to_latlon(ring)}, + expire_config) == merc_tiles); +} + +TEST_CASE("expire polygon interior in 4326 matches web mercator", "[NoDB]") +{ + expire_config_t expire_config; + expire_config.mode = expire_mode::full_area; + geom::ring_t const ring{{5000.0, 5000.0}, + {25000.0, 5000.0}, + {25000.0, 25000.0}, + {5000.0, 25000.0}, + {5000.0, 5000.0}}; + + auto const merc_tiles = + expire(defproj, geom::polygon_t{geom::ring_t{ring}}, expire_config); + auto const latlon_tiles = expire( + latlonproj, geom::polygon_t{merc_to_latlon(ring)}, expire_config); + + REQUIRE(merc_tiles.size() == 9); + REQUIRE(latlon_tiles == merc_tiles); + + // The one tile no part of the boundary touches. + auto const interior = tile_t{ZOOM, 2049, 2046}.quadkey(); + REQUIRE(std::find(latlon_tiles.cbegin(), latlon_tiles.cend(), interior) != + latlon_tiles.cend()); +}