Repair geographic migration on populated databases

packages/db/migrations/0023_geographic_queries.sql

@@ -1,7 +1,9 @@
 -- Geographic reads without a PostGIS dependency. Expression index also covers
 -- existing rows; no re-ingestion or stored-column table rewrite is required.
+-- CREATE INDEX restricts search_path while evaluating existing rows. Capture
+-- the installation path so nested geographic helpers still resolve there.
 create function ndb_coord(v text, lim double precision) returns double precision
-language plpgsql immutable parallel safe as $$
+language plpgsql immutable parallel safe set search_path from current as $$
 declare n double precision;
 begin
   if v is null or btrim(v) = '' then return null; end if;
@@ -14,7 +16,7 @@ end $$;
 -- Coverage takes precedence over a receiver/site point. Unknown or malformed
 -- coverage must not silently fall back to a receiver's physical location.
 create function ndb_geo_shape(d jsonb) returns jsonb
-language plpgsql immutable parallel safe as $$
+language plpgsql immutable parallel safe set search_path from current as $$
 declare p jsonb; x double precision; y double precision;
 begin
   if d ? 'coverage' and d->'coverage' <> 'null'::jsonb then return d->'coverage'; end if;
@@ -30,15 +32,15 @@ begin
 end $$;
 
 create function ndb_distance(x1 double precision, y1 double precision, x2 double precision, y2 double precision)
-returns double precision language sql immutable strict parallel safe as $$
+returns double precision language sql immutable strict parallel safe set search_path from current as $$
   select 12742017.6 * asin(sqrt(least(1.0, greatest(0.0,
     power(sin(radians(y2-y1)/2),2) + cos(radians(y1))*cos(radians(y2))*power(sin(radians(x2-x1)/2),2)))));
 $$;
 
 -- A conservative longitude/latitude envelope. A crossing or polar envelope is
 -- widened to the full longitude range so the index never loses a match.
 create function ndb_radius_box(x double precision, y double precision, radius_m double precision)
-returns box language plpgsql immutable strict parallel safe as $$
+returns box language plpgsql immutable strict parallel safe set search_path from current as $$
 declare dy double precision := degrees(radius_m / 6371008.8); dx double precision;
 begin
   if abs(y) + dy >= 90 then dx := 180;
@@ -48,7 +50,7 @@ begin
 end $$;
 
 create function ndb_geo_box(d jsonb) returns box
-language plpgsql immutable parallel safe as $$
+language plpgsql immutable parallel safe set search_path from current as $$
 declare g jsonb := ndb_geo_shape(d); p jsonb; ring jsonb; poly jsonb; polys jsonb;
   x double precision; y double precision; r double precision;
   west double precision := 180; east double precision := -180;
@@ -85,7 +87,7 @@ end $$;
 -- subdivided before spherical distance so longitude/latitude segments follow
 -- the same path as map renderers (rather than a single great-circle arc).
 create function ndb_polygon_distance(poly jsonb, x double precision, y double precision)
-returns double precision language plpgsql immutable parallel safe as $$
+returns double precision language plpgsql immutable parallel safe set search_path from current as $$
 declare ring jsonb; p jsonb; pts text; px double precision; py double precision;
   firstx double precision; prevx double precision; prevy double precision;
   qx double precision; inside boolean := false; hole boolean := false; idx integer := 0;
@@ -124,7 +126,7 @@ begin
 end $$;
 
 create function ndb_geo_distance(d jsonb, x double precision, y double precision)
-returns double precision language plpgsql immutable strict parallel safe as $$
+returns double precision language plpgsql immutable strict parallel safe set search_path from current as $$
 declare g jsonb := ndb_geo_shape(d); poly jsonb; best double precision := 'Infinity';
 begin
   if ndb_geo_box(d) is null then return null; end if;

packages/db/migrations/0024_geographic_function_search_path.sql

@@ -0,0 +1,10 @@
+-- Also repair installations where 0023 succeeded with an empty items table.
+-- Existing populated installations rolled 0023 back and use its repaired
+-- definitions first. Keep the same path for calls during CREATE INDEX/REINDEX.
+alter function ndb_coord(text, double precision) set search_path from current;
+alter function ndb_geo_shape(jsonb) set search_path from current;
+alter function ndb_distance(double precision, double precision, double precision, double precision) set search_path from current;
+alter function ndb_radius_box(double precision, double precision, double precision) set search_path from current;
+alter function ndb_geo_box(jsonb) set search_path from current;
+alter function ndb_polygon_distance(jsonb, double precision, double precision) set search_path from current;
+alter function ndb_geo_distance(jsonb, double precision, double precision) set search_path from current;

test/geo-migration.test.js

@@ -0,0 +1,78 @@
+import { afterAll, beforeAll, expect, test } from 'bun:test';
+import { readFile } from 'node:fs/promises';
+import { PGlite } from '@electric-sql/pglite';
+
+let db;
+beforeAll(async () => {
+  db = await PGlite.create();
+  await db.exec(`
+    create table items (id integer primary key, data jsonb not null);
+    insert into items values
+      (1, '{}'),
+      (2, '{"location":{"lat":40,"lon":-74}}'),
+      (3, '{"coverage":{"type":"Circle","coordinates":[-74,40],"radius_m":1000}}'),
+      (4, '{"coverage":{"type":"Polygon","coordinates":[[[-75,39],[-73,39],[-73,41],[-75,41],[-75,39]]]}}'),
+      (5, '{"location":{"lat":"unknown","lon":-74}}');
+  `);
+  const migration = await readFile(
+    new URL('../packages/db/migrations/0023_geographic_queries.sql', import.meta.url),
+    'utf8',
+  );
+  await db.exec(`begin; ${migration} commit;`);
+}, 60_000);
+afterAll(async () => db?.close());
+
+test('the geographic migration builds its index over existing rows', async () => {
+  const { rows } = await db.query(
+    'select id from items where ndb_geo_box(data) is not null order by id',
+  );
+  expect(rows.map((row) => row.id)).toEqual([2, 3, 4]);
+  expect((await db.query('select count(*)::int as n from items')).rows[0].n).toBe(5);
+  const indexes = await db.query("select indexname from pg_indexes where tablename='items'");
+  expect(indexes.rows.map((row) => row.indexname)).toContain('items_geo_box_idx');
+});
+
+test('nested geographic helpers resolve under an index-maintenance search path', async () => {
+  await db.exec('set search_path = pg_catalog, pg_temp');
+  try {
+    const { rows } = await db.query(`
+      select id, public.ndb_geo_distance(data,-74,40) as distance
+      from public.items where public.ndb_geo_box(data) is not null order by id
+    `);
+    expect(rows).toEqual([
+      { id: 2, distance: 0 },
+      { id: 3, distance: 0 },
+      { id: 4, distance: 0 },
+    ]);
+    await db.exec('reindex index public.items_geo_box_idx');
+  } finally {
+    await db.exec('reset search_path');
+  }
+});
+
+test('the forward migration also repairs helpers installed before the fix', async () => {
+  const { rows } = await db.query(
+    "select oid::regprocedure::text as signature from pg_proc where proname like 'ndb_%'",
+  );
+  expect(rows).toHaveLength(7);
+  for (const { signature } of rows) await db.exec(`alter function ${signature} reset search_path`);
+  await db.exec(
+    await readFile(
+      new URL(
+        '../packages/db/migrations/0024_geographic_function_search_path.sql',
+        import.meta.url,
+      ),
+      'utf8',
+    ),
+  );
+  await db.exec('set search_path = pg_catalog, pg_temp');
+  try {
+    await db.exec('reindex index public.items_geo_box_idx');
+    const result = await db.query(
+      'select public.ndb_geo_distance(data,-74,40) as distance from public.items where id=4',
+    );
+    expect(result.rows[0].distance).toBe(0);
+  } finally {
+    await db.exec('reset search_path');
+  }
+});