Skip to content

Fix a bug in diff expire handling #2492

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.

Sign up for GitHub

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

Merged
lonvia merged 1 commit into from
Jul 3, 2026
+142 −51
Merged

Fix a bug in diff expire handling #2492

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 56 additions & 51 deletions src/flex-table-column.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,24 +214,6 @@ void flex_table_column_t::add_expire(expire_config_t const &config)
}

namespace {

/**
* This expires all geometries in "geoms" by themselves. Used when we don't
* need diff expire.
*/
void separate_expire(std::vector<geom::geometry_t> const &geoms,
expire_config_t const &expire_config,
expire_tiles_t &expire_tiles,
std::vector<expire_output_t> *expire_outputs)
{
assert(expire_outputs);

for (auto const &geom : geoms) {
expire_tiles.from_geometry(geom, expire_config);
}
expire_tiles.commit_tiles(&expire_outputs->at(expire_config.expire_output));
}

/**
* When doing diff expire, we need to calculate the symmetric difference
* between old and new geometries. The difference is done by type, so points
Expand Down Expand Up @@ -282,28 +264,14 @@ void classify_geometries(T input_geoms, geom::multipoint_t *points,
}
// NOLINTEND(cppcoreguidelines-rvalue-reference-param-not-moved)

template <typename T>
void diff_and_expire(geom::multigeometry_t<T> const &old_geoms,
geom::multigeometry_t<T> const &new_geoms,
expire_config_t const &expire_config,
expire_tiles_t &expire_tiles)
{
std::vector<T> diffs;
boost::geometry::sym_difference(old_geoms, new_geoms, diffs);
for (auto const &geom : diffs) {
expire_tiles.from_geometry(geom, expire_config);
}
}

void diff_expire(std::vector<geom::geometry_t> *geoms_old,
std::vector<geom::geometry_t> *geoms_new,
expire_config_t const &expire_config,
expire_tiles_t &expire_tiles,
std::vector<expire_output_t> *expire_outputs)
void find_difference(std::vector<geom::geometry_t> *geoms_old,
std::vector<geom::geometry_t> *geoms_new,
std::vector<geom::point_t> *diff_points,
std::vector<geom::linestring_t> *diff_linestrings,
std::vector<geom::polygon_t> *diff_polygons)
{
assert(geoms_old);
assert(geoms_new);
assert(expire_outputs);

geom::multipoint_t old_points;
geom::multilinestring_t old_linestrings;
Expand All @@ -319,12 +287,10 @@ void diff_expire(std::vector<geom::geometry_t> *geoms_old,
classify_geometries(geoms_new, &new_points, &new_linestrings,
&new_polygons);

diff_and_expire(old_points, new_points, expire_config, expire_tiles);
diff_and_expire(old_linestrings, new_linestrings, expire_config,
expire_tiles);
diff_and_expire(old_polygons, new_polygons, expire_config, expire_tiles);

expire_tiles.commit_tiles(&expire_outputs->at(expire_config.expire_output));
boost::geometry::sym_difference(old_points, new_points, *diff_points);
boost::geometry::sym_difference(old_linestrings, new_linestrings,
*diff_linestrings);
boost::geometry::sym_difference(old_polygons, new_polygons, *diff_polygons);
}

} // anonymous namespace
Expand All @@ -340,19 +306,58 @@ void flex_table_column_t::do_expire(
assert(expire);
assert(expire_outputs);

// Sometimes it doesn't depend on the expire config whether we want to
// do diff expire.
bool const always_separate =
!enable_diff_expire || geoms_old->empty() || geoms_new->empty();

bool need_diff_expire = false;

for (auto const &expire_config : m_expires) {
assert(expire_config.expire_output < expire->size());
auto &expire_tiles = expire->at(expire_config.expire_output);

if (!expire_config.diff_expire || !enable_diff_expire ||
geoms_old->empty() || geoms_new->empty()) {
separate_expire(*geoms_old, expire_config, expire_tiles,
expire_outputs);
separate_expire(*geoms_new, expire_config, expire_tiles,
expire_outputs);
if (!expire_config.diff_expire || always_separate) {
for (auto const &geom : *geoms_old) {
expire_tiles.from_geometry(geom, expire_config);
}
for (auto const &geom : *geoms_new) {
expire_tiles.from_geometry(geom, expire_config);
}
expire_tiles.commit_tiles(
&expire_outputs->at(expire_config.expire_output));
} else {
diff_expire(geoms_old, geoms_new, expire_config, expire_tiles,
expire_outputs);
need_diff_expire = true;
}
}

if (always_separate || !need_diff_expire) {
return;
}

std::vector<geom::point_t> diff_points;
std::vector<geom::linestring_t> diff_linestrings;
std::vector<geom::polygon_t> diff_polygons;

find_difference(geoms_old, geoms_new, &diff_points, &diff_linestrings,
&diff_polygons);

for (auto const &expire_config : m_expires) {
assert(expire_config.expire_output < expire->size());
auto &expire_tiles = expire->at(expire_config.expire_output);

if (expire_config.diff_expire) {
for (auto const &geom : diff_points) {
expire_tiles.from_geometry(geom, expire_config);
}
for (auto const &geom : diff_linestrings) {
expire_tiles.from_geometry(geom, expire_config);
}
for (auto const &geom : diff_polygons) {
expire_tiles.from_geometry(geom, expire_config);
}
expire_tiles.commit_tiles(
&expire_outputs->at(expire_config.expire_output));
}
}
}
86 changes: 86 additions & 0 deletions tests/bdd/flex/expire-diff.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,3 +359,89 @@ Feature: Diff expire
| 8 | 129 | 126 |
| 8 | 130 | 126 |

Scenario: multiple expire configs, with and without diff expire
Given the OSM data
"""
n1 v1 x0 y0
n2 v1 x2 y0
n3 v1 x2 y1
n4 v1 x4 y1
w1 v1 Thighway=primary Nn1,n2,n3,n4
"""
And the lua style
"""
local eo1 = osm2pgsql.define_expire_output({
table = 'osm2pgsql_test_expire1',
maxzoom = 8,
})

local eo2 = osm2pgsql.define_expire_output({
table = 'osm2pgsql_test_expire2',
maxzoom = 8,
})

local eo3 = osm2pgsql.define_expire_output({
table = 'osm2pgsql_test_expire3',
maxzoom = 7,
})

local the_table = osm2pgsql.define_way_table('osm2pgsql_test', {
{ column = 'geom', type = 'linestring', expire = {
{ output = eo1, diff_expire = true },
{ output = eo2, diff_expire = false },
{ output = eo3, diff_expire = true }
}
},
})

function osm2pgsql.process_way(object)
the_table:insert{
geom = object:as_linestring()
}
end
"""
When running osm2pgsql flex with parameters
| --slim | -c |
Then table osm2pgsql_test has 1 rows
Then table osm2pgsql_test contains exactly
| way_id | geom!geo |
| 1 | 0 0,222638.98158654713 0,222638.98158654713 111325.14285463623,445277.96317309426 111325.14285463623 |
Then table osm2pgsql_test_expire1 has 0 rows
Then table osm2pgsql_test_expire2 has 0 rows
Then table osm2pgsql_test_expire3 has 0 rows

Given the OSM data
"""
n2 v2 x0 y1
"""
When running osm2pgsql flex with parameters
| --slim | -a |
Then table osm2pgsql_test contains exactly
| way_id | geom!geo |
| 1 | 0 0,0 111325.14285463623,222638.98158654713 111325.14285463623,445277.96317309426 111325.14285463623 |
Then table osm2pgsql_test_expire1 contains exactly
| zoom | x | y |
| 8 | 127 | 127 |
| 8 | 128 | 127 |
| 8 | 129 | 127 |
| 8 | 127 | 128 |
| 8 | 128 | 128 |
| 8 | 129 | 128 |

Then table osm2pgsql_test_expire2 contains exactly
| zoom | x | y |
| 8 | 127 | 127 |
| 8 | 128 | 127 |
| 8 | 129 | 127 |
| 8 | 130 | 127 |
| 8 | 127 | 128 |
| 8 | 128 | 128 |
| 8 | 129 | 128 |

Then table osm2pgsql_test_expire3 contains exactly
| zoom | x | y |
| 7 | 63 | 63 |
| 7 | 63 | 64 |
| 7 | 64 | 63 |
| 7 | 64 | 64 |

Loading