diff --git a/.github/workflows/run-tests.yaml b/.github/workflows/run-tests.yaml index 099cbaa1e..eab69b0e9 100644 --- a/.github/workflows/run-tests.yaml +++ b/.github/workflows/run-tests.yaml @@ -73,6 +73,8 @@ jobs: env: {} - package: graphile/graphile-connection-filter env: {} + - package: graphile/graphile-postgis + env: {} - package: graphql/server-test env: {} - package: graphql/env diff --git a/graphile/graphile-postgis/__tests__/connection-filter-operators.test.ts b/graphile/graphile-postgis/__tests__/connection-filter-operators.test.ts index e68588f68..7e8e9c068 100644 --- a/graphile/graphile-postgis/__tests__/connection-filter-operators.test.ts +++ b/graphile/graphile-postgis/__tests__/connection-filter-operators.test.ts @@ -236,34 +236,49 @@ describe('PostGIS operator factory (createPostgisOperatorFactory)', () => { describe('SQL generation', () => { describe('function-based operators', () => { - it('generates schema-qualified SQL for public schema', () => { + // The input-binding contract (regression guard for #724): every + // operator must wrap the GeoJSON input with ST_GeomFromGeoJSON(...) + // — PostgreSQL's geometry_in / geography_in parsers reject raw + // GeoJSON text. The compiled SQL must therefore always contain the + // schema-qualified function call binding the JSON-encoded input as + // `::text` before any further casting. + + it('wraps input with ST_GeomFromGeoJSON (public schema)', () => { const { registered } = runFactory({ schemaName: 'public' }); const containsOp = registered.find(r => r.operatorName === 'contains'); expect(containsOp).toBeDefined(); const i = sql.identifier('col'); const v = sql.identifier('val'); - const result = containsOp!.resolve(i, v, null, null, { + const input = { type: 'Point', coordinates: [-122.4194, 37.7749] }; + const result = containsOp!.resolve(i, v, input, null, { fieldName: null, operatorName: 'contains' }); const compiled = sql.compile(result); - expect(compiled.text).toBe('"public"."st_contains"("col", "val")'); + expect(compiled.text).toBe( + '"public"."st_contains"("col", "public"."st_geomfromgeojson"($1::text))' + ); + expect(compiled.values).toEqual([JSON.stringify(input)]); }); - it('generates schema-qualified SQL for non-public schema', () => { + it('wraps input with ST_GeomFromGeoJSON (non-public schema)', () => { const { registered } = runFactory({ schemaName: 'postgis' }); const containsOp = registered.find(r => r.operatorName === 'contains'); expect(containsOp).toBeDefined(); const i = sql.identifier('col'); const v = sql.identifier('val'); - const result = containsOp!.resolve(i, v, null, null, { + const input = { type: 'Point', coordinates: [-122.4194, 37.7749] }; + const result = containsOp!.resolve(i, v, input, null, { fieldName: null, operatorName: 'contains' }); const compiled = sql.compile(result); - expect(compiled.text).toBe('"postgis"."st_contains"("col", "val")'); + expect(compiled.text).toBe( + '"postgis"."st_contains"("col", "postgis"."st_geomfromgeojson"($1::text))' + ); + expect(compiled.values).toEqual([JSON.stringify(input)]); }); it('lowercases function names in SQL', () => { @@ -273,89 +288,86 @@ describe('PostGIS operator factory (createPostgisOperatorFactory)', () => { const i = sql.identifier('a'); const v = sql.identifier('b'); - const result = op3d!.resolve(i, v, null, null, { + const input = { type: 'Point', coordinates: [-122.4194, 37.7749, 254] }; + const result = op3d!.resolve(i, v, input, null, { fieldName: null, operatorName: 'intersects3D' }); const compiled = sql.compile(result); - expect(compiled.text).toBe('"public"."st_3dintersects"("a", "b")'); + expect(compiled.text).toBe( + '"public"."st_3dintersects"("a", "public"."st_geomfromgeojson"($1::text))' + ); }); - }); - describe('SQL operator-based operators', () => { - it('generates correct SQL for = operator', () => { + it('casts to geography when the operator is registered on a geography type', () => { const { registered } = runFactory(); - const exactOp = registered.find(r => r.operatorName === 'exactlyEquals'); - expect(exactOp).toBeDefined(); + // `intersects` is registered for both geometry and geography. We + // want the geography variant to append `::geography` after the + // ST_GeomFromGeoJSON wrap so PostGIS picks the geography overload. + const geogIntersects = registered.find( + r => r.operatorName === 'intersects' && r.typeName === 'GeographyPoint' + ); + expect(geogIntersects).toBeDefined(); const i = sql.identifier('col'); const v = sql.identifier('val'); - const result = exactOp!.resolve(i, v, null, null, { + const input = { type: 'Point', coordinates: [-122.4194, 37.7749] }; + const result = geogIntersects!.resolve(i, v, input, null, { fieldName: null, - operatorName: 'exactlyEquals' + operatorName: 'intersects' }); const compiled = sql.compile(result); - expect(compiled.text).toBe('"col" = "val"'); + expect(compiled.text).toBe( + '"public"."st_intersects"("col", "public"."st_geomfromgeojson"($1::text)::"public"."geography")' + ); }); + }); - it('generates correct SQL for && operator', () => { + describe('SQL operator-based operators', () => { + const runOp = (operatorName: string) => { const { registered } = runFactory(); - const bboxOp = registered.find(r => r.operatorName === 'bboxIntersects2D'); - expect(bboxOp).toBeDefined(); + const op = registered.find(r => r.operatorName === operatorName); + expect(op).toBeDefined(); - const i = sql.identifier('col'); - const v = sql.identifier('val'); - const result = bboxOp!.resolve(i, v, null, null, { - fieldName: null, - operatorName: 'bboxIntersects2D' - }); - const compiled = sql.compile(result); - expect(compiled.text).toBe('"col" && "val"'); + const input = { type: 'Point', coordinates: [-122.4194, 37.7749] }; + const result = op!.resolve( + sql.identifier('col'), + sql.identifier('val'), + input, + null, + { fieldName: null, operatorName } + ); + return sql.compile(result); + }; + + it('generates correct SQL for = operator', () => { + expect(runOp('exactlyEquals').text).toBe( + '"col" = "public"."st_geomfromgeojson"($1::text)' + ); }); - it('generates correct SQL for ~ operator', () => { - const { registered } = runFactory(); - const bboxContainsOp = registered.find(r => r.operatorName === 'bboxContains'); - expect(bboxContainsOp).toBeDefined(); + it('generates correct SQL for && operator', () => { + expect(runOp('bboxIntersects2D').text).toBe( + '"col" && "public"."st_geomfromgeojson"($1::text)' + ); + }); - const i = sql.identifier('col'); - const v = sql.identifier('val'); - const result = bboxContainsOp!.resolve(i, v, null, null, { - fieldName: null, - operatorName: 'bboxContains' - }); - const compiled = sql.compile(result); - expect(compiled.text).toBe('"col" ~ "val"'); + it('generates correct SQL for ~ operator', () => { + expect(runOp('bboxContains').text).toBe( + '"col" ~ "public"."st_geomfromgeojson"($1::text)' + ); }); it('generates correct SQL for ~= operator', () => { - const { registered } = runFactory(); - const bboxEqOp = registered.find(r => r.operatorName === 'bboxEquals'); - expect(bboxEqOp).toBeDefined(); - - const i = sql.identifier('col'); - const v = sql.identifier('val'); - const result = bboxEqOp!.resolve(i, v, null, null, { - fieldName: null, - operatorName: 'bboxEquals' - }); - const compiled = sql.compile(result); - expect(compiled.text).toBe('"col" ~= "val"'); + expect(runOp('bboxEquals').text).toBe( + '"col" ~= "public"."st_geomfromgeojson"($1::text)' + ); }); it('generates correct SQL for &&& operator', () => { - const { registered } = runFactory(); - const ndOp = registered.find(r => r.operatorName === 'bboxIntersectsND'); - expect(ndOp).toBeDefined(); - - const i = sql.identifier('col'); - const v = sql.identifier('val'); - const result = ndOp!.resolve(i, v, null, null, { - fieldName: null, - operatorName: 'bboxIntersectsND' - }); - const compiled = sql.compile(result); - expect(compiled.text).toBe('"col" &&& "val"'); + expect(runOp('bboxIntersectsND').text).toBe( + '"col" &&& "public"."st_geomfromgeojson"($1::text)' + ); }); }); }); diff --git a/graphile/graphile-postgis/src/plugins/connection-filter-operators.ts b/graphile/graphile-postgis/src/plugins/connection-filter-operators.ts index 63c6be07e..777649a22 100644 --- a/graphile/graphile-postgis/src/plugins/connection-filter-operators.ts +++ b/graphile/graphile-postgis/src/plugins/connection-filter-operators.ts @@ -221,6 +221,9 @@ export function createPostgisOperatorFactory(): ConnectionFilterOperatorFactory const { inflection } = build; const { schemaName, geometryCodec, geographyCodec } = postgisInfo; + const sqlGeomFromGeoJSON = sql.identifier(schemaName, 'st_geomfromgeojson'); + const sqlGeographyType = sql.identifier(schemaName, 'geography'); + // Collect all GQL type names for geometry and geography const gqlTypeNamesByBase: Record = { geometry: [], @@ -254,6 +257,7 @@ export function createPostgisOperatorFactory(): ConnectionFilterOperatorFactory typeNames: string[]; operatorName: string; description: string; + baseType: 'geometry' | 'geography'; resolve: (i: SQL, v: SQL) => SQL; } const allSpecs: InternalSpec[] = []; @@ -267,6 +271,7 @@ export function createPostgisOperatorFactory(): ConnectionFilterOperatorFactory typeNames: gqlTypeNamesByBase[baseType], operatorName, description, + baseType: baseType as 'geometry' | 'geography', resolve: (i: SQL, v: SQL) => sql.fragment`${sqlGisFunction}(${i}, ${v})` }); } @@ -281,6 +286,7 @@ export function createPostgisOperatorFactory(): ConnectionFilterOperatorFactory typeNames: gqlTypeNamesByBase[baseType], operatorName, description, + baseType: baseType as 'geometry' | 'geography', resolve: (i: SQL, v: SQL) => buildOperatorExpr(capturedOp, i, v) }); } @@ -292,8 +298,22 @@ export function createPostgisOperatorFactory(): ConnectionFilterOperatorFactory // Convert to ConnectionFilterOperatorRegistration format. // Each InternalSpec may target multiple type names; we expand each // into individual registrations keyed by typeName. + // + // The default operatorApply pipeline binds the filter value as a raw + // text parameter cast to the column codec's sqlType (geometry / + // geography). PostgreSQL's geometry_in / geography_in parsers reject + // GeoJSON text, so we must wrap the input with ST_GeomFromGeoJSON + // ourselves — see within-distance-operator.ts for the pattern. + // + // We disable the default bind via `resolveSqlValue: () => sql.null` + // and construct the geometry value from `input` inside resolve(), + // mirroring the ST_DWithin implementation. const registrations: ConnectionFilterOperatorRegistration[] = []; for (const spec of allSpecs) { + const geographyCast = spec.baseType === 'geography' + ? sql.fragment`::${sqlGeographyType}` + : sql.fragment``; + for (const typeName of spec.typeNames) { registrations.push({ typeNames: typeName, @@ -301,14 +321,17 @@ export function createPostgisOperatorFactory(): ConnectionFilterOperatorFactory spec: { description: spec.description, resolveType: (fieldType) => fieldType, + resolveSqlValue: () => sql.null, resolve( sqlIdentifier: SQL, - sqlValue: SQL, - _input: unknown, + _sqlValue: SQL, + input: unknown, _$where: any, _details: { fieldName: string | null; operatorName: string } ) { - return spec.resolve(sqlIdentifier, sqlValue); + const geoJsonStr = sql.value(JSON.stringify(input)); + const geomSql = sql.fragment`${sqlGeomFromGeoJSON}(${geoJsonStr}::text)${geographyCast}`; + return spec.resolve(sqlIdentifier, geomSql); } } satisfies ConnectionFilterOperatorSpec, });