fix: probe actual GIS capability instead of inferring it from the DB platform - #30
fix: probe actual GIS capability instead of inferring it from the DB platform#30torvalstrom wants to merge 1 commit into
Conversation
…platform A PostgreSQL platform does not imply PostGIS. On a plain postgres server (the default for most Nextcloud installs) detectGisType() returned GIS_TYPE_POSTGRES and every queryPoint() call ran PostGIS SQL that can only fail: one logged error and one doomed query per photo per cron run (~8.6k log lines/day on a ~30k-photo library) before falling back. Probe capability instead of assuming it: - postgres: check pg_extension for 'postgis' - mysql/mariadb: try a trivial ST_Contains() (built-in since MySQL 5.7 / MariaDB 10.2, but a stripped build should also degrade cleanly) When the probe fails, detectGisType() returns GIS_TYPE_NONE so queryPoint() goes straight to fallbackByFileId(), which resolves places from the oc_memories_places table Memories precomputes at index time - place names still resolve fully, with zero errors. Verified on Nextcloud 33 + plain postgres 16: occ journeys:cluster over 64k photos, 1105 clusters, 0 resolver errors (previously ~47k/day).
|
Superseded by #36, which fixes the underlying cause rather than detecting it. The probe in this PR was the right call given what I knew at the time, but it treats PostGIS as the missing ingredient. It isn't: Memories stores #36 uses the built-in operators instead — no extension needed, and indexable against the GiST Closing in favour of #36. Thanks for your patience with the two rounds. |
Problem
SimplePlaceResolver::detectGisType()infers GIS capability from the DB platform name: any PostgreSQL platform is assumed to have PostGIS. On a plain postgres server (no PostGIS extension - the common case for Nextcloud installs) everyqueryPoint()call then runs spatial SQL that can only fail:error_logline per photo per cron run - on a ~30k-photo library that was ~8,600 log lines/day, and ~47k/day on a 64k-photo libraryFix
Probe the actual capability once, in the constructor path:
SELECT 1 FROM pg_extension WHERE extname = 'postgis'ST_Contains()(built-in since MySQL 5.7 / MariaDB 10.2, but a stripped build also degrades cleanly this way)If the probe fails,
detectGisType()returnsGIS_TYPE_NONE, soqueryPoint()goes straight tofallbackByFileId()- which resolves places from theoc_memories_placestable that Memories precomputes at index time. Place names still resolve fully; all three callers already passfileid.Verified
Running this exact change in production on Nextcloud 33 + plain postgres 16 since July:
occ journeys:clusterover 64,341 photos → 1,105 clusters, 0 resolver errors (previously tens of thousands per day). Behavior on a server that does have PostGIS is unchanged (probe returns true, spatial path used as before).🤖 Generated with Claude Code