From 02a3e41faebf4d564bb372af2b4fb9687fd271fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Ronzon?= Date: Thu, 24 Sep 2026 14:50:35 -0600 Subject: [PATCH 1/4] fix(massif): add && bbox pre-filter to massif spatial joins The spatial joins resolving which massif contains an entrance called ST_Contains with no && bounding-box pre-filter. ST_Contains is a function call, not an indexable operator, so PostGIS could not use the GiST index on t_massif(geog_polygon) and evaluated exact point-in-polygon against every massif polygon. On production the per-entrance variant was the second-largest consumer of database time: 39.3 hours cumulative over 1,222,480 calls (115.9 ms mean) with avg_rows = 0, meaning most of that work produced no rows at all. EXPLAIN (ANALYZE, BUFFERS) measured on production, per call: entrance in a massif 47.67 ms -> 0.18 ms 32,407 -> 14 buffers entrance in no massif 45.79 ms -> 0.09 ms 32,401 -> 5 buffers The old plan compounded two problems: ~6,400 exact ST_Contains evaluations per call, and t_massif on the outer side of the nested loop, which re-scanned t_entrance by primary key 6,401 times to re-read the same row (25,604 of those buffers). It also launched parallel workers on every call. Two operand forms are needed, because point_geom is geometry while geog_polygon is geography: - entrance fixed, massifs scanned: e.point_geom && m.geog_polygon, so the implicit geometry->geography cast makes idx_t_massif_geog reachable - massif fixed by id, entrances scanned: e.point_geom && m.geog_polygon::geometry, since the t_entrance GiST index is geometry FIND_NETWORKS_IN_MASSIF needed the massif lifted out of a scalar subquery into a join to express the pre-filter. Its behaviour is unchanged: a missing massif or null polygon still yields no rows, and m.id = $1 is a primary key match so there is no row fan-out. Verified on production that no massif polygon crosses the antimeridian, so each geodetic bounding box is a superset of its planar one and the pre-filter cannot exclude a row that ST_Contains would have matched. Also batches the massif lookup for the two search-reindex fan-outs. MassifService.findMassifsByEntranceIds() now owns the batched join as the single copy of that SQL, shared with the dbSync export, and updateInSearch() accepts already-resolved massifs so massif/create and massif/mark-sensitive issue one query instead of one per entrance. The CSV import is left per-entrance deliberately: each call is now ~0.1 ms against a path that already issues many queries per row. Closes #1811 --- api/controllers/v1/massif/create.js | 9 +++- api/controllers/v1/massif/mark-sensitive.js | 9 +++- api/dbSync/entities/entrance.js | 33 ++++--------- api/services/CaveService.js | 2 +- api/services/EntranceService.js | 41 +++++++++++----- api/services/GeoLocService.js | 2 + api/services/MassifService.js | 52 ++++++++++++++++++++- 7 files changed, 106 insertions(+), 42 deletions(-) diff --git a/api/controllers/v1/massif/create.js b/api/controllers/v1/massif/create.js index 820b760d1..0bbdf2bcb 100644 --- a/api/controllers/v1/massif/create.js +++ b/api/controllers/v1/massif/create.js @@ -110,10 +110,17 @@ module.exports = async (req, res) => { }); if (newMassif.isSensitive) { + // Resolve the containing massifs in one batch so the fan-out does not run a + // spatial query per entrance. + const massifsByEntrance = + await MassifService.findMassifsByEntranceIds(updatedEntranceIds); await Promise.all( updatedEntranceIds.map(async (id) => { const populated = await EntranceService.getPopulatedEntrance(id); - if (populated) await EntranceService.updateInSearch(populated); + if (populated) + await EntranceService.updateInSearch(populated, { + massifs: massifsByEntrance[id] ?? [], + }); }) ); } diff --git a/api/controllers/v1/massif/mark-sensitive.js b/api/controllers/v1/massif/mark-sensitive.js index da5b7de9d..bc0ad9d85 100644 --- a/api/controllers/v1/massif/mark-sensitive.js +++ b/api/controllers/v1/massif/mark-sensitive.js @@ -57,13 +57,18 @@ module.exports = async (req, res) => { req.token.id ); - // Update search index for each affected entrance + // Update search index for each affected entrance. The containing massifs are + // resolved in one batch so the fan-out does not run a spatial query per entrance. + const massifsByEntrance = + await MassifService.findMassifsByEntranceIds(updatedEntranceIds); await Promise.all( updatedEntranceIds.map(async (id) => { const populatedEntrance = await EntranceService.getPopulatedEntrance(id); if (populatedEntrance) { - await EntranceService.updateInSearch(populatedEntrance); + await EntranceService.updateInSearch(populatedEntrance, { + massifs: massifsByEntrance[id] ?? [], + }); } }) ); diff --git a/api/dbSync/entities/entrance.js b/api/dbSync/entities/entrance.js index 31f820ac4..27aa38ce9 100644 --- a/api/dbSync/entities/entrance.js +++ b/api/dbSync/entities/entrance.js @@ -5,7 +5,6 @@ const { } = require('../../../config/constants/entrance'); const { getQualityData } = require('../../utils/computeEntranceDataQuality'); const { computeCommentsRating } = require('../../utils/commentsRating'); -const CommonService = require('../../services/CommonService'); const query = ` SELECT @@ -108,30 +107,14 @@ async function* processRows(source) { await Promise.all(joins.map((e) => exportUtils.joinMany(e))); - // Spatial join: find massifs containing each entrance - const ids = rows.map((r) => r.id); - const massifQuery = ` - SELECT e.id AS id_entrance, m.id AS id_massif, n.name AS massif_name, n.id_language AS language - FROM t_entrance e - JOIN t_massif m ON ST_Contains(m.geog_polygon::geometry, e.point_geom) - LEFT JOIN t_name n ON n.id_massif = m.id AND n.is_main = true AND n.is_deleted = false - WHERE e.id = ANY($1::int[]) - AND e.is_deleted = false - AND m.is_deleted = false - `; - const { rows: massifRows } = await CommonService.query(massifQuery, [ids]); - const massifsByEntrance = {}; - for (const mr of massifRows) { - if (!massifsByEntrance[mr.id_entrance]) { - massifsByEntrance[mr.id_entrance] = []; - } - massifsByEntrance[mr.id_entrance].push({ - id: mr.id_massif, - name: mr.massif_name, - language: mr.language, - isDeleted: false, - }); - } + // Spatial join: find massifs containing each entrance. + // Had to require in the function to avoid a circular dependency: SearchService + // requires this module at load time to read its search schema. + // eslint-disable-next-line global-require + const MassifService = require('../../services/MassifService'); + const massifsByEntrance = await MassifService.findMassifsByEntranceIds( + rows.map((r) => r.id) + ); for (const row of rows) { if (row.geology) row.geology = row.geology.trim(); diff --git a/api/services/CaveService.js b/api/services/CaveService.js index 28e203c41..37eedd739 100644 --- a/api/services/CaveService.js +++ b/api/services/CaveService.js @@ -123,7 +123,7 @@ module.exports = { const query = ` SELECT DISTINCT m.* FROM t_massif AS m - JOIN t_entrance AS e ON ST_Contains(m.geog_polygon::geometry, e.point_geom) + JOIN t_entrance AS e ON e.point_geom && m.geog_polygon AND ST_Contains(m.geog_polygon::geometry, e.point_geom) WHERE e.id_cave = $1 AND e.is_deleted = false AND m.is_deleted = false diff --git a/api/services/EntranceService.js b/api/services/EntranceService.js index dc1686819..cb6a0bc5e 100644 --- a/api/services/EntranceService.js +++ b/api/services/EntranceService.js @@ -5,6 +5,17 @@ const INTEREST_ENTRANCES_QUERY = // query to get a random entrance of interest const RANDOM_ENTRANCE_QUERY = `${INTEREST_ENTRANCES_QUERY} ORDER BY RANDOM() LIMIT 1`; +// query to get the massifs containing a single entrance. +// The `&&` bounding-box pre-filter is what lets PostGIS use the GiST index on +// t_massif(geog_polygon); without it every massif polygon is tested exactly. +const MASSIFS_CONTAINING_ENTRANCE_QUERY = ` + SELECT m.id, n.name, n.id_language AS language + FROM t_massif m + JOIN t_entrance e ON e.point_geom && m.geog_polygon AND ST_Contains(m.geog_polygon::geometry, e.point_geom) + LEFT JOIN t_name n ON n.id_massif = m.id AND n.is_main = true AND n.is_deleted = false + WHERE e.id = $1 AND e.is_deleted = false AND m.is_deleted = false +`; + const CommonService = require('./CommonService'); const SearchService = require('./SearchService'); const NotificationService = require('./NotificationService'); @@ -413,7 +424,15 @@ module.exports = { await SearchService.deleteDocument('entrances', entranceId); }, - async updateInSearch(populatedEntrance) { + /** + * @param {*} populatedEntrance + * @param {object} [options] + * @param {Array} [options.massifs] Containing massifs, already resolved by the + * caller. Supply this from bulk paths that hold a list of entrance ids (see + * MassifService.findMassifsByEntranceIds) to skip the per-entrance spatial + * lookup; pass an empty array for an entrance in no massif. + */ + async updateInSearch(populatedEntrance, { massifs } = {}) { // Warning: All linked entities may contain sensitive information (same as in document). // For example, the complete caver object for the 'author' and 'reviewer' fields. // Although we could leave them intact, since search results also pass through the converter, @@ -468,7 +487,8 @@ module.exports = { entrance.longitude = null; } - // Compute data quality score and fetch massifs in parallel (independent queries) + // Compute data quality score and fetch massifs in parallel (independent queries). + // The spatial lookup is skipped when the caller already resolved the massifs. const [qualityRows, massifRows] = await Promise.all([ CommonService.query( `SELECT general_latest_date_of_update, general_nb_contributions, @@ -481,25 +501,24 @@ module.exports = { FROM v_data_quality_compute_entrance WHERE id_entrance = $1 ORDER BY id_massif ASC LIMIT 1`, [rawEntrance.id] ), - CommonService.query( - `SELECT m.id, n.name, n.id_language AS language - FROM t_massif m - JOIN t_entrance e ON ST_Contains(m.geog_polygon::geometry, e.point_geom) - LEFT JOIN t_name n ON n.id_massif = m.id AND n.is_main = true AND n.is_deleted = false - WHERE e.id = $1 AND e.is_deleted = false AND m.is_deleted = false`, - [rawEntrance.id] - ), + massifs + ? null + : CommonService.query(MASSIFS_CONTAINING_ENTRANCE_QUERY, [ + rawEntrance.id, + ]), ]); entrance.dataQuality = qualityRows?.rows?.[0] ? getQualityData(qualityRows.rows[0]) : 0; entrance.massifs = + massifs ?? massifRows?.rows?.map((r) => ({ id: r.id, name: r.name, language: r.language, isDeleted: false, - })) ?? []; + })) ?? + []; await SearchService.updateDocument('entrances', entrance); }, diff --git a/api/services/GeoLocService.js b/api/services/GeoLocService.js index ff1693c0f..6c3a441e1 100644 --- a/api/services/GeoLocService.js +++ b/api/services/GeoLocService.js @@ -164,6 +164,7 @@ const MASSIFS_IN_BOUNDS = ` SELECT COUNT(e.id)::integer FROM t_entrance AS e WHERE e.is_deleted = false + AND e.point_geom && m.geog_polygon::geometry AND ST_Contains(m.geog_polygon::geometry, e.point_geom) ) AS "entranceCount", ( @@ -173,6 +174,7 @@ const MASSIFS_IN_BOUNDS = ` JOIN t_cave AS c ON c.id = e.id_cave WHERE e.is_deleted = false AND c.is_deleted = false + AND e.point_geom && m.geog_polygon::geometry AND ST_Contains(m.geog_polygon::geometry, e.point_geom) GROUP BY c.id HAVING COUNT(e.id) > 1 diff --git a/api/services/MassifService.js b/api/services/MassifService.js index 633aaecc9..faa1409fe 100644 --- a/api/services/MassifService.js +++ b/api/services/MassifService.js @@ -89,8 +89,10 @@ const FIND_NETWORKS_IN_MASSIF = ` SELECT c.*, c.length AS "caveLength", count(e.id_cave) as "nbEntrances" FROM t_entrance AS e LEFT JOIN t_cave c ON c.id = e.id_cave + JOIN t_massif AS m ON m.id = $1 WHERE c.is_deleted = false - AND ST_Contains((SELECT geog_polygon::geometry FROM t_massif WHERE id = $1 ), e.point_geom) + AND e.point_geom && m.geog_polygon::geometry + AND ST_Contains(m.geog_polygon::geometry, e.point_geom) GROUP BY c.id HAVING count(e.id_cave) > 1 `; @@ -104,7 +106,8 @@ const FIND_CAVES_IN_MASSIF = ` FROM t_cave AS c JOIN t_entrance AS e ON e.id_cave = c.id JOIN t_massif AS m ON m.id = $1 - WHERE ST_Contains(m.geog_polygon::geometry, e.point_geom) + WHERE e.point_geom && m.geog_polygon::geometry + AND ST_Contains(m.geog_polygon::geometry, e.point_geom) AND c.is_deleted = false AND e.is_deleted = false `; @@ -142,6 +145,19 @@ const COUNT_LOCKED_UNSENSITIVE_ENTRANCES_IN_MASSIF = ` AND e.is_sensitive_locked = true `; +// Resolves the containing massifs for a batch of entrances in one round trip. +// Shared by the search-index update path and the dbSync export so the spatial +// join exists in exactly one place. +const FIND_MASSIFS_BY_ENTRANCE_IDS = ` + SELECT e.id AS id_entrance, m.id AS id_massif, n.name AS massif_name, n.id_language AS language + FROM t_entrance e + JOIN t_massif m ON e.point_geom && m.geog_polygon AND ST_Contains(m.geog_polygon::geometry, e.point_geom) + LEFT JOIN t_name n ON n.id_massif = m.id AND n.is_main = true AND n.is_deleted = false + WHERE e.id = ANY($1::int[]) + AND e.is_deleted = false + AND m.is_deleted = false +`; + // Spatial queries using ST_Contains can throw when point_geom is null rather than // returning an empty result set. This wrapper normalises that to an empty array. async function querySpatialRows(sql, param) { @@ -282,6 +298,38 @@ module.exports = { getCaves: async (massifId) => querySpatialRows(FIND_CAVES_IN_MASSIF, massifId), + + /** + * Resolve the containing massifs for many entrances in a single query. + * + * Callers that already hold a list of entrance ids should use this instead of + * letting EntranceService.updateInSearch run its per-entrance spatial lookup, + * which turns a bulk operation into N round trips. + * + * @param {number[]} entranceIds + * @returns {Promise>} entrance id -> massifs (ids absent + * from the result simply have no containing massif) + */ + async findMassifsByEntranceIds(entranceIds) { + if (!entranceIds?.length) return {}; + const { rows } = await CommonService.query(FIND_MASSIFS_BY_ENTRANCE_IDS, [ + entranceIds, + ]); + const massifsByEntrance = {}; + for (const row of rows) { + if (!massifsByEntrance[row.id_entrance]) { + massifsByEntrance[row.id_entrance] = []; + } + massifsByEntrance[row.id_entrance].push({ + id: row.id_massif, + name: row.massif_name, + language: row.language, + isDeleted: false, + }); + } + return massifsByEntrance; + }, + countEntrances: async (massifId) => { try { const result = await CommonService.query(COUNT_ENTRANCES_IN_MASSIF, [ From acfebb3de6ba1039bce6f9d5d2e2ee2536df5b0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Ronzon?= Date: Thu, 24 Sep 2026 15:52:21 -0600 Subject: [PATCH 2/4] fix(massif): align the superseded v_region_info definition sql/2_2025_11_07_region_info_view.sql joins on ST_Contains with the point built inline by ST_MakePoint(e.longitude, e.latitude), so neither the GiST index on t_massif(geog_polygon) nor the one on t_entrance(point_geom) can be used. That definition is dead. 91_materialized_views.sql drops and recreates the view with the correct e.point_geom && m.geog_polygon form, and sorts after this file in the byte order the postgres initdb entrypoint uses, over the sql/ directory docker-compose mounts as docker-entrypoint-initdb.d. Verified on production: all four materialized views carry the pre-filter, so no migration is needed. Align the join anyway, because this is the one place left in sql/ that models a spatial join without a pre-filter, and add a comment pointing at 91_ as the authoritative definition so the file is not read as a model. --- sql/2_2025_11_07_region_info_view.sql | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/sql/2_2025_11_07_region_info_view.sql b/sql/2_2025_11_07_region_info_view.sql index 91ed900a1..aecd3219d 100644 --- a/sql/2_2025_11_07_region_info_view.sql +++ b/sql/2_2025_11_07_region_info_view.sql @@ -1,6 +1,13 @@ \c grottoce; -- Create v_region_info materialized view for region statistics +-- +-- Superseded: sql/91_materialized_views.sql drops and recreates this view, and +-- sorts after this file, so that definition is the one a built database holds. +-- The join below is kept in step with it so this file is not copied as a model +-- for spatial joins — ST_Contains alone cannot use an index, and building the +-- point with ST_MakePoint instead of the indexed point_geom column rules out +-- the GiST index on t_entrance as well. See #1811. CREATE MATERIALIZED VIEW v_region_info AS SELECT e.iso_3166_2 as id_region, c.id as id_cave, @@ -13,7 +20,7 @@ CREATE MATERIALIZED VIEW v_region_info AS FROM t_entrance e LEFT JOIN t_cave c ON e.id_cave = c.id AND c.is_deleted = false LEFT JOIN t_name n ON n.id_cave = c.id AND n.is_main = true - LEFT JOIN t_massif m ON ST_Contains(ST_SetSRID(m.geog_polygon::geometry, 4326), ST_SetSRID(ST_MakePoint(e.longitude, e.latitude), 4326)) + LEFT JOIN t_massif m ON e.point_geom && m.geog_polygon AND ST_Contains(m.geog_polygon::geometry, e.point_geom) AND m.is_deleted = false WHERE e.is_deleted = false AND e.iso_3166_2 IS NOT NULL From 5c772b79cd68ddb8eb93900dba464f1f86d746c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Ronzon?= Date: Thu, 24 Sep 2026 16:01:22 -0600 Subject: [PATCH 3/4] fix(massif): reject polygons straddling the antimeridian MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The && bounding-box pre-filter is equivalent to a bare ST_Contains only while a massif polygon stays on one side of the 180° meridian. The pre-filter compares geographies, so its box follows great-circle edges; ST_Contains compares geometries, so its box is planar. For a polygon that straddles the antimeridian the two invert, measured on a box off Fiji drawn at longitude 179.8 to 180.2: entrance at longitude 180 (inside the drawn box) ST_Contains f && t entrance at longitude 0 (20,000 km away) ST_Contains t && f So the join returns nothing where it used to return one wrong row. That was already broken before the pre-filter existed, but the invariant the pre-filter relies on was only ever verified against the corpus as it stood, never enforced, and massif polygons are user-drawn. Two shapes reach validatePolygon. The likely one is a longitude outside [-180, 180]: the map reports coordinates from a repeated world copy when the user pans past the edge, and the geometry -> geography cast on write wraps them (PostGIS: "Coordinate values were coerced into range"), turning a small box into one spanning nearly 360°. A polygon already in range but spanning more than 180° is the other. Both are rejected with POLYGON_CROSSES_ANTIMERIDIAN. The existing checks do not cover this. The Fiji box is 1879 km², well inside the 35000 km² cap, and valid in 2D, so ST_IsValid and ST_Area on geography both pass. The bounds ride along in the ST_IsValidDetail round trip rather than adding one, since they are geometry arithmetic and cannot raise on a geometry that is not computable as a geography. The span test is > 180 rather than >= 180, which leaves the geoJsonAntipodalEdge fixture spanning exactly 180° to the ST_Area XX000 path that already reports it as POLYGON_ANTIPODAL_EDGE. This also protects the six spatial joins that carried the pre-filter before this branch, in dbSync/entities/massif.js, the three COUNT_*_IN_MASSIF queries, isPointInSensitiveMassif and propagateSensitivityToEntrances, plus the four materialized views in 91_materialized_views.sql. Tests cover both rejected shapes at the service level and through the create and update routes, plus the invariant itself: that the geodetic bounding box covers the planar one for an accepted polygon and fails to for a rejected one. --- api/services/MassifService.js | 53 +++++++++++++++++-- .../1_services/MassifService.test.js | 48 +++++++++++++++++ .../integration/4_routes/Massifs/FAKE_DATA.js | 36 +++++++++++++ .../4_routes/Massifs/create.test.js | 24 +++++++++ .../4_routes/Massifs/update.test.js | 18 +++++++ 5 files changed, 175 insertions(+), 4 deletions(-) diff --git a/api/services/MassifService.js b/api/services/MassifService.js index faa1409fe..d8836fa4e 100644 --- a/api/services/MassifService.js +++ b/api/services/MassifService.js @@ -62,6 +62,7 @@ const POLYGON_ERROR_MESSAGES = [ const POLYGON_AREA_EXCEEDED = 'POLYGON_AREA_EXCEEDED'; const POLYGON_INVALID = 'POLYGON_INVALID'; +const POLYGON_CROSSES_ANTIMERIDIAN = 'POLYGON_CROSSES_ANTIMERIDIAN'; /** * Find the first matching error for a raw PostGIS/GEOS error string. @@ -187,8 +188,8 @@ module.exports = { /** * Validate a polygon's geometry and area constraints. - * Checks basic validity (ST_IsValid), geographic computability (ST_Area on - * geography), and maximum area limit. + * Checks basic validity (ST_IsValid), antimeridian span, geographic + * computability (ST_Area on geography), and maximum area limit. * @param {string} wktPolygon - WKT representation of the polygon * @returns {Promise<{code: string, message: string}|null>} null if valid, error object if invalid */ @@ -196,16 +197,60 @@ module.exports = { // Check basic geometry validity (self-intersections, etc.) // ST_IsValidDetail performs a single GEOS pass and returns both the // validity flag and the reason, avoiding a redundant traversal. - const validityQuery = `SELECT valid AS is_valid, reason FROM ST_IsValidDetail($1::geometry)`; + // The longitude bounds ride along in the same round trip: they are plain + // geometry arithmetic, so unlike ST_Area they cannot raise on a geometry + // that is valid in the plane but not computable as a geography. + const validityQuery = ` + WITH g AS (SELECT $1::geometry AS geom) + SELECT d.valid AS is_valid, + d.reason, + ST_XMin(g.geom) AS lon_min, + ST_XMax(g.geom) AS lon_max + FROM g, ST_IsValidDetail(g.geom) d + `; const validityResult = await CommonService.query(validityQuery, [ wktPolygon, ]); - const { is_valid: isValid, reason } = validityResult.rows[0]; + const { + is_valid: isValid, + reason, + lon_min: lonMin, + lon_max: lonMax, + } = validityResult.rows[0]; if (!isValid) { sails.log.warn(`Polygon validation failed (ST_IsValid): ${reason}`); return matchPolygonError(reason); } + // Reject a polygon that straddles the 180° meridian. + // + // Every spatial join matching entrances to massifs pairs an && bounding-box + // pre-filter with an exact ST_Contains (see #1811). The pre-filter compares + // geographies, so its box follows great-circle edges, while ST_Contains + // compares geometries, so its box is planar. The two agree only while a + // polygon stays on one side of the antimeridian; for one that straddles it + // they invert, and the join silently returns nothing. + // + // Two shapes get here. A polygon with a longitude outside [-180, 180] is + // the common one: the map hands back coordinates from a repeated world copy + // when the user pans past the edge, and the geometry -> geography cast on + // write then wraps them (PostGIS: "Coordinate values were coerced into + // range"), turning a small box into one spanning nearly 360°. A polygon + // already in range but spanning more than 180° is the other. + // + // This is what keeps the pre-filter provably equivalent to a bare + // ST_Contains, so it has to hold for stored data, not just for the corpus + // that happened to be clean when the pre-filter was added. + if (lonMin < -180 || lonMax > 180 || lonMax - lonMin > 180) { + sails.log.warn( + `Polygon validation failed (antimeridian): longitude ${lonMin} to ${lonMax}` + ); + return { + code: POLYGON_CROSSES_ANTIMERIDIAN, + message: `The polygon crosses the 180° meridian (longitude ${lonMin} to ${lonMax}). Please draw it on a single side of the antimeridian; if you panned past the edge of the map, pan back before drawing.`, + }; + } + // Compute area — also catches geometry errors that only manifest when // casting to geography (e.g. invalid winding order, antipodal edges). // Error structure: Waterline wraps pg errors as OperationalError with diff --git a/test/integration/1_services/MassifService.test.js b/test/integration/1_services/MassifService.test.js index 1085ffd57..6717e9ac0 100644 --- a/test/integration/1_services/MassifService.test.js +++ b/test/integration/1_services/MassifService.test.js @@ -70,6 +70,54 @@ describe('MassifService', () => { }); }); + describe('validatePolygon', () => { + it('should accept a polygon away from the antimeridian', async () => { + const wkt = await MassifService.geoJsonToWKT(massifPolygon.geoJsonSmall); + should(await MassifService.validatePolygon(wkt)).be.null(); + }); + + it('should reject a polygon straddling the antimeridian', async () => { + const wkt = await MassifService.geoJsonToWKT( + massifPolygon.geoJsonCrossesAntimeridian + ); + const error = await MassifService.validatePolygon(wkt); + should(error).not.be.null(); + should(error.code).equal('POLYGON_CROSSES_ANTIMERIDIAN'); + should(error.message).match(/180° meridian/); + }); + + it('should reject a polygon spanning more than 180° of longitude', async () => { + const wkt = await MassifService.geoJsonToWKT( + massifPolygon.geoJsonWideLongitudeSpan + ); + const error = await MassifService.validatePolygon(wkt); + should(error).not.be.null(); + should(error.code).equal('POLYGON_CROSSES_ANTIMERIDIAN'); + }); + + // The point of the check: a polygon it lets through cannot be one where the + // && pre-filter and ST_Contains disagree, because the geodetic bounding box + // is then a superset of the planar one. See #1811. + it('should only accept polygons where the bbox pre-filter agrees with ST_Contains', async () => { + const accepted = await MassifService.geoJsonToWKT( + massifPolygon.geoJsonSmall + ); + const rejected = await MassifService.geoJsonToWKT( + massifPolygon.geoJsonCrossesAntimeridian + ); + const geodeticBoxCoversPlanar = (wkt) => + `ST_Covers(Box2D(${wkt}::geometry::geography::geometry)::geometry, + Box2D(${wkt}::geometry)::geometry)`; + const { rows } = await CommonService.query( + `SELECT ${geodeticBoxCoversPlanar('$1')} AS accepted_holds, + ${geodeticBoxCoversPlanar('$2')} AS rejected_holds`, + [accepted, rejected] + ); + should(rows[0].accepted_holds).be.true(); + should(rows[0].rejected_holds).be.false(); + }); + }); + describe('deleteInSearch', () => { it('should delete massif from search index', async () => { const originalEnv = process.env.NODE_ENV; diff --git a/test/integration/4_routes/Massifs/FAKE_DATA.js b/test/integration/4_routes/Massifs/FAKE_DATA.js index fd6684e24..73a9cb20c 100644 --- a/test/integration/4_routes/Massifs/FAKE_DATA.js +++ b/test/integration/4_routes/Massifs/FAKE_DATA.js @@ -173,6 +173,40 @@ const geoJsonSelfIntersecting = { ], }; +// A small polygon off Fiji straddling the 180° meridian, with a longitude past +// 180 as the map reports it when the user pans into a repeated world copy. +// Only 1879 km², so it clears the area cap, and valid in 2D, so ST_IsValid and +// ST_Area on geography both pass — the antimeridian check is the only thing +// standing between it and a stored polygon that spans nearly 360° of longitude +// once the geography cast wraps it. See #1811. +const geoJsonCrossesAntimeridian = { + type: 'Polygon', + coordinates: [ + [ + [179.8, -17.9], + [180.2, -17.9], + [180.2, -17.5], + [179.8, -17.5], + [179.8, -17.9], + ], + ], +}; + +// The same failure with every longitude already inside [-180, 180]: a thin band +// spanning 340° of longitude, which no bounding box can pre-filter usefully. +const geoJsonWideLongitudeSpan = { + type: 'Polygon', + coordinates: [ + [ + [-170, 0], + [170, 0], + [170, 0.01], + [-170, 0.01], + [-170, 0], + ], + ], +}; + const massifPolygon = { geoJson1, geoJson2, @@ -181,6 +215,8 @@ const massifPolygon = { geoJsonSharedEdgeMultiPolygon, geoJsonAntipodalEdge, geoJsonSelfIntersecting, + geoJsonCrossesAntimeridian, + geoJsonWideLongitudeSpan, geoJson1ToString: JSON.stringify(geoJson1), geoJson2ToString: JSON.stringify(geoJson2), geoJsonSmallToString: JSON.stringify(geoJsonSmall), diff --git a/test/integration/4_routes/Massifs/create.test.js b/test/integration/4_routes/Massifs/create.test.js index 8bef0f917..b0cbc38e3 100644 --- a/test/integration/4_routes/Massifs/create.test.js +++ b/test/integration/4_routes/Massifs/create.test.js @@ -67,6 +67,30 @@ describe('Massif features', () => { }); }); + describe('Antimeridian-crossing polygon (#1811)', () => { + it('should return code 400 when the polygon straddles the 180° meridian', (done) => { + supertest(sails.hooks.http.app) + .post('/api/v1/massifs') + .send({ + name: 'Fiji Massif', + description: 'straddles the antimeridian', + descriptionTitle: 'Title', + descriptionAndNameLanguage: { id: 'fra' }, + geogPolygon: massifPolygon.geoJsonCrossesAntimeridian, + }) + .set('Authorization', adminToken) + .set('Content-type', 'application/json') + .set('Accept', 'application/json') + .expect(400) + .end((err, res) => { + if (err) return done(err); + should(res.body.code).equal('POLYGON_CROSSES_ANTIMERIDIAN'); + should(res.body.message).match(/180° meridian/); + return done(); + }); + }); + }); + describe('Shared-edge MultiPolygon (#1606)', () => { it('should return 400 with POLYGON_SELF_INTERSECTION for shared-edge MultiPolygon', (done) => { supertest(sails.hooks.http.app) diff --git a/test/integration/4_routes/Massifs/update.test.js b/test/integration/4_routes/Massifs/update.test.js index 8881facf1..bbf0e93a7 100644 --- a/test/integration/4_routes/Massifs/update.test.js +++ b/test/integration/4_routes/Massifs/update.test.js @@ -188,6 +188,24 @@ describe('Massif features', () => { }); }); + it('should return 400 when polygon straddles the 180° meridian (#1811)', (done) => { + supertest(sails.hooks.http.app) + .put(`/api/v1/massifs/${testMassifId}`) + .send({ + geogPolygon: massifPolygon.geoJsonCrossesAntimeridian, + }) + .set('Authorization', userToken) + .set('Content-type', 'application/json') + .set('Accept', 'application/json') + .expect(400) + .end((err, res) => { + if (err) return done(err); + should(res.body.code).equal('POLYGON_CROSSES_ANTIMERIDIAN'); + should(res.body.message).match(/180° meridian/); + return done(); + }); + }); + it('should return 400 when polygon has invalid geometry (#1606)', (done) => { supertest(sails.hooks.http.app) .put(`/api/v1/massifs/${testMassifId}`) From de1c04ca6fc4a0ee28c719b907923ece485d1f83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Ronzon?= Date: Mon, 28 Sep 2026 14:52:20 -0600 Subject: [PATCH 4/4] fix(massif): use the geometry pre-filter form at massif-fixed joins The three COUNT_*_IN_MASSIF queries, propagateSensitivityToEntrances and the dbSync massif export all fix the massif by id, so their pre-filter belongs in the ::geometry form used by the other massif-fixed joins. Review read the geography form there as costing an index scan on t_entrance, which it was not. ST_Contains carries a PostGIS index support function that rewrites it into an e.point_geom @ m.geog_polygon::geometry index condition by itself, so idx_entrance_geom_gist was reached either way. What the cast buys is equivalence - its box is exactly the box of the ST_Contains argument beside it, so these joins stop depending on the antimeridian invariant - plus one fewer per-row (e.point_geom)::geography cast in the filter. Verified plan-neutral on PostGIS 3.4.3 and PostgreSQL 16, with the same index, the same index condition and cost equal to two decimal places. Suite unchanged at 3478 passing, 4 failing, the same pre-existing #1772 failures. --- api/dbSync/entities/massif.js | 2 +- api/services/MassifService.js | 14 ++++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/api/dbSync/entities/massif.js b/api/dbSync/entities/massif.js index 28ca73cd9..9c39ac260 100644 --- a/api/dbSync/entities/massif.js +++ b/api/dbSync/entities/massif.js @@ -17,7 +17,7 @@ const query = ` LEFT JOIN t_name n ON n.id_massif = m.id AND n.is_main = true LEFT JOIN t_caver a ON a.id = m.id_author LEFT JOIN t_caver r ON r.id = m.id_reviewer - LEFT JOIN t_entrance e ON e.point_geom && m.geog_polygon AND ST_Contains(m.geog_polygon::geometry, e.point_geom) AND e.is_deleted = false + LEFT JOIN t_entrance e ON e.point_geom && m.geog_polygon::geometry AND ST_Contains(m.geog_polygon::geometry, e.point_geom) AND e.is_deleted = false WHERE m.is_deleted = false GROUP BY m.id, m.geog_polygon, n.name, n.id_language, r.nickname, a.nickname ORDER BY m.id ASC diff --git a/api/services/MassifService.js b/api/services/MassifService.js index d8836fa4e..eeefbfd6f 100644 --- a/api/services/MassifService.js +++ b/api/services/MassifService.js @@ -102,6 +102,12 @@ const FIND_NETWORKS_IN_MASSIF = ` // point_geom. A cave with no entrance will not appear in these results, which // is acceptable because every cave in the domain model must have at least one // entrance. +// +// The massif is fixed by id here, so the pre-filter carries the ::geometry cast. +// Its box is then exactly the box of the ST_Contains argument beside it, which +// makes the pre-filter provably unable to drop a row ST_Contains would match. +// The geography form (no cast) belongs only where the entrance is the fixed side +// and idx_t_massif_geog is the index to reach — see FIND_MASSIFS_BY_ENTRANCE_IDS. const FIND_CAVES_IN_MASSIF = ` SELECT DISTINCT c.* FROM t_cave AS c @@ -117,7 +123,7 @@ const COUNT_ENTRANCES_IN_MASSIF = ` SELECT COUNT(e.id)::integer AS count FROM t_entrance AS e JOIN t_massif AS m - ON e.point_geom && m.geog_polygon AND ST_Contains(m.geog_polygon::geometry, e.point_geom) + ON e.point_geom && m.geog_polygon::geometry AND ST_Contains(m.geog_polygon::geometry, e.point_geom) WHERE m.id = $1 AND e.is_deleted = false `; @@ -126,7 +132,7 @@ const COUNT_UNSENSITIVE_ENTRANCES_IN_MASSIF = ` SELECT COUNT(e.id)::integer AS count FROM t_entrance AS e JOIN t_massif AS m - ON e.point_geom && m.geog_polygon AND ST_Contains(m.geog_polygon::geometry, e.point_geom) + ON e.point_geom && m.geog_polygon::geometry AND ST_Contains(m.geog_polygon::geometry, e.point_geom) WHERE m.id = $1 AND e.is_deleted = false AND e.is_sensitive = false @@ -139,7 +145,7 @@ const COUNT_LOCKED_UNSENSITIVE_ENTRANCES_IN_MASSIF = ` SELECT COUNT(e.id)::integer AS count FROM t_entrance AS e JOIN t_massif AS m - ON e.point_geom && m.geog_polygon AND ST_Contains(m.geog_polygon::geometry, e.point_geom) + ON e.point_geom && m.geog_polygon::geometry AND ST_Contains(m.geog_polygon::geometry, e.point_geom) WHERE m.id = $1 AND e.is_deleted = false AND e.is_sensitive = false @@ -482,7 +488,7 @@ module.exports = { FROM t_entrance AS e JOIN t_massif AS m ON m.id = $1 WHERE e.is_deleted = false - AND e.point_geom && m.geog_polygon + AND e.point_geom && m.geog_polygon::geometry AND ST_Contains(m.geog_polygon::geometry, e.point_geom) AND e.is_sensitive = false AND e.is_sensitive_locked = false