From 815f14721e84af51d1efd0cfeda8bae35ae96172 Mon Sep 17 00:00:00 2001 From: zetazzz Date: Fri, 14 Aug 2026 04:13:29 +0800 Subject: [PATCH 1/4] Add a reusable Graphile performance harness --- packages/perf-harness/README.md | 39 +++ .../perf-harness/__tests__/fixture.test.ts | 22 ++ .../__tests__/fixtures/fake-worker.js | 30 ++ .../perf-harness/__tests__/process.test.ts | 22 ++ .../perf-harness/__tests__/report.test.ts | 78 +++++ packages/perf-harness/__tests__/run.test.ts | 37 +++ .../perf-harness/__tests__/schedule.test.ts | 29 ++ packages/perf-harness/jest.config.js | 12 + packages/perf-harness/package.json | 36 +++ packages/perf-harness/src/fixture.ts | 127 ++++++++ packages/perf-harness/src/index.ts | 20 ++ packages/perf-harness/src/metrics.ts | 79 +++++ packages/perf-harness/src/process.ts | 130 +++++++++ packages/perf-harness/src/report.ts | 123 ++++++++ packages/perf-harness/src/run.ts | 274 ++++++++++++++++++ packages/perf-harness/src/schedule.ts | 77 +++++ packages/perf-harness/src/stock-worker.ts | 94 ++++++ packages/perf-harness/src/types.ts | 129 +++++++++ packages/perf-harness/tsconfig.esm.json | 8 + packages/perf-harness/tsconfig.json | 11 + pnpm-lock.yaml | 31 ++ 21 files changed, 1408 insertions(+) create mode 100644 packages/perf-harness/README.md create mode 100644 packages/perf-harness/__tests__/fixture.test.ts create mode 100644 packages/perf-harness/__tests__/fixtures/fake-worker.js create mode 100644 packages/perf-harness/__tests__/process.test.ts create mode 100644 packages/perf-harness/__tests__/report.test.ts create mode 100644 packages/perf-harness/__tests__/run.test.ts create mode 100644 packages/perf-harness/__tests__/schedule.test.ts create mode 100644 packages/perf-harness/jest.config.js create mode 100644 packages/perf-harness/package.json create mode 100644 packages/perf-harness/src/fixture.ts create mode 100644 packages/perf-harness/src/index.ts create mode 100644 packages/perf-harness/src/metrics.ts create mode 100644 packages/perf-harness/src/process.ts create mode 100644 packages/perf-harness/src/report.ts create mode 100644 packages/perf-harness/src/run.ts create mode 100644 packages/perf-harness/src/schedule.ts create mode 100644 packages/perf-harness/src/stock-worker.ts create mode 100644 packages/perf-harness/src/types.ts create mode 100644 packages/perf-harness/tsconfig.esm.json create mode 100644 packages/perf-harness/tsconfig.json diff --git a/packages/perf-harness/README.md b/packages/perf-harness/README.md new file mode 100644 index 0000000000..484670a951 --- /dev/null +++ b/packages/perf-harness/README.md @@ -0,0 +1,39 @@ +# Graphile performance harness + +Reusable infrastructure for measuring Graphile schema builds in fresh Node +processes. The core accepts any list of serializable benchmark cases; it does not +interpret case names or optimization-specific configuration. + +Each measurement receives a new PID, starts Node with `--expose-gc`, runs a +deterministic GC sequence, records build time and memory metrics, validates a +runtime query, and reports a schema hash. Cases can opt into schema equivalence +groups and provide their own lifecycle validation through the worker result. + +## Extending the harness + +Define a suite and provide a dedicated worker entry point: + +```ts +const suite = { + name: 'example', + cases: [ + { + name: 'baseline', + workerConfig: { schemas: ['cperf_example'] }, + expectedSchemaGroup: 'example-schema', + }, + ], +}; + +await runBenchmarkSuite(suite, options, workerPath); +``` + +`workerConfig` must be JSON-serializable. Logic is implemented in the worker +entry rather than serializing functions across process boundaries. + +The package includes `stock-worker.js` as a minimal upstream Graphile baseline. +Database credentials are supplied through `CPERF_DATABASE_URL` and are redacted +from worker failures and JSON reports. + +The PostgreSQL fixture command only creates a previously absent schema whose +name starts with `cperf_`; it never drops or replaces schemas. diff --git a/packages/perf-harness/__tests__/fixture.test.ts b/packages/perf-harness/__tests__/fixture.test.ts new file mode 100644 index 0000000000..08e9109e1e --- /dev/null +++ b/packages/perf-harness/__tests__/fixture.test.ts @@ -0,0 +1,22 @@ +import { + validateFixtureSchema, + validateFixtureTableCount, +} from '../src/fixture'; + +describe('fixture safety', () => { + test('only accepts narrowly scoped benchmark schema names', () => { + expect(validateFixtureSchema('cperf_example_1')).toBe('cperf_example_1'); + expect(() => validateFixtureSchema('public')).toThrow( + 'must start with cperf_' + ); + expect(() => + validateFixtureSchema('cperf_example; drop schema public') + ).toThrow('must start with cperf_'); + }); + + test('bounds generated fixture size', () => { + expect(validateFixtureTableCount(64)).toBe(64); + expect(() => validateFixtureTableCount(0)).toThrow('between 1 and 500'); + expect(() => validateFixtureTableCount(501)).toThrow('between 1 and 500'); + }); +}); diff --git a/packages/perf-harness/__tests__/fixtures/fake-worker.js b/packages/perf-harness/__tests__/fixtures/fake-worker.js new file mode 100644 index 0000000000..ea69962ace --- /dev/null +++ b/packages/perf-harness/__tests__/fixtures/fake-worker.js @@ -0,0 +1,30 @@ +'use strict'; + +const envelope = JSON.parse( + Buffer.from(process.env.CPERF_WORKER_CONFIG, 'base64url').toString('utf8') +); +const value = envelope.workerConfig.value; +const memory = { + rss: value, + heapTotal: value, + heapUsed: value, + external: value, + arrayBuffers: value, +}; +const result = { + status: 'ok', + pid: process.pid, + caseName: envelope.caseName, + buildMs: value, + schemaHash: envelope.workerConfig.schemaHash, + schemaTypeCount: 10, + runtimeVerified: true, + caseValidation: { passed: true, errors: [] }, + memory: { + baseline: memory, + afterBuild: memory, + delta: memory, + processPeakRss: value, + }, +}; +process.stdout.write(`CPERF_RESULT ${JSON.stringify(result)}\n`); diff --git a/packages/perf-harness/__tests__/process.test.ts b/packages/perf-harness/__tests__/process.test.ts new file mode 100644 index 0000000000..65fd7ef64c --- /dev/null +++ b/packages/perf-harness/__tests__/process.test.ts @@ -0,0 +1,22 @@ +import { resolve } from 'node:path'; + +import { runWorkerProcess } from '../src/process'; + +describe('fresh worker process', () => { + test('uses distinct PIDs and does not expose the database URL', async () => { + const worker = resolve(__dirname, 'fixtures/fake-worker.js'); + const definition = { + name: 'baseline', + workerConfig: { value: 1, schemaHash: 'same' }, + }; + const databaseUrl = 'postgres://secret@example.test/database'; + const first = await runWorkerProcess(worker, databaseUrl, definition); + const second = await runWorkerProcess(worker, databaseUrl, definition); + expect(first.pid).not.toBe(process.pid); + expect(second.pid).not.toBe(process.pid); + expect(first.pid).not.toBe(second.pid); + expect(JSON.stringify([first.result, second.result])).not.toContain( + databaseUrl + ); + }); +}); diff --git a/packages/perf-harness/__tests__/report.test.ts b/packages/perf-harness/__tests__/report.test.ts new file mode 100644 index 0000000000..6d7e043d7a --- /dev/null +++ b/packages/perf-harness/__tests__/report.test.ts @@ -0,0 +1,78 @@ +import { + compareCases, + summarizeCase, + validateSchemaGroups, +} from '../src/report'; +import type { BenchmarkRun, SuccessfulWorkerResult } from '../src/types'; + +const result = (caseName: string, value: number): SuccessfulWorkerResult => ({ + status: 'ok', + pid: value, + caseName, + buildMs: value, + schemaHash: 'same', + schemaTypeCount: 10, + runtimeVerified: true, + caseValidation: { passed: true, errors: [] }, + memory: { + baseline: { + rss: 10, + heapTotal: 10, + heapUsed: 10, + external: 10, + arrayBuffers: 10, + }, + afterBuild: { + rss: value, + heapTotal: value, + heapUsed: value, + external: value, + arrayBuffers: value, + }, + delta: { + rss: value - 10, + heapTotal: value - 10, + heapUsed: value - 10, + external: value - 10, + arrayBuffers: value - 10, + }, + processPeakRss: value, + }, +}); + +describe('generic reports', () => { + test('summarizes arbitrary cases and compares medians', () => { + const runs: BenchmarkRun[] = [10, 30, 20].map((value, index) => ({ + repetition: index + 1, + position: 1, + caseName: 'base', + result: result('base', value), + })); + runs.push( + ...[5, 15, 10].map((value, index) => ({ + repetition: index + 1, + position: 2, + caseName: 'candidate', + result: result('candidate', value), + })) + ); + const base = summarizeCase(runs, 'base')!; + const candidate = summarizeCase(runs, 'candidate')!; + expect( + compareCases('base', 'candidate', base, candidate).buildMs.percentChange + ).toBe(-50); + expect( + validateSchemaGroups( + [ + { name: 'base', workerConfig: null, expectedSchemaGroup: 'schema' }, + { + name: 'candidate', + workerConfig: null, + expectedSchemaGroup: 'schema', + }, + ], + runs + ) + ).toEqual({ equivalent: true, hashes: { schema: 'same' }, errors: [] }); + }); +}); diff --git a/packages/perf-harness/__tests__/run.test.ts b/packages/perf-harness/__tests__/run.test.ts new file mode 100644 index 0000000000..0628e1ba43 --- /dev/null +++ b/packages/perf-harness/__tests__/run.test.ts @@ -0,0 +1,37 @@ +import { resolve } from 'node:path'; + +import { runBenchmarkSuite } from '../src/run'; + +describe('generic suite runner', () => { + test('validates fresh processes and schema groups without fixed case names', async () => { + const report = await runBenchmarkSuite( + { + name: 'test-suite', + cases: ['alpha', 'beta', 'gamma'].map((name, index) => ({ + name, + workerConfig: { value: index + 1, schemaHash: 'same' }, + expectedSchemaGroup: 'schema', + })), + }, + { + databaseUrl: 'postgres:///not-used-by-fake-worker', + repetitions: 1, + seed: 1, + order: ['alpha', 'beta', 'gamma'], + }, + resolve(__dirname, 'fixtures/fake-worker.js') + ); + expect(report.validation).toEqual( + expect.objectContaining({ + allRunsSucceeded: true, + freshProcessPerRun: true, + caseValidationPassed: true, + schemaGroupsEquivalent: true, + schemaGroups: { schema: 'same' }, + errors: [], + }) + ); + expect(new Set(report.runs.map((run) => run.result.pid)).size).toBe(3); + expect(JSON.stringify(report)).not.toContain('postgres:///'); + }); +}); diff --git a/packages/perf-harness/__tests__/schedule.test.ts b/packages/perf-harness/__tests__/schedule.test.ts new file mode 100644 index 0000000000..89d5e672f8 --- /dev/null +++ b/packages/perf-harness/__tests__/schedule.test.ts @@ -0,0 +1,29 @@ +import { makeSchedule } from '../src/schedule'; + +const cases = [ + { name: 'a', workerConfig: null }, + { name: 'b', workerConfig: null }, + { name: 'c', workerConfig: null }, +]; + +describe('generic benchmark scheduling', () => { + test('is deterministic and supports any case list', () => { + const first = makeSchedule(cases, 4, 1234); + expect(makeSchedule(cases, 4, 1234)).toEqual(first); + expect(makeSchedule(cases, 4, 4321)).not.toEqual(first); + for (let repetition = 1; repetition <= 4; repetition += 1) { + expect( + first + .filter((item) => item.repetition === repetition) + .map((item) => item.caseName) + .sort() + ).toEqual(['a', 'b', 'c']); + } + }); + + test('accepts an exact order containing each case once', () => { + expect( + makeSchedule(cases, 2, 1, ['c', 'a', 'b']).map((item) => item.caseName) + ).toEqual(['c', 'a', 'b', 'c', 'a', 'b']); + }); +}); diff --git a/packages/perf-harness/jest.config.js b/packages/perf-harness/jest.config.js new file mode 100644 index 0000000000..f34711d4ed --- /dev/null +++ b/packages/perf-harness/jest.config.js @@ -0,0 +1,12 @@ +/** @type {import('ts-jest').JestConfigWithTsJest} */ +module.exports = { + preset: 'ts-jest', + testEnvironment: 'node', + transform: { + '^.+\\.tsx?$': ['ts-jest', { tsconfig: 'tsconfig.json' }], + }, + testRegex: '(/__tests__/.*|(\\.|/)(test|spec))\\.(jsx?|tsx?)$', + moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx', 'json', 'node'], + modulePathIgnorePatterns: ['dist/*'], + testPathIgnorePatterns: ['/__tests__/fixtures/'], +}; diff --git a/packages/perf-harness/package.json b/packages/perf-harness/package.json new file mode 100644 index 0000000000..8d23b8bc10 --- /dev/null +++ b/packages/perf-harness/package.json @@ -0,0 +1,36 @@ +{ + "name": "@constructive-io/perf-harness", + "version": "0.1.0", + "private": true, + "description": "Reusable fresh-process Graphile performance harness", + "main": "index.js", + "module": "esm/index.js", + "types": "index.d.ts", + "bin": { + "cperf": "index.js" + }, + "scripts": { + "clean": "makage clean", + "build": "makage build", + "build:dev": "makage build --dev", + "lint": "eslint . --fix", + "test": "jest" + }, + "dependencies": { + "graphile-build": "5.1.1", + "graphile-build-pg": "5.1.3", + "graphile-config": "1.1.0", + "graphql": "16.13.0", + "pg": "^8.21.0", + "postgraphile": "5.1.4" + }, + "devDependencies": { + "@types/node": "^22.19.11", + "@types/pg": "^8.20.4", + "makage": "^0.3.0" + }, + "engines": { + "node": ">=22" + }, + "license": "MIT" +} diff --git a/packages/perf-harness/src/fixture.ts b/packages/perf-harness/src/fixture.ts new file mode 100644 index 0000000000..b7f35935b6 --- /dev/null +++ b/packages/perf-harness/src/fixture.ts @@ -0,0 +1,127 @@ +import { Pool } from 'pg'; + +export const FIXTURE_VERSION = 1; + +export interface PrepareFixtureOptions { + databaseUrl: string; + schema: string; + tables: number; +} + +export interface PreparedFixture { + fixtureVersion: number; + database: string; + serverVersion: string; + schema: string; + tableCount: number; + functionCount: number; +} + +export const validateFixtureSchema = (schema: string): string => { + if ( + !/^cperf_[a-z0-9_]*$/.test(schema) || + schema.length > 63 || + schema.includes('\0') + ) { + throw new Error( + 'fixture schema must start with cperf_, use only lowercase letters, digits, and underscores, and fit PostgreSQL identifiers' + ); + } + return schema; +}; + +export const validateFixtureTableCount = (tables: number): number => { + if (!Number.isSafeInteger(tables) || tables < 1 || tables > 500) { + throw new Error('fixture table count must be an integer between 1 and 500'); + } + return tables; +}; + +const quoteIdentifier = (identifier: string): string => + `"${identifier.replaceAll('"', '""')}"`; + +export const prepareFixture = async ( + options: PrepareFixtureOptions +): Promise => { + const schema = validateFixtureSchema(options.schema); + const tables = validateFixtureTableCount(options.tables); + const quotedSchema = quoteIdentifier(schema); + const pool = new Pool({ connectionString: options.databaseUrl, max: 1 }); + const client = await pool.connect(); + try { + await client.query('begin'); + const existing = await client.query<{ exists: boolean }>( + 'select exists(select 1 from pg_catalog.pg_namespace where nspname = $1) as exists', + [schema] + ); + if (existing.rows[0]?.exists) { + throw new Error( + `fixture schema '${schema}' already exists; this command never replaces schemas` + ); + } + await client.query(`create schema ${quotedSchema}`); + await client.query( + `comment on schema ${quotedSchema} is 'cperf fixture version ${FIXTURE_VERSION}'` + ); + await client.query( + `create type ${quotedSchema}."entity_status" as enum ('draft', 'active', 'archived')` + ); + await client.query(` + create table ${quotedSchema}."account" ( + id bigint generated always as identity primary key, + external_id uuid not null unique, + name text not null, + metadata jsonb not null default '{}'::jsonb, + created_at timestamptz not null default now() + ) + `); + for (let index = 1; index <= tables; index += 1) { + const table = quoteIdentifier(`entity_${index}`); + const functionName = quoteIdentifier(`entity_${index}_by_account`); + const indexName = quoteIdentifier(`entity_${index}_account_created_idx`); + await client.query(` + create table ${quotedSchema}.${table} ( + id bigint generated always as identity primary key, + account_id bigint not null references ${quotedSchema}."account"(id), + status ${quotedSchema}."entity_status" not null default 'draft', + title text not null, + tags text[] not null default array[]::text[], + metadata jsonb not null default '{}'::jsonb, + created_at timestamptz not null default now(), + unique (account_id, title) + ); + create index ${indexName} + on ${quotedSchema}.${table} (account_id, created_at desc); + create function ${quotedSchema}.${functionName}(requested_account_id bigint) + returns setof ${quotedSchema}.${table} + language sql stable + as 'select * from ${quotedSchema}.${table} where account_id = requested_account_id'; + `); + } + const identity = await client.query<{ + database: string; + server_version: string; + }>( + "select current_database() as database, current_setting('server_version') as server_version" + ); + await client.query('commit'); + return { + fixtureVersion: FIXTURE_VERSION, + database: identity.rows[0].database, + serverVersion: identity.rows[0].server_version, + schema, + tableCount: tables + 1, + functionCount: tables, + }; + } catch (error) { + try { + await client.query('rollback'); + } catch { + // Preserve the fixture preparation error; the client is discarded below. + } + throw error; + } finally { + client.release(); + await pool.end(); + } +}; diff --git a/packages/perf-harness/src/index.ts b/packages/perf-harness/src/index.ts new file mode 100644 index 0000000000..7829d7dc66 --- /dev/null +++ b/packages/perf-harness/src/index.ts @@ -0,0 +1,20 @@ +#!/usr/bin/env node + +export * from './fixture'; +export * from './metrics'; +export * from './process'; +export * from './report'; +export * from './run'; +export * from './schedule'; +export * from './types'; + +import { cliMain } from './run'; + +if (require.main === module) { + void cliMain().catch((error: unknown) => { + process.stderr.write( + `${error instanceof Error ? error.message : String(error)}\n` + ); + process.exitCode = 1; + }); +} diff --git a/packages/perf-harness/src/metrics.ts b/packages/perf-harness/src/metrics.ts new file mode 100644 index 0000000000..007e79fa13 --- /dev/null +++ b/packages/perf-harness/src/metrics.ts @@ -0,0 +1,79 @@ +import { performance } from 'node:perf_hooks'; + +import type { + CaseValidation, + JsonValue, + MemorySnapshot, + SuccessfulWorkerResult, +} from './types'; + +export interface MeasuredCaseResult { + schemaHash: string; + schemaTypeCount: number; + runtimeVerified: true; + caseValidation?: CaseValidation; + metadata?: Record; +} + +const collectGarbage = (): void => { + if (typeof global.gc !== 'function') { + throw new Error('benchmark worker requires Node --expose-gc'); + } + global.gc(); + global.gc(); + global.gc(); +}; + +const memorySnapshot = (): MemorySnapshot => { + const memory = process.memoryUsage(); + return { + rss: memory.rss, + heapTotal: memory.heapTotal, + heapUsed: memory.heapUsed, + external: memory.external, + arrayBuffers: memory.arrayBuffers, + }; +}; + +const memoryDelta = ( + baseline: MemorySnapshot, + afterBuild: MemorySnapshot +): MemorySnapshot => ({ + rss: afterBuild.rss - baseline.rss, + heapTotal: afterBuild.heapTotal - baseline.heapTotal, + heapUsed: afterBuild.heapUsed - baseline.heapUsed, + external: afterBuild.external - baseline.external, + arrayBuffers: afterBuild.arrayBuffers - baseline.arrayBuffers, +}); + +export const measureBenchmarkCase = async ( + caseName: string, + build: () => Promise, + validate: (built: Built) => Promise +): Promise => { + collectGarbage(); + const baseline = memorySnapshot(); + const startedAt = performance.now(); + const built = await build(); + const buildMs = performance.now() - startedAt; + const measured = await validate(built); + collectGarbage(); + const afterBuild = memorySnapshot(); + return { + status: 'ok', + pid: process.pid, + caseName, + buildMs, + schemaHash: measured.schemaHash, + schemaTypeCount: measured.schemaTypeCount, + runtimeVerified: measured.runtimeVerified, + caseValidation: measured.caseValidation ?? { passed: true, errors: [] }, + ...(measured.metadata ? { metadata: measured.metadata } : {}), + memory: { + baseline, + afterBuild, + delta: memoryDelta(baseline, afterBuild), + processPeakRss: process.resourceUsage().maxRSS * 1024, + }, + }; +}; diff --git a/packages/perf-harness/src/process.ts b/packages/perf-harness/src/process.ts new file mode 100644 index 0000000000..14c5464a60 --- /dev/null +++ b/packages/perf-harness/src/process.ts @@ -0,0 +1,130 @@ +import { spawn } from 'node:child_process'; + +import type { + BenchmarkCaseDefinition, + WorkerConfigEnvelope, + WorkerResult, +} from './types'; + +export const WORKER_RESULT_PREFIX = 'CPERF_RESULT '; +export const DATABASE_URL_ENV = 'CPERF_DATABASE_URL'; +export const WORKER_CONFIG_ENV = 'CPERF_WORKER_CONFIG'; + +export interface SpawnedWorkerResult { + pid: number; + result: WorkerResult; +} + +const lastLines = (value: string, count = 20): string => + value.trim().split('\n').slice(-count).join('\n'); + +export const redactSecret = (value: string, secret: string): string => + secret ? value.replaceAll(secret, '') : value; + +export const runWorkerProcess = ( + workerPath: string, + databaseUrl: string, + definition: BenchmarkCaseDefinition +): Promise => + new Promise((resolve, reject) => { + const config: WorkerConfigEnvelope = { + caseName: definition.name, + workerConfig: definition.workerConfig, + }; + const child = spawn(process.execPath, ['--expose-gc', workerPath], { + env: { + ...process.env, + NODE_ENV: 'production', + GRAPHILE_ENV: 'production', + [DATABASE_URL_ENV]: databaseUrl, + [WORKER_CONFIG_ENV]: Buffer.from(JSON.stringify(config)).toString( + 'base64url' + ), + }, + stdio: ['ignore', 'pipe', 'pipe'], + }); + const pid = child.pid; + let stdout = ''; + let stderr = ''; + child.stdout.setEncoding('utf8'); + child.stderr.setEncoding('utf8'); + child.stdout.on('data', (chunk: string) => { + stdout += chunk; + }); + child.stderr.on('data', (chunk: string) => { + stderr += chunk; + }); + child.once('error', reject); + child.once('close', (code, signal) => { + const resultLine = stdout + .split('\n') + .reverse() + .find((line) => line.startsWith(WORKER_RESULT_PREFIX)); + if (!resultLine) { + reject( + new Error( + redactSecret( + `benchmark worker ${pid ?? 'unknown'} exited without a result ` + + `(code=${String(code)}, signal=${String(signal)})` + + (stderr.trim() ? `\n${lastLines(stderr)}` : ''), + databaseUrl + ) + ) + ); + return; + } + try { + const result = JSON.parse( + resultLine.slice(WORKER_RESULT_PREFIX.length) + ) as WorkerResult; + if (typeof pid !== 'number' || result.pid !== pid) { + throw new Error( + `worker PID mismatch: spawned ${String(pid)}, reported ${String( + result.pid + )}` + ); + } + if (result.caseName !== definition.name) { + throw new Error( + `worker case mismatch: expected ${definition.name}, reported ${result.caseName}` + ); + } + if (result.status === 'ok' && code !== 0) { + throw new Error(`successful worker exited with code ${String(code)}`); + } + resolve({ pid, result }); + } catch (error) { + reject( + new Error( + redactSecret( + `invalid result from benchmark worker ${String(pid)}: ${String( + error instanceof Error ? error.message : error + )}`, + databaseUrl + ) + ) + ); + } + }); + }); + +export const parseWorkerEnvelope = ( + encoded: string | undefined +): WorkerConfigEnvelope => { + if (!encoded) throw new Error(`${WORKER_CONFIG_ENV} is required`); + const parsed = JSON.parse( + Buffer.from(encoded, 'base64url').toString('utf8') + ) as Partial; + if ( + typeof parsed.caseName !== 'string' || + parsed.caseName.length === 0 || + parsed.workerConfig === undefined + ) { + throw new Error('worker configuration envelope is invalid'); + } + return parsed as WorkerConfigEnvelope; +}; + +export const writeWorkerResult = (result: WorkerResult): void => { + process.stdout.write(`${WORKER_RESULT_PREFIX}${JSON.stringify(result)}\n`); +}; diff --git a/packages/perf-harness/src/report.ts b/packages/perf-harness/src/report.ts new file mode 100644 index 0000000000..680852a961 --- /dev/null +++ b/packages/perf-harness/src/report.ts @@ -0,0 +1,123 @@ +import type { + BenchmarkCaseDefinition, + BenchmarkRun, + CaseComparison, + CaseSummary, + MetricComparison, + MetricSummary, + SuccessfulWorkerResult, +} from './types'; + +const median = (values: readonly number[]): number => { + if (values.length === 0) throw new Error('cannot summarize zero values'); + const sorted = [...values].sort((left, right) => left - right); + const middle = Math.floor(sorted.length / 2); + return sorted.length % 2 === 0 + ? (sorted[middle - 1] + sorted[middle]) / 2 + : sorted[middle]; +}; + +const metricSummary = (values: number[]): MetricSummary => ({ + median: median(values), + min: Math.min(...values), + max: Math.max(...values), + samples: values, +}); + +const successfulResultsFor = ( + runs: readonly BenchmarkRun[], + caseName: string +): SuccessfulWorkerResult[] => + runs + .filter((run) => run.caseName === caseName && run.result.status === 'ok') + .map((run) => run.result as SuccessfulWorkerResult); + +export const summarizeCase = ( + runs: readonly BenchmarkRun[], + caseName: string +): CaseSummary | undefined => { + const results = successfulResultsFor(runs, caseName); + if (results.length === 0) return undefined; + return { + sampleCount: results.length, + buildMs: metricSummary(results.map((result) => result.buildMs)), + heapUsedAfterBuild: metricSummary( + results.map((result) => result.memory.afterBuild.heapUsed) + ), + heapUsedDelta: metricSummary( + results.map((result) => result.memory.delta.heapUsed) + ), + rssAfterBuild: metricSummary( + results.map((result) => result.memory.afterBuild.rss) + ), + rssDelta: metricSummary(results.map((result) => result.memory.delta.rss)), + processPeakRss: metricSummary( + results.map((result) => result.memory.processPeakRss) + ), + }; +}; + +const compareMetric = ( + baseline: MetricSummary, + candidate: MetricSummary +): MetricComparison => ({ + baseline: baseline.median, + candidate: candidate.median, + difference: candidate.median - baseline.median, + percentChange: + baseline.median === 0 + ? null + : ((candidate.median - baseline.median) / Math.abs(baseline.median)) * + 100, +}); + +export const compareCases = ( + baselineCase: string, + candidateCase: string, + baseline: CaseSummary, + candidate: CaseSummary +): CaseComparison => ({ + baselineCase, + candidateCase, + buildMs: compareMetric(baseline.buildMs, candidate.buildMs), + heapUsedAfterBuild: compareMetric( + baseline.heapUsedAfterBuild, + candidate.heapUsedAfterBuild + ), + heapUsedDelta: compareMetric(baseline.heapUsedDelta, candidate.heapUsedDelta), + rssAfterBuild: compareMetric(baseline.rssAfterBuild, candidate.rssAfterBuild), + rssDelta: compareMetric(baseline.rssDelta, candidate.rssDelta), + processPeakRss: compareMetric( + baseline.processPeakRss, + candidate.processPeakRss + ), +}); + +export const validateSchemaGroups = ( + definitions: readonly BenchmarkCaseDefinition[], + runs: readonly BenchmarkRun[] +): { + equivalent: boolean; + hashes: Record; + errors: string[]; +} => { + const groups = new Map>(); + for (const definition of definitions) { + if (!definition.expectedSchemaGroup) continue; + const hashes = + groups.get(definition.expectedSchemaGroup) ?? new Set(); + for (const result of successfulResultsFor(runs, definition.name)) { + hashes.add(result.schemaHash); + } + groups.set(definition.expectedSchemaGroup, hashes); + } + const output: Record = {}; + const errors: string[] = []; + for (const [group, hashes] of groups) { + output[group] = hashes.size === 1 ? [...hashes][0] : null; + if (hashes.size !== 1) { + errors.push(`schema group '${group}' did not produce one schema hash`); + } + } + return { equivalent: errors.length === 0, hashes: output, errors }; +}; diff --git a/packages/perf-harness/src/run.ts b/packages/perf-harness/src/run.ts new file mode 100644 index 0000000000..d1b889a23b --- /dev/null +++ b/packages/perf-harness/src/run.ts @@ -0,0 +1,274 @@ +import { mkdir, rename, writeFile } from 'node:fs/promises'; +import { dirname, resolve } from 'node:path'; + +import { prepareFixture } from './fixture'; +import { DATABASE_URL_ENV, redactSecret, runWorkerProcess } from './process'; +import { summarizeCase, validateSchemaGroups } from './report'; +import { makeSchedule, validateCaseDefinitions } from './schedule'; +import type { + BenchmarkCaseDefinition, + BenchmarkReport, + BenchmarkRun, + BenchmarkSuiteDefinition, +} from './types'; + +export interface RunSuiteOptions { + databaseUrl: string; + repetitions: number; + seed: number; + order: string[] | null; + output?: string; +} + +export const runBenchmarkSuite = async ( + suite: BenchmarkSuiteDefinition, + options: RunSuiteOptions, + workerPath: string +): Promise => { + validateCaseDefinitions(suite.cases); + const byName = new Map( + suite.cases.map((definition) => [definition.name, definition]) + ); + const schedule = makeSchedule( + suite.cases, + options.repetitions, + options.seed, + options.order + ); + const runs: BenchmarkRun[] = []; + for (const coordinate of schedule) { + const definition = byName.get(coordinate.caseName)!; + process.stderr.write( + `[${runs.length + 1}/${schedule.length}] repetition ${ + coordinate.repetition + }, ${coordinate.caseName}\n` + ); + try { + const spawned = await runWorkerProcess( + workerPath, + options.databaseUrl, + definition + ); + runs.push({ ...coordinate, result: spawned.result }); + } catch (error) { + runs.push({ + ...coordinate, + result: { + status: 'error', + pid: -1, + caseName: coordinate.caseName, + error: redactSecret( + error instanceof Error ? error.message : String(error), + options.databaseUrl + ), + }, + }); + } + } + const successfulRuns = runs.filter((run) => run.result.status === 'ok'); + const allRunsSucceeded = successfulRuns.length === schedule.length; + const pids = successfulRuns.map((run) => run.result.pid); + const freshProcessPerRun = + allRunsSucceeded && + pids.every((pid) => pid > 0 && pid !== process.pid) && + new Set(pids).size === pids.length; + const caseValidationPassed = + allRunsSucceeded && + successfulRuns.every( + (run) => run.result.status === 'ok' && run.result.caseValidation.passed + ); + const schemaGroups = validateSchemaGroups(suite.cases, runs); + const errors: string[] = []; + for (const run of runs) { + if (run.result.status === 'error') { + errors.push( + `${run.caseName} repetition ${run.repetition}: ${run.result.error}` + ); + } else { + for (const error of run.result.caseValidation.errors) { + errors.push(`${run.caseName} repetition ${run.repetition}: ${error}`); + } + } + } + if (!freshProcessPerRun) { + errors.push('fresh-process validation did not pass for every run'); + } + errors.push(...schemaGroups.errors); + const summaries: Record< + string, + NonNullable> + > = {}; + for (const definition of suite.cases) { + const summary = summarizeCase(runs, definition.name); + if (summary) summaries[definition.name] = summary; + } + return { + format: 'constructive-performance-suite/v1', + generatedAt: new Date().toISOString(), + node: process.version, + platform: process.platform, + architecture: process.arch, + suite, + config: { + repetitions: options.repetitions, + seed: options.seed, + order: options.order, + }, + schedule, + runs, + validation: { + allRunsSucceeded, + freshProcessPerRun, + caseValidationPassed, + schemaGroupsEquivalent: schemaGroups.equivalent, + schemaGroups: schemaGroups.hashes, + errors, + }, + summaries, + }; +}; + +export const writeJsonAtomically = async ( + output: string, + value: unknown +): Promise => { + const absoluteOutput = resolve(output); + await mkdir(dirname(absoluteOutput), { recursive: true }); + const temporary = `${absoluteOutput}.tmp-${process.pid}`; + await writeFile(temporary, `${JSON.stringify(value, null, 2)}\n`, { + encoding: 'utf8', + mode: 0o600, + }); + await rename(temporary, absoluteOutput); + return absoluteOutput; +}; + +interface ParsedArgs { + values: Map; +} + +const parseArgs = (args: readonly string[]): ParsedArgs => { + const values = new Map(); + for (let index = 0; index < args.length; index += 2) { + const flag = args[index]; + const value = args[index + 1]; + if ( + !flag?.startsWith('--') || + value === undefined || + value.startsWith('--') + ) { + throw new Error(`expected --name value near '${flag ?? ''}'`); + } + const name = flag.slice(2); + if (values.has(name)) + throw new Error(`--${name} may only be specified once`); + values.set(name, value); + } + return { values }; +}; + +const positiveInteger = ( + value: string | undefined, + name: string, + defaultValue: number, + maximum: number +): number => { + if (value === undefined) return defaultValue; + if (!/^\d+$/.test(value)) throw new Error(`--${name} must be an integer`); + const parsed = Number(value); + if (!Number.isSafeInteger(parsed) || parsed < 1 || parsed > maximum) { + throw new Error(`--${name} must be between 1 and ${maximum}`); + } + return parsed; +}; + +const databaseUrl = (args: ParsedArgs): string => { + const value = + args.values.get('database-url') ?? process.env[DATABASE_URL_ENV]; + if (!value) { + throw new Error( + `--database-url or the ${DATABASE_URL_ENV} environment variable is required` + ); + } + return value; +}; + +const stringList = (value: string | undefined): string[] | null => { + if (value === undefined) return null; + const result = value.split(','); + if ( + result.some((item) => item.length === 0 || item.trim() !== item) || + new Set(result).size !== result.length + ) { + throw new Error('list values must be unique exact non-empty strings'); + } + return result; +}; + +const parseCases = (encoded: string): BenchmarkCaseDefinition[] => { + const parsed = JSON.parse(Buffer.from(encoded, 'base64url').toString('utf8')); + if (!Array.isArray(parsed)) + throw new Error('--cases must encode a JSON array'); + return parsed as BenchmarkCaseDefinition[]; +}; + +export const cliMain = async (args = process.argv.slice(2)): Promise => { + const [command, ...rest] = args; + const parsed = parseArgs(rest); + if (command === 'prepare') { + const schema = parsed.values.get('schema'); + if (!schema) throw new Error('--schema is required'); + const result = await prepareFixture({ + databaseUrl: databaseUrl(parsed), + schema, + tables: positiveInteger(parsed.values.get('tables'), 'tables', 64, 500), + }); + process.stdout.write(`${JSON.stringify(result)}\n`); + return; + } + if (command !== 'run') { + throw new Error('expected prepare or run command'); + } + const cases = parsed.values.get('cases'); + const worker = parsed.values.get('worker'); + if (!cases || !worker) throw new Error('--cases and --worker are required'); + const report = await runBenchmarkSuite( + { + name: parsed.values.get('suite') ?? 'benchmark-suite', + cases: parseCases(cases), + }, + { + databaseUrl: databaseUrl(parsed), + repetitions: positiveInteger( + parsed.values.get('repetitions'), + 'repetitions', + 3, + 50 + ), + seed: positiveInteger( + parsed.values.get('seed'), + 'seed', + 20260813, + 0xffffffff + ), + order: stringList(parsed.values.get('order')), + output: parsed.values.get('output'), + }, + resolve(worker) + ); + const output = await writeJsonAtomically( + parsed.values.get('output') ?? 'performance-report.json', + report + ); + process.stdout.write( + `${JSON.stringify({ output, validation: report.validation })}\n` + ); + if ( + !report.validation.allRunsSucceeded || + !report.validation.freshProcessPerRun || + !report.validation.caseValidationPassed || + !report.validation.schemaGroupsEquivalent + ) { + process.exitCode = 1; + } +}; diff --git a/packages/perf-harness/src/schedule.ts b/packages/perf-harness/src/schedule.ts new file mode 100644 index 0000000000..8eaa98fb86 --- /dev/null +++ b/packages/perf-harness/src/schedule.ts @@ -0,0 +1,77 @@ +import type { BenchmarkCaseDefinition, BenchmarkCoordinate } from './types'; + +const seededRandom = (seed: number): (() => number) => { + let state = seed >>> 0; + return () => { + state += 0x6d2b79f5; + let value = state; + value = Math.imul(value ^ (value >>> 15), value | 1); + value ^= value + Math.imul(value ^ (value >>> 7), value | 61); + return ((value ^ (value >>> 14)) >>> 0) / 4294967296; + }; +}; + +const shuffledCaseNames = ( + definitions: readonly BenchmarkCaseDefinition[], + seed: number, + repetition: number +): string[] => { + const result = definitions.map(({ name }) => name); + const random = seededRandom((seed ^ Math.imul(repetition, 0x9e3779b1)) >>> 0); + for (let index = result.length - 1; index > 0; index -= 1) { + const swapIndex = Math.floor(random() * (index + 1)); + [result[index], result[swapIndex]] = [result[swapIndex], result[index]]; + } + return result; +}; + +export const validateCaseDefinitions = ( + definitions: readonly BenchmarkCaseDefinition[] +): void => { + if (definitions.length === 0) { + throw new Error('benchmark suite must contain at least one case'); + } + const names = definitions.map(({ name }) => name); + if ( + names.some( + (name) => name.length === 0 || name.trim() !== name || name.includes('\0') + ) || + new Set(names).size !== names.length + ) { + throw new Error( + 'benchmark case names must be unique exact non-empty strings' + ); + } +}; + +export const makeSchedule = ( + definitions: readonly BenchmarkCaseDefinition[], + repetitions: number, + seed: number, + exactOrder: readonly string[] | null = null +): BenchmarkCoordinate[] => { + validateCaseDefinitions(definitions); + if (!Number.isSafeInteger(repetitions) || repetitions < 1) { + throw new Error('repetitions must be a positive safe integer'); + } + const expectedNames = definitions.map(({ name }) => name).sort(); + const schedule: BenchmarkCoordinate[] = []; + for (let repetition = 1; repetition <= repetitions; repetition += 1) { + const order = exactOrder + ? [...exactOrder] + : shuffledCaseNames(definitions, seed, repetition); + if ( + order.length !== definitions.length || + new Set(order).size !== definitions.length || + [...order].sort().some((name, index) => name !== expectedNames[index]) + ) { + throw new Error( + 'exact order must contain each benchmark case exactly once' + ); + } + order.forEach((caseName, index) => { + schedule.push({ repetition, position: index + 1, caseName }); + }); + } + return schedule; +}; diff --git a/packages/perf-harness/src/stock-worker.ts b/packages/perf-harness/src/stock-worker.ts new file mode 100644 index 0000000000..71eb1f357f --- /dev/null +++ b/packages/perf-harness/src/stock-worker.ts @@ -0,0 +1,94 @@ +import { createHash } from 'node:crypto'; + +import { + defaultPreset as graphileBuildPreset, + makeSchema, +} from 'graphile-build'; +import { defaultPreset as graphileBuildPgPreset } from 'graphile-build-pg'; +import { execute, lexicographicSortSchema, parse, printSchema } from 'graphql'; +import { makePgService } from 'postgraphile/adaptors/pg'; + +import { measureBenchmarkCase } from './metrics'; +import { + DATABASE_URL_ENV, + parseWorkerEnvelope, + redactSecret, + WORKER_CONFIG_ENV, + writeWorkerResult, +} from './process'; + +interface StockConfig { + schemas: string[]; +} + +const validateConfig = (value: unknown): StockConfig => { + const schemas = (value as Partial)?.schemas; + if ( + !Array.isArray(schemas) || + schemas.length === 0 || + schemas.some((schema) => typeof schema !== 'string' || schema.length === 0) + ) { + throw new Error('stock worker requires a non-empty schemas array'); + } + return { schemas }; +}; + +const main = async (): Promise => { + const databaseUrl = process.env[DATABASE_URL_ENV] ?? ''; + let caseName = 'unknown'; + try { + if (!databaseUrl) throw new Error(`${DATABASE_URL_ENV} is required`); + const envelope = parseWorkerEnvelope(process.env[WORKER_CONFIG_ENV]); + caseName = envelope.caseName; + const config = validateConfig(envelope.workerConfig); + const service = makePgService({ + connectionString: databaseUrl, + schemas: config.schemas, + pubsub: false, + }); + try { + const result = await measureBenchmarkCase( + caseName, + async () => + makeSchema({ + extends: [graphileBuildPreset, graphileBuildPgPreset], + pgServices: [service], + }), + async ({ schema }) => { + const execution = await execute({ + schema, + document: parse('{ __typename }'), + }); + if ( + execution.errors?.length || + execution.data?.__typename !== 'Query' + ) { + throw new Error('runtime verification query failed'); + } + const schemaText = printSchema(lexicographicSortSchema(schema)); + return { + schemaHash: createHash('sha256').update(schemaText).digest('hex'), + schemaTypeCount: Object.keys(schema.getTypeMap()).length, + runtimeVerified: true as const, + }; + } + ); + writeWorkerResult(result); + } finally { + await service.release(); + } + } catch (error) { + writeWorkerResult({ + status: 'error', + pid: process.pid, + caseName, + error: redactSecret( + error instanceof Error ? error.message : String(error), + databaseUrl + ), + }); + process.exitCode = 1; + } +}; + +if (require.main === module) void main(); diff --git a/packages/perf-harness/src/types.ts b/packages/perf-harness/src/types.ts new file mode 100644 index 0000000000..3713a4edc4 --- /dev/null +++ b/packages/perf-harness/src/types.ts @@ -0,0 +1,129 @@ +export type JsonPrimitive = string | number | boolean | null; +export type JsonValue = + JsonPrimitive | JsonValue[] | { [key: string]: JsonValue }; + +export interface BenchmarkCaseDefinition { + name: string; + workerConfig: JsonValue; + expectedSchemaGroup?: string; +} + +export interface BenchmarkSuiteDefinition { + name: string; + cases: BenchmarkCaseDefinition[]; +} + +export interface BenchmarkCoordinate { + repetition: number; + position: number; + caseName: string; +} + +export interface WorkerConfigEnvelope { + caseName: string; + workerConfig: JsonValue; +} + +export interface MemorySnapshot { + rss: number; + heapTotal: number; + heapUsed: number; + external: number; + arrayBuffers: number; +} + +export interface CaseValidation { + passed: boolean; + errors: string[]; +} + +export interface SuccessfulWorkerResult { + status: 'ok'; + pid: number; + caseName: string; + buildMs: number; + schemaHash: string; + schemaTypeCount: number; + runtimeVerified: true; + caseValidation: CaseValidation; + metadata?: Record; + memory: { + baseline: MemorySnapshot; + afterBuild: MemorySnapshot; + delta: MemorySnapshot; + processPeakRss: number; + }; +} + +export interface FailedWorkerResult { + status: 'error'; + pid: number; + caseName: string; + error: string; +} + +export type WorkerResult = SuccessfulWorkerResult | FailedWorkerResult; + +export interface BenchmarkRun extends BenchmarkCoordinate { + result: WorkerResult; +} + +export interface MetricSummary { + median: number; + min: number; + max: number; + samples: number[]; +} + +export interface CaseSummary { + sampleCount: number; + buildMs: MetricSummary; + heapUsedAfterBuild: MetricSummary; + heapUsedDelta: MetricSummary; + rssAfterBuild: MetricSummary; + rssDelta: MetricSummary; + processPeakRss: MetricSummary; +} + +export interface MetricComparison { + baseline: number; + candidate: number; + difference: number; + percentChange: number | null; +} + +export interface CaseComparison { + baselineCase: string; + candidateCase: string; + buildMs: MetricComparison; + heapUsedAfterBuild: MetricComparison; + heapUsedDelta: MetricComparison; + rssAfterBuild: MetricComparison; + rssDelta: MetricComparison; + processPeakRss: MetricComparison; +} + +export interface BenchmarkReport { + format: 'constructive-performance-suite/v1'; + generatedAt: string; + node: string; + platform: string; + architecture: string; + suite: BenchmarkSuiteDefinition; + config: { + repetitions: number; + seed: number; + order: string[] | null; + }; + schedule: BenchmarkCoordinate[]; + runs: BenchmarkRun[]; + validation: { + allRunsSucceeded: boolean; + freshProcessPerRun: boolean; + caseValidationPassed: boolean; + schemaGroupsEquivalent: boolean; + schemaGroups: Record; + errors: string[]; + }; + summaries: Record; +} diff --git a/packages/perf-harness/tsconfig.esm.json b/packages/perf-harness/tsconfig.esm.json new file mode 100644 index 0000000000..1a3c9914f1 --- /dev/null +++ b/packages/perf-harness/tsconfig.esm.json @@ -0,0 +1,8 @@ +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "outDir": "dist/esm", + "module": "esnext", + "moduleResolution": "bundler" + } +} diff --git a/packages/perf-harness/tsconfig.json b/packages/perf-harness/tsconfig.json new file mode 100644 index 0000000000..319daa4b0d --- /dev/null +++ b/packages/perf-harness/tsconfig.json @@ -0,0 +1,11 @@ +{ + "extends": "../../tsconfig.json", + "compilerOptions": { + "outDir": "dist", + "rootDir": "src", + "moduleResolution": "nodenext", + "module": "nodenext", + "isolatedModules": true + }, + "include": ["src/**/*"] +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9f681e75ea..0fac205134 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2711,6 +2711,37 @@ importers: version: 0.8.0 publishDirectory: dist + packages/perf-harness: + dependencies: + graphile-build: + specifier: 5.1.1 + version: 5.1.1(grafast@1.1.2(graphql@16.13.0))(graphile-config@1.1.0)(graphql@16.13.0) + graphile-build-pg: + specifier: 5.1.3 + version: 5.1.3(@dataplan/pg@1.1.1(@dataplan/json@1.0.1(grafast@1.1.2(graphql@16.13.0)))(grafast@1.1.2(graphql@16.13.0))(graphile-config@1.1.0)(graphql@16.13.0)(pg-sql2@5.0.1)(pg@8.21.0))(grafast@1.1.2(graphql@16.13.0))(graphile-build@5.1.1(grafast@1.1.2(graphql@16.13.0))(graphile-config@1.1.0)(graphql@16.13.0))(graphile-config@1.1.0)(graphql@16.13.0)(pg-sql2@5.0.1)(pg@8.21.0)(tamedevil@0.1.1) + graphile-config: + specifier: 1.1.0 + version: 1.1.0 + graphql: + specifier: 16.13.0 + version: 16.13.0 + pg: + specifier: ^8.21.0 + version: 8.21.0 + postgraphile: + specifier: 5.1.4 + version: 5.1.4(f282a162d8bd20a217e08c60f5396af8) + devDependencies: + '@types/node': + specifier: ^22.19.11 + version: 22.19.19 + '@types/pg': + specifier: ^8.20.4 + version: 8.20.4 + makage: + specifier: ^0.3.0 + version: 0.3.0 + packages/postmaster: dependencies: 12factor-env: From 5f2c453d224aaec943d08f348abd4939cec9cfd8 Mon Sep 17 00:00:00 2001 From: zetazzz Date: Tue, 18 Aug 2026 13:10:40 +0800 Subject: [PATCH 2/4] Use CLI arguments for performance workers --- packages/perf-harness/README.md | 7 +- .../__tests__/fixtures/fake-worker.js | 12 ++- .../perf-harness/__tests__/process.test.ts | 51 +++++++++++- packages/perf-harness/__tests__/run.test.ts | 8 +- packages/perf-harness/src/process.ts | 83 +++++++++++++++---- packages/perf-harness/src/run.ts | 39 ++------- packages/perf-harness/src/stock-worker.ts | 11 ++- 7 files changed, 151 insertions(+), 60 deletions(-) diff --git a/packages/perf-harness/README.md b/packages/perf-harness/README.md index 484670a951..01fc2f1d47 100644 --- a/packages/perf-harness/README.md +++ b/packages/perf-harness/README.md @@ -32,8 +32,11 @@ await runBenchmarkSuite(suite, options, workerPath); entry rather than serializing functions across process boundaries. The package includes `stock-worker.js` as a minimal upstream Graphile baseline. -Database credentials are supplied through `CPERF_DATABASE_URL` and are redacted -from worker failures and JSON reports. +The top-level commands require `--database-url`; the runner forwards it and the +opaque case configuration to each short-lived worker as CLI arguments. Database +credentials are redacted from worker failures and JSON reports. This harness is +intended for local development on a trusted machine because command arguments +may be visible to other local processes. The PostgreSQL fixture command only creates a previously absent schema whose name starts with `cperf_`; it never drops or replaces schemas. diff --git a/packages/perf-harness/__tests__/fixtures/fake-worker.js b/packages/perf-harness/__tests__/fixtures/fake-worker.js index ea69962ace..5ba1e05b83 100644 --- a/packages/perf-harness/__tests__/fixtures/fake-worker.js +++ b/packages/perf-harness/__tests__/fixtures/fake-worker.js @@ -1,7 +1,17 @@ 'use strict'; +const valueFor = (name) => { + const flag = `--${name}`; + const index = process.argv.indexOf(flag); + if (index < 0 || !process.argv[index + 1]) { + throw new Error(`${flag} is required`); + } + return process.argv[index + 1]; +}; + +valueFor('database-url'); const envelope = JSON.parse( - Buffer.from(process.env.CPERF_WORKER_CONFIG, 'base64url').toString('utf8') + Buffer.from(valueFor('worker-config'), 'base64url').toString('utf8') ); const value = envelope.workerConfig.value; const memory = { diff --git a/packages/perf-harness/__tests__/process.test.ts b/packages/perf-harness/__tests__/process.test.ts index 65fd7ef64c..4016c8df51 100644 --- a/packages/perf-harness/__tests__/process.test.ts +++ b/packages/perf-harness/__tests__/process.test.ts @@ -1,6 +1,55 @@ import { resolve } from 'node:path'; -import { runWorkerProcess } from '../src/process'; +import { parseWorkerProcessArgs, runWorkerProcess } from '../src/process'; + +describe('worker CLI protocol', () => { + const encodedConfig = Buffer.from( + JSON.stringify({ caseName: 'baseline', workerConfig: { value: 1 } }) + ).toString('base64url'); + + test('parses the database URL and worker envelope from CLI arguments', () => { + expect( + parseWorkerProcessArgs([ + '--database-url', + 'postgres:///benchmark', + '--worker-config', + encodedConfig, + ]) + ).toEqual({ + databaseUrl: 'postgres:///benchmark', + envelope: { caseName: 'baseline', workerConfig: { value: 1 } }, + }); + }); + + test('rejects missing, duplicate, and unsupported worker arguments', () => { + expect(() => + parseWorkerProcessArgs(['--worker-config', encodedConfig]) + ).toThrow('--database-url is required'); + expect(() => + parseWorkerProcessArgs(['--database-url', 'postgres:///benchmark']) + ).toThrow('--worker-config is required'); + expect(() => + parseWorkerProcessArgs([ + '--database-url', + 'postgres:///one', + '--database-url', + 'postgres:///two', + '--worker-config', + encodedConfig, + ]) + ).toThrow('--database-url may only be specified once'); + expect(() => + parseWorkerProcessArgs([ + '--database-url', + 'postgres:///benchmark', + '--worker-config', + encodedConfig, + '--unexpected', + 'value', + ]) + ).toThrow("unsupported worker argument '--unexpected'"); + }); +}); describe('fresh worker process', () => { test('uses distinct PIDs and does not expose the database URL', async () => { diff --git a/packages/perf-harness/__tests__/run.test.ts b/packages/perf-harness/__tests__/run.test.ts index 0628e1ba43..4e8ef65d93 100644 --- a/packages/perf-harness/__tests__/run.test.ts +++ b/packages/perf-harness/__tests__/run.test.ts @@ -1,8 +1,14 @@ import { resolve } from 'node:path'; -import { runBenchmarkSuite } from '../src/run'; +import { cliMain, runBenchmarkSuite } from '../src/run'; describe('generic suite runner', () => { + test('requires the database URL as an explicit CLI argument', async () => { + await expect( + cliMain(['prepare', '--schema', 'cperf_explicit_cli']) + ).rejects.toThrow('--database-url is required'); + }); + test('validates fresh processes and schema groups without fixed case names', async () => { const report = await runBenchmarkSuite( { diff --git a/packages/perf-harness/src/process.ts b/packages/perf-harness/src/process.ts index 14c5464a60..dee9097204 100644 --- a/packages/perf-harness/src/process.ts +++ b/packages/perf-harness/src/process.ts @@ -7,8 +7,17 @@ import type { } from './types'; export const WORKER_RESULT_PREFIX = 'CPERF_RESULT '; -export const DATABASE_URL_ENV = 'CPERF_DATABASE_URL'; -export const WORKER_CONFIG_ENV = 'CPERF_WORKER_CONFIG'; +export const DATABASE_URL_ARGUMENT = 'database-url'; +export const WORKER_CONFIG_ARGUMENT = 'worker-config'; + +export interface ParsedValueArgs { + values: Map; +} + +export interface WorkerProcessArgs { + databaseUrl: string; + envelope: WorkerConfigEnvelope; +} export interface SpawnedWorkerResult { pid: number; @@ -21,6 +30,43 @@ const lastLines = (value: string, count = 20): string => export const redactSecret = (value: string, secret: string): string => secret ? value.replaceAll(secret, '') : value; +export const parseValueArgs = (args: readonly string[]): ParsedValueArgs => { + const values = new Map(); + for (let index = 0; index < args.length; index += 2) { + const flag = args[index]; + const value = args[index + 1]; + if ( + !flag?.startsWith('--') || + value === undefined || + value.startsWith('--') + ) { + throw new Error(`expected --name value near '${flag ?? ''}'`); + } + const name = flag.slice(2); + if (values.has(name)) + throw new Error(`--${name} may only be specified once`); + values.set(name, value); + } + return { values }; +}; + +export const parseWorkerProcessArgs = ( + args: readonly string[] +): WorkerProcessArgs => { + const parsed = parseValueArgs(args); + for (const name of parsed.values.keys()) { + if (name !== DATABASE_URL_ARGUMENT && name !== WORKER_CONFIG_ARGUMENT) { + throw new Error(`unsupported worker argument '--${name}'`); + } + } + const databaseUrl = parsed.values.get(DATABASE_URL_ARGUMENT); + if (!databaseUrl) throw new Error('--database-url is required'); + return { + databaseUrl, + envelope: parseWorkerEnvelope(parsed.values.get(WORKER_CONFIG_ARGUMENT)), + }; +}; + export const runWorkerProcess = ( workerPath: string, databaseUrl: string, @@ -31,18 +77,25 @@ export const runWorkerProcess = ( caseName: definition.name, workerConfig: definition.workerConfig, }; - const child = spawn(process.execPath, ['--expose-gc', workerPath], { - env: { - ...process.env, - NODE_ENV: 'production', - GRAPHILE_ENV: 'production', - [DATABASE_URL_ENV]: databaseUrl, - [WORKER_CONFIG_ENV]: Buffer.from(JSON.stringify(config)).toString( - 'base64url' - ), - }, - stdio: ['ignore', 'pipe', 'pipe'], - }); + const child = spawn( + process.execPath, + [ + '--expose-gc', + workerPath, + `--${DATABASE_URL_ARGUMENT}`, + databaseUrl, + `--${WORKER_CONFIG_ARGUMENT}`, + Buffer.from(JSON.stringify(config)).toString('base64url'), + ], + { + env: { + ...process.env, + NODE_ENV: 'production', + GRAPHILE_ENV: 'production', + }, + stdio: ['ignore', 'pipe', 'pipe'], + } + ); const pid = child.pid; let stdout = ''; let stderr = ''; @@ -111,7 +164,7 @@ export const runWorkerProcess = ( export const parseWorkerEnvelope = ( encoded: string | undefined ): WorkerConfigEnvelope => { - if (!encoded) throw new Error(`${WORKER_CONFIG_ENV} is required`); + if (!encoded) throw new Error('--worker-config is required'); const parsed = JSON.parse( Buffer.from(encoded, 'base64url').toString('utf8') ) as Partial; diff --git a/packages/perf-harness/src/run.ts b/packages/perf-harness/src/run.ts index d1b889a23b..ea6cbaed89 100644 --- a/packages/perf-harness/src/run.ts +++ b/packages/perf-harness/src/run.ts @@ -2,7 +2,7 @@ import { mkdir, rename, writeFile } from 'node:fs/promises'; import { dirname, resolve } from 'node:path'; import { prepareFixture } from './fixture'; -import { DATABASE_URL_ENV, redactSecret, runWorkerProcess } from './process'; +import { parseValueArgs, redactSecret, runWorkerProcess } from './process'; import { summarizeCase, validateSchemaGroups } from './report'; import { makeSchedule, validateCaseDefinitions } from './schedule'; import type { @@ -143,30 +143,6 @@ export const writeJsonAtomically = async ( return absoluteOutput; }; -interface ParsedArgs { - values: Map; -} - -const parseArgs = (args: readonly string[]): ParsedArgs => { - const values = new Map(); - for (let index = 0; index < args.length; index += 2) { - const flag = args[index]; - const value = args[index + 1]; - if ( - !flag?.startsWith('--') || - value === undefined || - value.startsWith('--') - ) { - throw new Error(`expected --name value near '${flag ?? ''}'`); - } - const name = flag.slice(2); - if (values.has(name)) - throw new Error(`--${name} may only be specified once`); - values.set(name, value); - } - return { values }; -}; - const positiveInteger = ( value: string | undefined, name: string, @@ -182,14 +158,9 @@ const positiveInteger = ( return parsed; }; -const databaseUrl = (args: ParsedArgs): string => { - const value = - args.values.get('database-url') ?? process.env[DATABASE_URL_ENV]; - if (!value) { - throw new Error( - `--database-url or the ${DATABASE_URL_ENV} environment variable is required` - ); - } +const databaseUrl = (args: ReturnType): string => { + const value = args.values.get('database-url'); + if (!value) throw new Error('--database-url is required'); return value; }; @@ -214,7 +185,7 @@ const parseCases = (encoded: string): BenchmarkCaseDefinition[] => { export const cliMain = async (args = process.argv.slice(2)): Promise => { const [command, ...rest] = args; - const parsed = parseArgs(rest); + const parsed = parseValueArgs(rest); if (command === 'prepare') { const schema = parsed.values.get('schema'); if (!schema) throw new Error('--schema is required'); diff --git a/packages/perf-harness/src/stock-worker.ts b/packages/perf-harness/src/stock-worker.ts index 71eb1f357f..c68cc18f0d 100644 --- a/packages/perf-harness/src/stock-worker.ts +++ b/packages/perf-harness/src/stock-worker.ts @@ -10,10 +10,8 @@ import { makePgService } from 'postgraphile/adaptors/pg'; import { measureBenchmarkCase } from './metrics'; import { - DATABASE_URL_ENV, - parseWorkerEnvelope, + parseWorkerProcessArgs, redactSecret, - WORKER_CONFIG_ENV, writeWorkerResult, } from './process'; @@ -34,11 +32,12 @@ const validateConfig = (value: unknown): StockConfig => { }; const main = async (): Promise => { - const databaseUrl = process.env[DATABASE_URL_ENV] ?? ''; + let databaseUrl = ''; let caseName = 'unknown'; try { - if (!databaseUrl) throw new Error(`${DATABASE_URL_ENV} is required`); - const envelope = parseWorkerEnvelope(process.env[WORKER_CONFIG_ENV]); + const workerArgs = parseWorkerProcessArgs(process.argv.slice(2)); + databaseUrl = workerArgs.databaseUrl; + const { envelope } = workerArgs; caseName = envelope.caseName; const config = validateConfig(envelope.workerConfig); const service = makePgService({ From 658736e7e633709ea454f461ce5af3075dde358d Mon Sep 17 00:00:00 2001 From: Codex Date: Sun, 6 Sep 2026 01:26:52 +0000 Subject: [PATCH 3/4] chore(perf-harness): align rebased package with current build and CI policy --- .github/workflows/run-tests.yaml | 2 +- packages/perf-harness/package.json | 2 +- pnpm-lock.yaml | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/run-tests.yaml b/.github/workflows/run-tests.yaml index 7a89b17250..5a4b52fdf1 100644 --- a/.github/workflows/run-tests.yaml +++ b/.github/workflows/run-tests.yaml @@ -109,7 +109,7 @@ jobs: - batch: packages-core packages: 'packages/url-domains packages/coerce packages/csrf packages/oauth packages/12factor-env packages/orm packages/express-context packages/errors packages/llm-env packages/node-type-registry packages/query-spec packages/server-utils packages/site-deploy examples/site-deploy-ssg postgres/pg-cache postgres/pg-env' - batch: packages-services - packages: 'packages/postmaster packages/smtppostmaster packages/csv-to-pg packages/cli postgres/pgsql-client postgres/pg-ast' + packages: 'packages/postmaster packages/smtppostmaster packages/csv-to-pg packages/cli packages/perf-harness postgres/pgsql-client postgres/pg-ast' - batch: graphql packages: 'graphql/query graphql/codegen' - batch: graphile-unit diff --git a/packages/perf-harness/package.json b/packages/perf-harness/package.json index 8d23b8bc10..d975054937 100644 --- a/packages/perf-harness/package.json +++ b/packages/perf-harness/package.json @@ -27,7 +27,7 @@ "devDependencies": { "@types/node": "^22.19.11", "@types/pg": "^8.20.4", - "makage": "^0.3.0" + "makage": "^0.8.0" }, "engines": { "node": ">=22" diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 0fac205134..ca6ae59d08 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2739,8 +2739,8 @@ importers: specifier: ^8.20.4 version: 8.20.4 makage: - specifier: ^0.3.0 - version: 0.3.0 + specifier: ^0.8.0 + version: 0.8.0 packages/postmaster: dependencies: From 8ee2464fe59b90c6831084dc94a99cb41d5585fa Mon Sep 17 00:00:00 2001 From: zetazzz Date: Mon, 17 Aug 2026 08:19:12 +0800 Subject: [PATCH 4/4] fix(graphile): qualify built-in plugin SQL --- .../__tests__/identifier-quoting.test.ts | 47 +++ graphile/graphile-bulk-mutations/package.json | 3 + .../src/plugins/BulkDeletePlugin.ts | 31 +- .../src/plugins/BulkInsertPlugin.ts | 3 +- .../src/plugins/BulkUpdatePlugin.ts | 37 +- .../src/plugins/BulkUpsertPlugin.ts | 3 +- .../src/utils/sql-builder.ts | 23 +- graphile/graphile-i18n/package.json | 1 + .../src/__tests__/sql-qualification.test.ts | 123 +++++++ graphile/graphile-i18n/src/plugin.ts | 317 ++++++++++-------- graphile/graphile-llm/package.json | 1 + .../src/__tests__/rag-sql.test.ts | 123 +++++++ .../graphile-llm/src/plugins/rag-plugin.ts | 148 +++++++- graphile/graphile-llm/src/types.ts | 2 + .../__tests__/schema-qualified-sql.test.ts | 233 +++++++++++++ .../plugins/connection-filter-operators.ts | 60 ++-- .../src/plugins/detect-ltree.ts | 138 ++++++-- .../src/plugins/folder-filter-operators.ts | 56 ++-- .../graphile-ltree/src/plugins/ltree-codec.ts | 36 +- .../src/plugins/qualified-sql.ts | 41 +++ .../graphile-postgis/__tests__/codec.test.ts | 35 +- .../connection-filter-operators.test.ts | 10 +- .../__tests__/detect-extension.test.ts | 54 ++- .../__tests__/spatial-relations.test.ts | 18 + .../graphile-postgis/src/plugins/codec.ts | 35 +- .../plugins/connection-filter-operators.ts | 32 +- .../src/plugins/detect-extension.ts | 96 +++--- .../src/plugins/spatial-relations.ts | 6 +- graphile/graphile-postgis/src/types.ts | 2 + graphile/graphile-search/package.json | 1 + .../extension-schema-qualification.test.ts | 248 ++++++++++++++ .../src/__tests__/search-config.test.ts | 63 +++- .../src/__tests__/sql-qualification.test.ts | 45 +++ graphile/graphile-search/src/adapters/bm25.ts | 25 +- .../graphile-search/src/adapters/pgvector.ts | 77 ++++- graphile/graphile-search/src/adapters/trgm.ts | 52 ++- .../graphile-search/src/codecs/bm25-codec.ts | 6 + .../src/codecs/operator-factories.ts | 9 +- .../src/codecs/vector-codec.ts | 35 +- .../graphile-search/src/extension-metadata.ts | 232 +++++++++++++ graphile/graphile-search/src/index.ts | 7 + graphile/graphile-search/src/plugin.ts | 16 + pnpm-lock.yaml | 12 + 43 files changed, 2151 insertions(+), 391 deletions(-) create mode 100644 graphile/graphile-bulk-mutations/__tests__/identifier-quoting.test.ts create mode 100644 graphile/graphile-i18n/src/__tests__/sql-qualification.test.ts create mode 100644 graphile/graphile-llm/src/__tests__/rag-sql.test.ts create mode 100644 graphile/graphile-ltree/src/__tests__/schema-qualified-sql.test.ts create mode 100644 graphile/graphile-ltree/src/plugins/qualified-sql.ts create mode 100644 graphile/graphile-search/src/__tests__/extension-schema-qualification.test.ts create mode 100644 graphile/graphile-search/src/__tests__/sql-qualification.test.ts create mode 100644 graphile/graphile-search/src/extension-metadata.ts diff --git a/graphile/graphile-bulk-mutations/__tests__/identifier-quoting.test.ts b/graphile/graphile-bulk-mutations/__tests__/identifier-quoting.test.ts new file mode 100644 index 0000000000..f7f724d552 --- /dev/null +++ b/graphile/graphile-bulk-mutations/__tests__/identifier-quoting.test.ts @@ -0,0 +1,47 @@ +import { + buildBulkDeleteSQL, + buildBulkInsertSQL, + buildBulkUpdateSQL, +} from '../src/utils/sql-builder'; + +describe('bulk mutation catalog identifier quoting', () => { + const hostile = 'value" RETURNING secret --'; + const quoted = '"value"" RETURNING secret --"'; + + it('escapes insert, conflict, update, and returning identifiers', () => { + const [query] = buildBulkInsertSQL( + 'tenant_a.items', + [{ name: hostile, sqlType: 'text' }], + [{ [hostile]: 'safe-value' }], + [hostile], + { conflictColumns: [hostile], action: 'UPDATE', updateColumns: [hostile] } + ); + + expect(query.text).toContain(`(${quoted})`); + expect(query.text).toContain(`ON CONFLICT (${quoted})`); + expect(query.text).toContain(`${quoted} = EXCLUDED.${quoted}`); + expect(query.text).toContain(`RETURNING ${quoted}`); + expect(query.values).toEqual(['safe-value']); + }); + + it('escapes update and delete identifiers', () => { + const update = buildBulkUpdateSQL( + 'tenant_a.items', + { [hostile]: 'safe-value' }, + [{ name: hostile, sqlType: 'text' }], + [hostile], + 'TRUE', + [] + ); + const deletion = buildBulkDeleteSQL( + 'tenant_a.items', + [hostile], + 'TRUE', + [] + ); + + expect(update.text).toContain(`${quoted} = $1::text`); + expect(update.text).toContain(`RETURNING ${quoted}`); + expect(deletion.text).toContain(`RETURNING ${quoted}`); + }); +}); diff --git a/graphile/graphile-bulk-mutations/package.json b/graphile/graphile-bulk-mutations/package.json index 860fce4a76..5ee1b3809c 100644 --- a/graphile/graphile-bulk-mutations/package.json +++ b/graphile/graphile-bulk-mutations/package.json @@ -41,6 +41,9 @@ "bugs": { "url": "https://github.com/constructive-io/constructive/issues" }, + "dependencies": { + "@pgsql/quotes": "^18.2.4" + }, "devDependencies": { "@types/node": "^22.19.11", "graphile-test": "workspace:^", diff --git a/graphile/graphile-bulk-mutations/src/plugins/BulkDeletePlugin.ts b/graphile/graphile-bulk-mutations/src/plugins/BulkDeletePlugin.ts index 4731702283..c1881bdfa6 100644 --- a/graphile/graphile-bulk-mutations/src/plugins/BulkDeletePlugin.ts +++ b/graphile/graphile-bulk-mutations/src/plugins/BulkDeletePlugin.ts @@ -1,6 +1,7 @@ import '../augmentations'; import { sideEffectWithPgClient } from '@dataplan/pg'; +import { QuoteUtils } from '@pgsql/quotes'; import type { GraphileConfig } from 'graphile-config'; import type { GraphQLInputType,GraphQLOutputType } from 'graphql'; @@ -79,7 +80,9 @@ export const BulkDeletePlugin: GraphileConfig.Plugin = { // Extract primary key columns for RETURNING clause const primaryUnique = resource.uniques.find((u: any) => u.isPrimary) ?? resource.uniques[0]; const pkColumns: string[] = primaryUnique.attributes; - const pkReturning = pkColumns.map((c) => `"${c}"`).join(', '); + const pkReturning = pkColumns + .map((c) => QuoteUtils.quoteIdentifier(c)) + .join(', '); const compiledFrom = sql.compile(resource.from).text; @@ -125,11 +128,13 @@ export const BulkDeletePlugin: GraphileConfig.Plugin = { const sqlType = attrToSqlType[attrName]; if (spec === null) { - whereClauses.push(`"${attrName}" IS NULL`); + whereClauses.push(`${QuoteUtils.quoteIdentifier(attrName)} IS NULL`); } else if (spec !== undefined && typeof spec !== 'object') { // Simple equality (Condition type) values.push(spec); - whereClauses.push(`"${attrName}" = $${values.length}::${sqlType}`); + whereClauses.push( + `${QuoteUtils.quoteIdentifier(attrName)} = $${values.length}::${sqlType}` + ); } else if (spec && typeof spec === 'object') { // Operator-based (Filter type) for (const [op, val] of Object.entries(spec) as [string, any][]) { @@ -137,22 +142,22 @@ export const BulkDeletePlugin: GraphileConfig.Plugin = { const paramRef = `$${values.length}::${sqlType}`; switch (op) { case 'equalTo': - whereClauses.push(`"${attrName}" = ${paramRef}`); + whereClauses.push(`${QuoteUtils.quoteIdentifier(attrName)} = ${paramRef}`); break; case 'notEqualTo': - whereClauses.push(`"${attrName}" != ${paramRef}`); + whereClauses.push(`${QuoteUtils.quoteIdentifier(attrName)} != ${paramRef}`); break; case 'greaterThan': - whereClauses.push(`"${attrName}" > ${paramRef}`); + whereClauses.push(`${QuoteUtils.quoteIdentifier(attrName)} > ${paramRef}`); break; case 'greaterThanOrEqualTo': - whereClauses.push(`"${attrName}" >= ${paramRef}`); + whereClauses.push(`${QuoteUtils.quoteIdentifier(attrName)} >= ${paramRef}`); break; case 'lessThan': - whereClauses.push(`"${attrName}" < ${paramRef}`); + whereClauses.push(`${QuoteUtils.quoteIdentifier(attrName)} < ${paramRef}`); break; case 'lessThanOrEqualTo': - whereClauses.push(`"${attrName}" <= ${paramRef}`); + whereClauses.push(`${QuoteUtils.quoteIdentifier(attrName)} <= ${paramRef}`); break; case 'in': if (Array.isArray(val)) { @@ -161,15 +166,17 @@ export const BulkDeletePlugin: GraphileConfig.Plugin = { return `$${values.length}::${sqlType}`; }); values.pop(); - whereClauses.push(`"${attrName}" IN (${placeholders.join(', ')})`); + whereClauses.push( + `${QuoteUtils.quoteIdentifier(attrName)} IN (${placeholders.join(', ')})` + ); } break; case 'isNull': values.pop(); if (val) { - whereClauses.push(`"${attrName}" IS NULL`); + whereClauses.push(`${QuoteUtils.quoteIdentifier(attrName)} IS NULL`); } else { - whereClauses.push(`"${attrName}" IS NOT NULL`); + whereClauses.push(`${QuoteUtils.quoteIdentifier(attrName)} IS NOT NULL`); } break; default: diff --git a/graphile/graphile-bulk-mutations/src/plugins/BulkInsertPlugin.ts b/graphile/graphile-bulk-mutations/src/plugins/BulkInsertPlugin.ts index 6b128024a1..73eed3539e 100644 --- a/graphile/graphile-bulk-mutations/src/plugins/BulkInsertPlugin.ts +++ b/graphile/graphile-bulk-mutations/src/plugins/BulkInsertPlugin.ts @@ -1,6 +1,7 @@ import '../augmentations'; import { sideEffectWithPgClient } from '@dataplan/pg'; +import { QuoteUtils } from '@pgsql/quotes'; import type { GraphileConfig } from 'graphile-config'; import type { GraphQLInputType, GraphQLOutputType } from 'graphql'; @@ -275,7 +276,7 @@ export const BulkInsertPlugin: GraphileConfig.Plugin = { const pkConditions = allPkRows.map((pkRow, rowIdx) => { return pkColumns.map((col, colIdx) => { const paramIdx = rowIdx * pkColumns.length + colIdx + 1; - return `"${col}" = $${paramIdx}`; + return `${QuoteUtils.quoteIdentifier(col)} = $${paramIdx}`; }).join(' AND '); }); const whereClause = pkConditions.map((c) => `(${c})`).join(' OR '); diff --git a/graphile/graphile-bulk-mutations/src/plugins/BulkUpdatePlugin.ts b/graphile/graphile-bulk-mutations/src/plugins/BulkUpdatePlugin.ts index 8ef1170067..4ba276de8b 100644 --- a/graphile/graphile-bulk-mutations/src/plugins/BulkUpdatePlugin.ts +++ b/graphile/graphile-bulk-mutations/src/plugins/BulkUpdatePlugin.ts @@ -1,6 +1,7 @@ import '../augmentations'; import { sideEffectWithPgClient } from '@dataplan/pg'; +import { QuoteUtils } from '@pgsql/quotes'; import type { GraphileConfig } from 'graphile-config'; import type { GraphQLInputType, GraphQLOutputType } from 'graphql'; @@ -80,7 +81,9 @@ export const BulkUpdatePlugin: GraphileConfig.Plugin = { // Extract primary key columns for RETURNING clause const primaryUnique = resource.uniques.find((u: any) => u.isPrimary) ?? resource.uniques[0]; const pkColumns: string[] = primaryUnique.attributes; - const pkReturning = pkColumns.map((c) => `"${c}"`).join(', '); + const pkReturning = pkColumns + .map((c) => QuoteUtils.quoteIdentifier(c)) + .join(', '); const compiledFrom = sql.compile(resource.from).text; @@ -126,7 +129,9 @@ export const BulkUpdatePlugin: GraphileConfig.Plugin = { if (!attrName) continue; const sqlType = attrToSqlType[attrName]; values.push(val); - setClauses.push(`"${attrName}" = $${values.length}::${sqlType}`); + setClauses.push( + `${QuoteUtils.quoteIdentifier(attrName)} = $${values.length}::${sqlType}` + ); } if (setClauses.length === 0) { @@ -144,11 +149,13 @@ export const BulkUpdatePlugin: GraphileConfig.Plugin = { const sqlType = attrToSqlType[attrName]; if (spec === null) { - whereClauses.push(`"${attrName}" IS NULL`); + whereClauses.push(`${QuoteUtils.quoteIdentifier(attrName)} IS NULL`); } else if (spec !== undefined && typeof spec !== 'object') { // Simple equality (Condition type) values.push(spec); - whereClauses.push(`"${attrName}" = $${values.length}::${sqlType}`); + whereClauses.push( + `${QuoteUtils.quoteIdentifier(attrName)} = $${values.length}::${sqlType}` + ); } else if (spec && typeof spec === 'object') { // Operator-based (Filter type) for (const [op, val] of Object.entries(spec) as [string, any][]) { @@ -156,22 +163,22 @@ export const BulkUpdatePlugin: GraphileConfig.Plugin = { const paramRef = `$${values.length}::${sqlType}`; switch (op) { case 'equalTo': - whereClauses.push(`"${attrName}" = ${paramRef}`); + whereClauses.push(`${QuoteUtils.quoteIdentifier(attrName)} = ${paramRef}`); break; case 'notEqualTo': - whereClauses.push(`"${attrName}" != ${paramRef}`); + whereClauses.push(`${QuoteUtils.quoteIdentifier(attrName)} != ${paramRef}`); break; case 'greaterThan': - whereClauses.push(`"${attrName}" > ${paramRef}`); + whereClauses.push(`${QuoteUtils.quoteIdentifier(attrName)} > ${paramRef}`); break; case 'greaterThanOrEqualTo': - whereClauses.push(`"${attrName}" >= ${paramRef}`); + whereClauses.push(`${QuoteUtils.quoteIdentifier(attrName)} >= ${paramRef}`); break; case 'lessThan': - whereClauses.push(`"${attrName}" < ${paramRef}`); + whereClauses.push(`${QuoteUtils.quoteIdentifier(attrName)} < ${paramRef}`); break; case 'lessThanOrEqualTo': - whereClauses.push(`"${attrName}" <= ${paramRef}`); + whereClauses.push(`${QuoteUtils.quoteIdentifier(attrName)} <= ${paramRef}`); break; case 'in': if (Array.isArray(val)) { @@ -180,15 +187,17 @@ export const BulkUpdatePlugin: GraphileConfig.Plugin = { return `$${values.length}::${sqlType}`; }); values.pop(); - whereClauses.push(`"${attrName}" IN (${placeholders.join(', ')})`); + whereClauses.push( + `${QuoteUtils.quoteIdentifier(attrName)} IN (${placeholders.join(', ')})` + ); } break; case 'isNull': values.pop(); if (val) { - whereClauses.push(`"${attrName}" IS NULL`); + whereClauses.push(`${QuoteUtils.quoteIdentifier(attrName)} IS NULL`); } else { - whereClauses.push(`"${attrName}" IS NOT NULL`); + whereClauses.push(`${QuoteUtils.quoteIdentifier(attrName)} IS NOT NULL`); } break; default: @@ -222,7 +231,7 @@ export const BulkUpdatePlugin: GraphileConfig.Plugin = { const pkConditions = pkRows.map((pkRow, rowIdx) => { return pkColumns.map((col, colIdx) => { const paramIdx = rowIdx * pkColumns.length + colIdx + 1; - return `"${col}" = $${paramIdx}`; + return `${QuoteUtils.quoteIdentifier(col)} = $${paramIdx}`; }).join(' AND '); }); const selectWhere = pkConditions.map((c) => `(${c})`).join(' OR '); diff --git a/graphile/graphile-bulk-mutations/src/plugins/BulkUpsertPlugin.ts b/graphile/graphile-bulk-mutations/src/plugins/BulkUpsertPlugin.ts index 4b261c7562..30459ff9ba 100644 --- a/graphile/graphile-bulk-mutations/src/plugins/BulkUpsertPlugin.ts +++ b/graphile/graphile-bulk-mutations/src/plugins/BulkUpsertPlugin.ts @@ -1,6 +1,7 @@ import '../augmentations'; import { sideEffectWithPgClient } from '@dataplan/pg'; +import { QuoteUtils } from '@pgsql/quotes'; import type { GraphileConfig } from 'graphile-config'; import type { GraphQLInputType, GraphQLOutputType } from 'graphql'; @@ -193,7 +194,7 @@ export const BulkUpsertPlugin: GraphileConfig.Plugin = { const pkConditions = allPkRows.map((pkRow, rowIdx) => { return pkColumns.map((col, colIdx) => { const paramIdx = rowIdx * pkColumns.length + colIdx + 1; - return `"${col}" = $${paramIdx}`; + return `${QuoteUtils.quoteIdentifier(col)} = $${paramIdx}`; }).join(' AND '); }); const whereClause = pkConditions.map((c) => `(${c})`).join(' OR '); diff --git a/graphile/graphile-bulk-mutations/src/utils/sql-builder.ts b/graphile/graphile-bulk-mutations/src/utils/sql-builder.ts index b649aa3cf6..a1cb046d31 100644 --- a/graphile/graphile-bulk-mutations/src/utils/sql-builder.ts +++ b/graphile/graphile-bulk-mutations/src/utils/sql-builder.ts @@ -12,6 +12,8 @@ * See: https://github.com/pyramation/graphile-column-privileges-mutations */ +import { QuoteUtils } from '@pgsql/quotes'; + import { PG_MAX_PARAMS } from '../types'; export interface ColumnSpec { @@ -45,12 +47,12 @@ export function buildBulkInsertSQL( updateColumns?: string[]; } ): InsertBatch[] { - const colNames = columns.map((c) => `"${c.name}"`); + const colNames = columns.map((c) => QuoteUtils.quoteIdentifier(c.name)); const colsPerRow = columns.length; const maxRowsPerBatch = Math.floor(PG_MAX_PARAMS / colsPerRow); const returningClause = returningColumns.length > 0 - ? returningColumns.map((c) => `"${c}"`).join(', ') + ? returningColumns.map((c) => QuoteUtils.quoteIdentifier(c)).join(', ') : '*'; const batches: InsertBatch[] = []; @@ -79,7 +81,9 @@ export function buildBulkInsertSQL( if (onConflict) { if (onConflict.conflictColumns && onConflict.conflictColumns.length > 0) { - const colList = onConflict.conflictColumns.map((c) => `"${c}"`).join(', '); + const colList = onConflict.conflictColumns + .map((c) => QuoteUtils.quoteIdentifier(c)) + .join(', '); text += `\nON CONFLICT (${colList})`; } else { text += '\nON CONFLICT'; @@ -93,7 +97,10 @@ export function buildBulkInsertSQL( ? onConflict.updateColumns : columns.map((c) => c.name); const setClause = setCols - .map((c) => `"${c}" = EXCLUDED."${c}"`) + .map((c) => { + const identifier = QuoteUtils.quoteIdentifier(c); + return `${identifier} = EXCLUDED.${identifier}`; + }) .join(', '); text += ` DO UPDATE SET ${setClause}`; } @@ -130,7 +137,9 @@ export function buildBulkUpdateSQL( if (value === undefined) continue; values.push(value); - setClauses.push(`"${col.name}" = $${values.length}::${col.sqlType}`); + setClauses.push( + `${QuoteUtils.quoteIdentifier(col.name)} = $${values.length}::${col.sqlType}` + ); } if (setClauses.length === 0) { @@ -146,7 +155,7 @@ export function buildBulkUpdateSQL( values.push(...whereParams); const returningClause = returningColumns.length > 0 - ? returningColumns.map((c) => `"${c}"`).join(', ') + ? returningColumns.map((c) => QuoteUtils.quoteIdentifier(c)).join(', ') : '*'; const text = `UPDATE ${tableName}\nSET ${setClauses.join(', ')}\nWHERE ${renumberedWhere}\nRETURNING ${returningClause}`; @@ -167,7 +176,7 @@ export function buildBulkDeleteSQL( whereParams: unknown[] ): { text: string; values: unknown[] } { const returningClause = returningColumns.length > 0 - ? returningColumns.map((c) => `"${c}"`).join(', ') + ? returningColumns.map((c) => QuoteUtils.quoteIdentifier(c)).join(', ') : '*'; const text = `DELETE FROM ${tableName}\nWHERE ${whereClause}\nRETURNING ${returningClause}`; diff --git a/graphile/graphile-i18n/package.json b/graphile/graphile-i18n/package.json index 3d449c21fe..2813f35a22 100644 --- a/graphile/graphile-i18n/package.json +++ b/graphile/graphile-i18n/package.json @@ -29,6 +29,7 @@ "url": "https://github.com/constructive-io/constructive/issues" }, "dependencies": { + "@pgsql/quotes": "^18.2.4", "accept-language-parser": "^1.5.0", "graphile-plugin-utils": "workspace:^" }, diff --git a/graphile/graphile-i18n/src/__tests__/sql-qualification.test.ts b/graphile/graphile-i18n/src/__tests__/sql-qualification.test.ts new file mode 100644 index 0000000000..e7b676c5af --- /dev/null +++ b/graphile/graphile-i18n/src/__tests__/sql-qualification.test.ts @@ -0,0 +1,123 @@ +import sql from 'pg-sql2'; + +import { buildI18nLookupSql, resolveI18nTableInfo } from '../plugin'; + +describe('i18n SQL qualification', () => { + it('resolves exact physical resources and quotes hostile identifiers', () => { + const uuidCodec = { + name: 'uuid', + sqlType: sql.identifier('pg_catalog', 'uuid'), + }; + const textCodec = { + name: 'text', + sqlType: sql.identifier('pg_catalog', 'text'), + }; + const hostile = 'title" FROM secrets --'; + const baseCodec: any = { + name: 'articles', + attributes: { + id: { codec: uuidCodec }, + [hostile]: { codec: textCodec }, + }, + extensions: { tags: { i18n: 'article_translations' } }, + }; + const translationCodec: any = { + name: 'articleTranslations', + attributes: { + article_id: { codec: uuidCodec }, + lang_code: { codec: textCodec }, + [hostile]: { codec: textCodec }, + }, + extensions: { + pg: { + serviceName: 'tenant_service', + schemaName: 'tenant-app', + name: 'article_translations', + }, + }, + }; + const build: any = { + sql, + inflection: { camelCase: (value: string) => value }, + input: { + pgRegistry: { + pgResources: { + articles: { + codec: baseCodec, + parameters: null, + uniques: [{ isPrimary: true, attributes: ['id'] }], + extensions: { + pg: { + serviceName: 'tenant_service', + schemaName: 'tenant-app', + name: 'articles', + }, + }, + }, + translations: { codec: translationCodec, parameters: null }, + }, + }, + }, + }; + + const info = resolveI18nTableInfo(build, baseCodec, 'lang_code', ['text'])!; + const query = buildI18nLookupSql(info, 'lang_code'); + + expect(info.baseTable).toBe('articles'); + expect(query).toContain('FROM "tenant-app".articles b'); + expect(query).toContain('LEFT JOIN "tenant-app".article_translations v'); + expect(query).toContain('"title"" FROM secrets --"'); + expect(query).toContain('$1::"pg_catalog"."uuid"'); + }); + + it('rejects a same-named translation table from another schema', () => { + const uuidCodec = { + name: 'uuid', + sqlType: sql.identifier('pg_catalog', 'uuid'), + }; + const baseCodec: any = { + name: 'articles', + attributes: { id: { codec: uuidCodec } }, + extensions: { tags: { i18n: 'article_translations' } }, + }; + const build: any = { + sql, + inflection: { camelCase: (value: string) => value }, + input: { + pgRegistry: { + pgResources: { + articles: { + codec: baseCodec, + parameters: null, + uniques: [{ isPrimary: true, attributes: ['id'] }], + extensions: { + pg: { + serviceName: 'tenant_service', + schemaName: 'tenant_a', + name: 'articles', + }, + }, + }, + translations: { + parameters: null, + codec: { + attributes: {}, + extensions: { + pg: { + serviceName: 'tenant_service', + schemaName: 'tenant_b', + name: 'article_translations', + }, + }, + }, + }, + }, + }, + }, + }; + + expect(() => + resolveI18nTableInfo(build, baseCodec, 'lang_code', ['text']) + ).toThrow(/same-service, same-schema/); + }); +}); diff --git a/graphile/graphile-i18n/src/plugin.ts b/graphile/graphile-i18n/src/plugin.ts index 9226830a7a..4f31921501 100644 --- a/graphile/graphile-i18n/src/plugin.ts +++ b/graphile/graphile-i18n/src/plugin.ts @@ -22,6 +22,7 @@ import 'graphile-build-pg'; import type { PgCodecWithAttributes } from '@dataplan/pg'; import { TYPES } from '@dataplan/pg'; +import { QuoteUtils } from '@pgsql/quotes'; import { context as grafastContext, lambda, object } from 'grafast'; import type { GraphileConfig } from 'graphile-config'; import { withSystemLaneClient } from 'graphile-plugin-utils'; @@ -48,15 +49,6 @@ function hasI18nTag(codec: PgCodecWithAttributes): string | false { return false; } -function resolvePgTypeName(codec: any): string { - if (codec === TYPES.uuid) return 'uuid'; - if (codec === TYPES.int) return 'int4'; - if (codec === TYPES.bigint) return 'int8'; - if (codec === TYPES.text) return 'text'; - if (codec === TYPES.varchar) return 'text'; - return codec?.name ?? 'text'; -} - function resolveAttrPgType(codec: any): string { if (codec === TYPES.text) return 'text'; if (codec === TYPES.varchar) return 'text'; @@ -64,6 +56,186 @@ function resolveAttrPgType(codec: any): string { return codec?.name ?? 'text'; } +function resourceIdentity(resource: any, label: string): { + serviceName: string; + schemaName: string; + name: string; +} { + const pg = resource?.codec?.extensions?.pg ?? resource?.extensions?.pg; + if (!pg?.serviceName || !pg?.schemaName || !pg?.name) { + throw new Error(`[graphile-i18n] ${label} is missing exact service/schema/table metadata`); + } + return pg; +} + +function compilePgType(build: any, codec: any, label: string): string { + if (!codec?.sqlType || typeof build?.sql?.compile !== 'function') { + throw new Error(`[graphile-i18n] ${label} has no compilable PostgreSQL type`); + } + const compiled = build.sql.compile(codec.sqlType); + if (!compiled?.text || (compiled.values?.length ?? 0) !== 0) { + throw new Error(`[graphile-i18n] ${label} PostgreSQL type did not compile to a static identifier`); + } + return compiled.text; +} + +/** Resolve one @i18n tag exclusively against this exact build registry. */ +export function resolveI18nTableInfo( + build: any, + codec: PgCodecWithAttributes, + langCodeColumn: string, + allowedTypes: readonly string[] +): I18nTableInfo | null { + const translationTableName = hasI18nTag(codec); + if (!translationTableName) return null; + + const resources = Object.values(build.input?.pgRegistry?.pgResources ?? {}) as any[]; + const baseMatches = resources.filter( + (resource) => !resource?.parameters && resource?.codec === codec + ); + if (baseMatches.length !== 1) { + throw new Error( + `[graphile-i18n] @i18n codec '${codec.name}' must resolve exactly one base resource ` + + `(matches=${baseMatches.length})` + ); + } + const baseResource = baseMatches[0]; + const base = resourceIdentity(baseResource, 'base resource'); + + const primaryKeys = (baseResource.uniques as Array<{ + attributes: string[]; + isPrimary?: boolean; + }> | undefined)?.filter((unique) => unique.isPrimary) ?? []; + if (primaryKeys.length !== 1 || primaryKeys[0].attributes.length !== 1) { + throw new Error( + `[graphile-i18n] @i18n base '${base.schemaName}.${base.name}' requires one ` + + 'single-column primary key' + ); + } + const pkColumn = primaryKeys[0].attributes[0]; + const pkAttr = codec.attributes?.[pkColumn] as any; + if (!pkAttr) { + throw new Error( + `[graphile-i18n] Primary key '${pkColumn}' is missing from ` + + `'${base.schemaName}.${base.name}'` + ); + } + const pkType = compilePgType(build, pkAttr.codec, `${base.schemaName}.${base.name}.${pkColumn}`); + + const translationMatches = resources.filter((resource) => { + if (resource?.parameters || !resource?.codec?.attributes) return false; + const pg = resource.codec.extensions?.pg ?? resource.extensions?.pg; + return pg?.serviceName === base.serviceName && + pg?.schemaName === base.schemaName && + pg?.name === translationTableName; + }); + if (translationMatches.length !== 1) { + throw new Error( + `[graphile-i18n] @i18n on '${base.schemaName}.${base.name}' must resolve exactly ` + + `one same-service, same-schema '${translationTableName}' resource ` + + `(matches=${translationMatches.length})` + ); + } + + const translationResource = translationMatches[0]; + const translation = resourceIdentity(translationResource, 'translation resource'); + const translationCodec = translationResource.codec as PgCodecWithAttributes; + if (!translationCodec.attributes?.[langCodeColumn]) { + throw new Error( + `[graphile-i18n] Translation table '${translation.schemaName}.${translation.name}' ` + + `is missing language column '${langCodeColumn}'` + ); + } + + const conventionalFk = `${base.name}_id`; + const matchingFkColumns = Object.entries(translationCodec.attributes) + .filter(([attrName, attr]) => + attrName !== 'id' && + attrName !== langCodeColumn && + (attr as any).codec === pkAttr.codec + ) + .map(([attrName]) => attrName); + const fkColumn = matchingFkColumns.includes(conventionalFk) + ? conventionalFk + : matchingFkColumns.length === 1 + ? matchingFkColumns[0] + : null; + if (!fkColumn) { + throw new Error( + `[graphile-i18n] Translation table '${translation.schemaName}.${translation.name}' ` + + `has ambiguous or missing FK metadata for '${base.schemaName}.${base.name}'` + ); + } + + const fields: Record = {}; + for (const [attrName, attr] of Object.entries(translationCodec.attributes)) { + if (attrName === langCodeColumn || attrName === fkColumn) continue; + if (attrName === 'id' || attrName === 'created_at' || attrName === 'updated_at') continue; + + const pgType = resolveAttrPgType((attr as any).codec); + if (!allowedTypes.includes(pgType)) continue; + if (!codec.attributes?.[attrName]) { + throw new Error( + `[graphile-i18n] Translation field '${translation.schemaName}.${translation.name}.` + + `${attrName}' has no matching base field on '${base.schemaName}.${base.name}'` + ); + } + + const gqlName = build.inflection.camelCase(attrName); + fields[gqlName] = { + column: attrName, + type: pgType, + isNotNull: !!(attr as any).notNull, + }; + } + if (Object.keys(fields).length === 0) { + throw new Error( + `[graphile-i18n] Translation table '${translation.schemaName}.${translation.name}' ` + + 'has no eligible translatable fields' + ); + } + + return { + baseTable: base.name, + translationTable: translation.name, + schemaName: base.schemaName, + fkColumn, + pkColumn, + pkType, + fields, + }; +} + +export function buildI18nLookupSql( + info: I18nTableInfo, + langCodeColumn: string +): string { + const { + schemaName, + baseTable, + translationTable, + fkColumn, + pkColumn, + pkType, + fields, + } = info; + const qi = (name: string): string => QuoteUtils.quoteIdentifier(name); + const coalescedCols = Object.values(fields) + .map((field) => `coalesce(v.${qi(field.column)}, b.${qi(field.column)}) as ${qi(field.column)}`) + .join(', '); + const baseTableRef = QuoteUtils.quoteQualifiedIdentifier(schemaName, baseTable); + const translationTableRef = QuoteUtils.quoteQualifiedIdentifier(schemaName, translationTable); + + return `SELECT v.${qi(langCodeColumn)} AS "lang_code", ${coalescedCols} + FROM ${baseTableRef} b + LEFT JOIN ${translationTableRef} v + ON v.${qi(fkColumn)} = b.${qi(pkColumn)} + AND array_position($2::text[], v.${qi(langCodeColumn)}) IS NOT NULL + WHERE b.${qi(pkColumn)} = $1::${pkType} + ORDER BY array_position($2::text[], v.${qi(langCodeColumn)}) ASC NULLS LAST + LIMIT 1`; +} + // ─── Plugin Factory ────────────────────────────────────────────────────────── export function createI18nPlugin(options: I18nPluginOptions = {}): GraphileConfig.Plugin { @@ -92,113 +264,9 @@ export function createI18nPlugin(options: I18nPluginOptions = {}): GraphileConfi const c = codec as PgCodecWithAttributes; if (!c.attributes) continue; - const translationTableName = hasI18nTag(c); - if (!translationTableName) continue; - - // Get schema name from the codec's pg extensions - let schemaName = (c.extensions as any)?.pg?.schemaName ?? 'public'; - let pkColumn: string | null = null; - let pkType = 'text'; - for (const [, resource] of Object.entries(build.input.pgRegistry.pgResources)) { - const r = resource as any; - if (r.codec === c) { - // Try multiple sources for schema name - const rSchema = r.extensions?.pg?.schemaName ?? r.schemaName; - if (rSchema) schemaName = rSchema; - // Extract PK from the resource's uniques array - const uniques = r.uniques as Array<{ attributes: string[]; isPrimary?: boolean }> | undefined; - if (uniques) { - const pk = uniques.find((u: any) => u.isPrimary); - if (pk && pk.attributes.length === 1) { - pkColumn = pk.attributes[0]; - const pkAttr = c.attributes[pkColumn]; - if (pkAttr) { - pkType = resolvePgTypeName((pkAttr as any).codec); - } - } - } - break; - } - } - if (!pkColumn) continue; - - // Find the translation codec. The @i18n tag value is the SQL table name - // (e.g. 'posts_translations'), but PostGraphile inflects codec names - // to camelCase (e.g. 'postsTranslations'). Match via resource name. - let translationCodec: PgCodecWithAttributes | null = null; - for (const [, resource] of Object.entries(build.input.pgRegistry.pgResources)) { - const r = resource as any; - if (!r.codec?.attributes) continue; - // Match by the resource's SQL name (which preserves snake_case) - const sqlName = r.codec?.extensions?.pg?.name ?? r.name; - if (sqlName === translationTableName) { - translationCodec = r.codec as PgCodecWithAttributes; - break; - } - } - // Fallback: try matching the inflected codec name directly - if (!translationCodec) { - const inflectedName = build.inflection.camelCase(translationTableName); - for (const [, tCodec] of Object.entries(build.input.pgRegistry.pgCodecs)) { - const tc = tCodec as any; - if (!tc.attributes) continue; - if (tc.name === translationTableName || tc.name === inflectedName) { - translationCodec = tc; - break; - } - } - } - - if (!translationCodec) continue; - - // Find FK column on translation table — convention first, then type match - let fkColumn: string | null = null; - const conventionalFk = `${c.name}_id`; - if (translationCodec.attributes[conventionalFk]) { - fkColumn = conventionalFk; - } - if (!fkColumn) { - // Fallback: find a column with the same type as the PK, excluding - // common non-FK columns (id, lang_code) - for (const [attrName, attr] of Object.entries(translationCodec.attributes)) { - if (attrName === 'id' || attrName === langCodeColumn) continue; - const a = attr as any; - if (a.codec === (c.attributes[pkColumn] as any).codec) { - fkColumn = attrName; - break; - } - } - } - if (!fkColumn) continue; - - // Discover translatable fields - const fields: Record = {}; - for (const [attrName, attr] of Object.entries(translationCodec.attributes)) { - if (attrName === langCodeColumn || attrName === fkColumn) continue; - if (attrName === 'id' || attrName === 'created_at' || attrName === 'updated_at') continue; - - const pgType = resolveAttrPgType((attr as any).codec); - if (!allowedTypes.includes(pgType)) continue; - - const gqlName = build.inflection.camelCase(attrName); - fields[gqlName] = { - column: attrName, - type: pgType, - isNotNull: !!(attr as any).notNull, - }; - } - - if (Object.keys(fields).length === 0) continue; - - i18nRegistry[c.name] = { - baseTable: c.name, - translationTable: translationTableName, - schemaName, - fkColumn, - pkColumn, - pkType, - fields, - }; + if (!hasI18nTag(c)) continue; + const info = resolveI18nTableInfo(build, c, langCodeColumn, allowedTypes); + if (info) i18nRegistry[c.name] = info; } return _; @@ -234,21 +302,8 @@ export function createI18nPlugin(options: I18nPluginOptions = {}): GraphileConfi } const localeType = localeTypeCache[localeTypeName]; - const { schemaName, baseTable, translationTable, fkColumn, pkColumn, pkType, fields: i18nFields } = info; - - const coalescedCols = Object.values(i18nFields) - .map(f => `coalesce(v."${f.column}", b."${f.column}") as "${f.column}"`) - .join(', '); - - // Build the SQL query template - const sqlQuery = `SELECT v."${langCodeColumn}" AS "lang_code", ${coalescedCols} - FROM "${schemaName}"."${baseTable}" b - LEFT JOIN "${schemaName}"."${translationTable}" v - ON v."${fkColumn}" = b."${pkColumn}" - AND array_position($2::text[], v."${langCodeColumn}") IS NOT NULL - WHERE b."${pkColumn}" = $1::${pkType} - ORDER BY array_position($2::text[], v."${langCodeColumn}") ASC NULLS LAST - LIMIT 1`; + const { pkColumn, fields: i18nFields } = info; + const sqlQuery = buildI18nLookupSql(info, langCodeColumn); // Build column names list for mapping base values const baseColNames = Object.entries(i18nFields).map(([gqlName, f]) => ({ diff --git a/graphile/graphile-llm/package.json b/graphile/graphile-llm/package.json index abb7fd1d2f..2e0f9327ff 100644 --- a/graphile/graphile-llm/package.json +++ b/graphile/graphile-llm/package.json @@ -32,6 +32,7 @@ "@agentic-kit/ollama": "workspace:*", "@constructive-io/express-context": "workspace:^", "@constructive-io/llm-env": "workspace:^", + "@pgsql/quotes": "^18.2.4", "graphile-cache": "workspace:^" }, "peerDependencies": { diff --git a/graphile/graphile-llm/src/__tests__/rag-sql.test.ts b/graphile/graphile-llm/src/__tests__/rag-sql.test.ts new file mode 100644 index 0000000000..e5cc383bd6 --- /dev/null +++ b/graphile/graphile-llm/src/__tests__/rag-sql.test.ts @@ -0,0 +1,123 @@ +import { + buildChunkSearchSql, + discoverChunkTables, +} from '../plugins/rag-plugin'; +import type { ChunkTableInfo } from '../types'; + +const chunkTable = ( + overrides: Partial = {} +): ChunkTableInfo => ({ + parentCodecName: 'articles', + chunksSchema: 'tenant-a-app-public', + vectorSchema: 'tenant-a-extensions', + chunksTableName: 'article_chunks', + parentFkField: 'article_id', + parentPkField: 'id', + embeddingField: 'embedding', + contentField: 'content', + ...overrides, +}); + +describe('RAG SQL qualification', () => { + it('quotes tenant schemas and keeps parameter values separate', () => { + const query = buildChunkSearchSql(chunkTable(), '[1,0]', 7, 0.4); + expect(query.text).toContain('FROM "tenant-a-app-public".article_chunks'); + expect(query.text).toContain('$1::"tenant-a-extensions".vector'); + expect(query.text).toContain('OPERATOR("tenant-a-extensions".<=>)'); + expect(query.values).toEqual(['[1,0]', 0.4, 7]); + }); + + it('discovers the exact physical chunks resource and vector type schema', () => { + const vectorCodec = { + name: 'vector', + extensions: { + pg: { + serviceName: 'main', + schemaName: 'tenant-a-extensions', + name: 'vector', + }, + }, + }; + const chunkCodec = { + name: 'articleChunks', + attributes: { + article_id: {}, + content: {}, + embedding: { codec: vectorCodec }, + }, + extensions: { + pg: { + serviceName: 'main', + schemaName: 'tenant-a-app-public', + name: 'article_chunks', + }, + }, + }; + const tables = discoverChunkTables({ + input: { + pgRegistry: { + pgCodecs: { + articles: { + name: 'articles', + attributes: { id: {} }, + extensions: { + pg: { + serviceName: 'main', + schemaName: 'tenant-a-app-public', + name: 'articles', + }, + tags: { + hasChunks: { + chunksTable: 'article_chunks', + parentFk: 'article_id', + }, + }, + }, + }, + }, + pgResources: { articleChunks: { codec: chunkCodec } }, + }, + }, + resolvedPreset: { + pgServices: [{ name: 'main', schemas: ['tenant-a-app-public'] }], + }, + }); + expect(tables).toHaveLength(1); + expect(tables[0].chunksSchema).toBe('tenant-a-app-public'); + expect(tables[0].vectorSchema).toBe('tenant-a-extensions'); + }); + + it('rejects a chunks schema outside the exact service allowlist', () => { + expect(() => + discoverChunkTables({ + input: { + pgRegistry: { + pgCodecs: { + articles: { + name: 'articles', + attributes: { id: {} }, + extensions: { + pg: { + serviceName: 'main', + schemaName: 'tenant_a', + name: 'articles', + }, + tags: { + hasChunks: { + chunksSchema: 'tenant_b', + chunksTable: 'article_chunks', + }, + }, + }, + }, + }, + pgResources: {}, + }, + }, + resolvedPreset: { + pgServices: [{ name: 'main', schemas: ['tenant_a'] }], + }, + }) + ).toThrow(/outside service 'main'/); + }); +}); diff --git a/graphile/graphile-llm/src/plugins/rag-plugin.ts b/graphile/graphile-llm/src/plugins/rag-plugin.ts index 3c1a3e15cf..ad7438df7b 100644 --- a/graphile/graphile-llm/src/plugins/rag-plugin.ts +++ b/graphile/graphile-llm/src/plugins/rag-plugin.ts @@ -20,6 +20,7 @@ * 2. Falls back to error if not configured */ +import { QuoteUtils } from '@pgsql/quotes'; import { context as grafastContext, lambda, object } from 'grafast'; import type { GraphileConfig } from 'graphile-config'; import { extendSchema, gql } from 'graphile-utils'; @@ -76,6 +77,7 @@ function parseHasChunksTag(raw: any, codec: any): ChunkTableInfo | null { return { parentCodecName: codec.name || 'unknown', chunksSchema, + vectorSchema: '', chunksTableName: parsed.chunksTable, parentFkField: parsed.parentFk || 'parent_id', parentPkField: parsed.parentPk || 'id', @@ -84,10 +86,60 @@ function parseHasChunksTag(raw: any, codec: any): ChunkTableInfo | null { }; } +function requirePgIdentity(value: any, label: string): { + serviceName: string; + schemaName: string; + name: string; +} { + const pg = value?.extensions?.pg; + if (!pg?.serviceName || !pg?.schemaName || !pg?.name) { + throw new Error(`[graphile-llm] ${label} is missing exact service/schema/table metadata`); + } + return pg; +} + +function configuredSchemas(build: any, serviceName: string): ReadonlySet | null { + const services = build?.resolvedPreset?.pgServices; + if (!Array.isArray(services)) return null; + const matches = services.filter( + (service: any) => (service?.name ?? 'main') === serviceName + ); + if (matches.length !== 1) { + throw new Error( + `[graphile-llm] @hasChunks cannot resolve exact service '${serviceName}' ` + + `(matches=${matches.length})` + ); + } + const service = matches[0]; + const schemas = service?.schemas; + if (!Array.isArray(schemas) || schemas.length === 0) { + throw new Error( + `[graphile-llm] @hasChunks service '${serviceName}' has no configured schema allowlist` + ); + } + const dependencySchemas = service?.introspectionAllowedDependencySchemas; + if (dependencySchemas !== undefined && !Array.isArray(dependencySchemas)) { + throw new Error( + `[graphile-llm] @hasChunks service '${serviceName}' has an invalid dependency schema allowlist` + ); + } + return new Set([...schemas, ...(dependencySchemas ?? [])]); +} + +function requireField(codec: any, fieldName: string, label: string, table: string): any { + const field = codec?.attributes?.[fieldName]; + if (!field) { + throw new Error( + `[graphile-llm] @hasChunks ${label} '${fieldName}' does not exist on '${table}'` + ); + } + return field; +} + /** * Discover all chunk-aware tables from the pgRegistry. */ -function discoverChunkTables(build: any): ChunkTableInfo[] { +export function discoverChunkTables(build: any): ChunkTableInfo[] { const chunkTables: ChunkTableInfo[] = []; const pgRegistry = build.input?.pgRegistry ?? build.pgRegistry; if (!pgRegistry) return chunkTables; @@ -101,9 +153,67 @@ function discoverChunkTables(build: any): ChunkTableInfo[] { if (!tags?.hasChunks) continue; const info = parseHasChunksTag(tags.hasChunks, c); - if (info) { - chunkTables.push(info); + if (!info) { + throw new Error(`[graphile-llm] @hasChunks on '${c.name}' must be a valid JSON object`); + } + + const parent = requirePgIdentity(c, 'parent codec'); + if (!info.chunksSchema) { + throw new Error(`[graphile-llm] @hasChunks on '${parent.name}' has no chunks schema`); + } + const allowedSchemas = configuredSchemas(build, parent.serviceName); + if (allowedSchemas && !allowedSchemas.has(info.chunksSchema)) { + throw new Error( + `[graphile-llm] @hasChunks on '${parent.schemaName}.${parent.name}' references ` + + `schema '${info.chunksSchema}' outside service '${parent.serviceName}'` + ); + } + + const matches = Object.values(pgRegistry.pgResources ?? {}).filter((resource: any) => { + if (resource?.parameters || !resource?.codec?.attributes) return false; + const pg = resource.codec.extensions?.pg; + return pg?.serviceName === parent.serviceName && + pg?.schemaName === info.chunksSchema && + pg?.name === info.chunksTableName; + }) as any[]; + if (matches.length !== 1) { + throw new Error( + `[graphile-llm] @hasChunks on '${parent.schemaName}.${parent.name}' must resolve ` + + `exactly one '${info.chunksSchema}.${info.chunksTableName}' resource ` + + `(matches=${matches.length})` + ); } + + const chunksCodec = matches[0].codec; + const chunks = requirePgIdentity(chunksCodec, 'chunks codec'); + requireField(c, info.parentPkField, 'parentPk', `${parent.schemaName}.${parent.name}`); + requireField(chunksCodec, info.parentFkField, 'parentFk', `${chunks.schemaName}.${chunks.name}`); + requireField(chunksCodec, info.contentField, 'contentField', `${chunks.schemaName}.${chunks.name}`); + const embedding = requireField( + chunksCodec, + info.embeddingField, + 'embeddingField', + `${chunks.schemaName}.${chunks.name}` + ); + const vectorPg = embedding.codec?.extensions?.pg; + if ( + vectorPg?.name !== 'vector' || + vectorPg?.serviceName !== parent.serviceName || + !vectorPg?.schemaName + ) { + throw new Error( + `[graphile-llm] @hasChunks embedding '${chunks.schemaName}.${chunks.name}.` + + `${info.embeddingField}' is not bound to an exact vector type for service ` + + `'${parent.serviceName}'` + ); + } + + chunkTables.push({ + ...info, + chunksSchema: chunks.schemaName, + chunksTableName: chunks.name, + vectorSchema: vectorPg.schemaName, + }); } return chunkTables; @@ -112,37 +222,43 @@ function discoverChunkTables(build: any): ChunkTableInfo[] { /** * Build a SQL query string to search a chunks table for similar embeddings. */ -function buildChunkSearchSql( +export function buildChunkSearchSql( table: ChunkTableInfo, vectorString: string, limit: number, maxDistance: number | null ): { text: string; values: any[] } { - const schema = table.chunksSchema; - const qualifiedTable = schema - ? `"${schema}"."${table.chunksTableName}"` - : `"${table.chunksTableName}"`; - - const embeddingCol = `"${table.embeddingField}"`; - const contentCol = `"${table.contentField}"`; - const parentFkCol = `"${table.parentFkField}"`; + const qualifiedTable = QuoteUtils.quoteQualifiedIdentifier( + table.chunksSchema || null, + table.chunksTableName + ); + + const embeddingCol = QuoteUtils.quoteIdentifier(table.embeddingField); + const contentCol = QuoteUtils.quoteIdentifier(table.contentField); + const parentFkCol = QuoteUtils.quoteIdentifier(table.parentFkField); + if (!table.vectorSchema) { + throw new Error('[graphile-llm] RAG chunk table is missing an exact vector schema'); + } + const vectorType = QuoteUtils.quoteQualifiedIdentifier(table.vectorSchema, 'vector'); + const vectorDistanceOperator = `OPERATOR(${QuoteUtils.quoteIdentifier(table.vectorSchema)}.<=>)`; let text = ` SELECT ${contentCol} AS content, ${parentFkCol}::text AS parent_id, - (${embeddingCol} <=> $1::vector) AS distance + (${embeddingCol} ${vectorDistanceOperator} $1::${vectorType}) AS distance FROM ${qualifiedTable} `; const values: any[] = [vectorString]; if (maxDistance !== null) { - text += ` WHERE (${embeddingCol} <=> $1::vector) <= $2`; + text += ` WHERE (${embeddingCol} ${vectorDistanceOperator} $1::${vectorType}) <= $2`; values.push(maxDistance); } - text += ` ORDER BY ${embeddingCol} <=> $1::vector LIMIT $${values.length + 1}`; + text += ` ORDER BY ${embeddingCol} ${vectorDistanceOperator} $1::${vectorType} ` + + `LIMIT $${values.length + 1}`; values.push(limit); return { text, values }; @@ -174,7 +290,7 @@ export function createLlmRagPlugin( let embedder: EmbedderFunction | null = null; let chatCompleter: ChatFunction | null = null; - const schemaExtension = extendSchema((build) => { + const schemaExtension = extendSchema((_build) => { return { typeDefs: gql` """A source chunk retrieved during RAG context assembly.""" diff --git a/graphile/graphile-llm/src/types.ts b/graphile/graphile-llm/src/types.ts index c60d8e6bfe..81c32a84fe 100644 --- a/graphile/graphile-llm/src/types.ts +++ b/graphile/graphile-llm/src/types.ts @@ -174,6 +174,8 @@ export interface ChunkTableInfo { parentCodecName: string; /** Schema of the chunks table (or null for public/default) */ chunksSchema: string | null; + /** Exact schema containing the pgvector type and operators. */ + vectorSchema: string; /** Name of the chunks table */ chunksTableName: string; /** FK column on chunks table pointing to parent */ diff --git a/graphile/graphile-ltree/src/__tests__/schema-qualified-sql.test.ts b/graphile/graphile-ltree/src/__tests__/schema-qualified-sql.test.ts new file mode 100644 index 0000000000..fb9dee9821 --- /dev/null +++ b/graphile/graphile-ltree/src/__tests__/schema-qualified-sql.test.ts @@ -0,0 +1,233 @@ +import sql from 'pg-sql2'; + +import { createLtreeOperatorFactory } from '../plugins/connection-filter-operators'; +import { + resolveLtreeExtensionInfo, + type LtreeExtensionInfo, +} from '../plugins/detect-ltree'; +import { createFolderOperatorFactory } from '../plugins/folder-filter-operators'; +import { LtreeCodecPlugin } from '../plugins/ltree-codec'; + +const codec = (name: string, schemaName = 'extension_tools') => ({ + name, + extensions: { + pg: { + serviceName: 'tenant_service', + schemaName, + name, + }, + }, +}); + +const helperResource = ( + name: 'to_path' | 'to_query', + returnCodec: any, + schemaName = 'tenant_helpers' +) => ({ + name: `resource_${name}`, + parameters: [{ codec: codec('text', 'pg_catalog') }], + codec: returnCodec, + extensions: { + pg: { + serviceName: 'tenant_service', + schemaName, + name, + }, + }, +}); + +const registryBuild = ( + options: { + includeHelpers?: boolean; + ltreeCodec?: any; + lqueryCodec?: any; + resources?: Record; + } = {} +) => { + const ltreeCodec = options.ltreeCodec ?? codec('ltree'); + const lqueryCodec = options.lqueryCodec ?? codec('lquery'); + const includeHelpers = options.includeHelpers ?? false; + let pgResources = options.resources; + if (!pgResources) { + if (includeHelpers) { + pgResources = { + toPath: helperResource('to_path', ltreeCodec), + toQuery: helperResource('to_query', lqueryCodec), + }; + } else { + pgResources = {}; + } + } + return { + input: { + pgRegistry: { + pgCodecs: { ltree: ltreeCodec, lquery: lqueryCodec }, + pgResources, + }, + }, + }; +}; + +const resolveSql = ( + info: LtreeExtensionInfo, + factory: ReturnType, + operatorName: string, + input: string +) => { + const registration = factory({ pgLtreeExtensionInfo: info } as any).find( + (entry) => entry.operatorName === operatorName + )!; + const fragment = registration.spec.resolve!( + sql.identifier('path'), + sql.null, + input, + null, + { fieldName: 'path', operatorName } + ); + return sql.compile(fragment!); +}; + +describe('ltree extension identity', () => { + it('qualifies and annotates a native codec from gather introspection', async () => { + const gatherHook = (LtreeCodecPlugin as any).gather.hooks + .pgCodecs_findPgCodec; + const event: any = { + pgCodec: { + name: 'ltree', + sqlType: sql.fragment`ltree`, + extensions: undefined, + }, + pgType: { typname: 'ltree', typnamespace: '910', _id: '911' }, + serviceName: 'tenant_service', + }; + const originalCodec = event.pgCodec; + await gatherHook( + { + helpers: { + pgIntrospection: { + getNamespace: jest + .fn() + .mockResolvedValue({ nspname: 'extension_tools' }), + }, + }, + }, + event + ); + + expect(event.pgCodec).toBe(originalCodec); + expect(event.pgCodec.extensions).toMatchObject({ + oid: '911', + pg: { + serviceName: 'tenant_service', + schemaName: 'extension_tools', + name: 'ltree', + }, + }); + expect(sql.compile(event.pgCodec.sqlType).text).toBe( + '"extension_tools"."ltree"' + ); + }); + + it('derives codec and actual helper schemas from one service/build', () => { + const info = resolveLtreeExtensionInfo( + registryBuild({ includeHelpers: true }) + ); + expect(info).toMatchObject({ + serviceName: 'tenant_service', + schemaName: 'extension_tools', + helperSchemaName: 'tenant_helpers', + }); + }); + + it('fails closed on missing codec identity and incomplete helpers', () => { + expect(() => + resolveLtreeExtensionInfo( + registryBuild({ + ltreeCodec: { name: 'ltree', extensions: { pg: { name: 'ltree' } } }, + }) + ) + ).toThrow(/missing exact service\/schema metadata/); + + const ltreeCodec = codec('ltree'); + expect(() => + resolveLtreeExtensionInfo( + registryBuild({ + ltreeCodec, + resources: { + onlyPath: helperResource('to_path', ltreeCodec), + }, + }) + ) + ).toThrow(/incomplete or ambiguous/); + }); + + it('fails closed when ltree and lquery identities disagree', () => { + expect(() => + resolveLtreeExtensionInfo( + registryBuild({ + lqueryCodec: codec('lquery', 'other_extension_schema'), + }) + ) + ).toThrow(/does not match/); + }); +}); + +describe('ltree SQL qualification', () => { + it('qualifies helper functions and operators in the folder factory', () => { + const info = resolveLtreeExtensionInfo( + registryBuild({ includeHelpers: true }) + )!; + const within = resolveSql( + info, + createFolderOperatorFactory(), + 'within', + '/a/b' + ); + const glob = resolveSql( + info, + createFolderOperatorFactory(), + 'glob', + '/a/*' + ); + + expect(within.text).toContain('OPERATOR("extension_tools".<@)'); + expect(within.text).toContain('"tenant_helpers"."to_path"($1)'); + expect(glob.text).toContain('OPERATOR("extension_tools".~)'); + expect(glob.text).toContain('"tenant_helpers"."to_query"($1)'); + }); + + it('qualifies inline casts when helper functions are absent', () => { + const info = resolveLtreeExtensionInfo(registryBuild())!; + const within = resolveSql( + info, + createFolderOperatorFactory(), + 'within', + '/a/b' + ); + const glob = resolveSql( + info, + createFolderOperatorFactory(), + 'glob', + '/a/*' + ); + + expect(within.text).toContain('::"extension_tools"."ltree"'); + expect(within.text).toContain('OPERATOR("extension_tools".<@)'); + expect(glob.text).toContain('::"extension_tools"."lquery"'); + expect(glob.text).toContain('OPERATOR("extension_tools".~)'); + }); + + it('qualifies the deprecated duplicate operator factory too', () => { + const info = resolveLtreeExtensionInfo(registryBuild())!; + const result = resolveSql( + info, + createLtreeOperatorFactory() as ReturnType< + typeof createFolderOperatorFactory + >, + 'isDescendantOf', + '/a/b' + ); + expect(result.text).toContain('OPERATOR("extension_tools".@>)'); + expect(result.text).toContain('::"extension_tools"."ltree"'); + }); +}); diff --git a/graphile/graphile-ltree/src/plugins/connection-filter-operators.ts b/graphile/graphile-ltree/src/plugins/connection-filter-operators.ts index aa2b6114dd..7533ab95de 100644 --- a/graphile/graphile-ltree/src/plugins/connection-filter-operators.ts +++ b/graphile/graphile-ltree/src/plugins/connection-filter-operators.ts @@ -10,35 +10,13 @@ import type { import type { SQL } from 'pg-sql2'; import sql from 'pg-sql2'; +import type { LtreeExtensionInfo } from './detect-ltree'; import { LTREE_SCALAR_NAME } from './ltree-codec'; - -function hasLtreeHelpers(build: any): boolean { - const pgRegistry = build.input?.pgRegistry; - if (!pgRegistry) return false; - for (const resource of Object.values(pgRegistry.pgResources)) { - const r = resource as any; - if (r?.extensions?.pg?.schemaName === 'ltree_helpers') return true; - } - return false; -} - -function toPathExpr(value: SQL, useHelpers: boolean): SQL { - if (useHelpers) { - return sql.fragment`ltree_helpers.to_path(${value})`; - } - return sql.fragment`replace(ltrim(${value}, '/'), '/', '.')::ltree`; -} - -function toQueryExpr(value: SQL, useHelpers: boolean): SQL { - if (useHelpers) { - return sql.fragment`ltree_helpers.to_query(${value})`; - } - // Glob → lquery conversion: - // ** → * (0+ labels in lquery) - // * → *{1} (exactly 1 label) - // We use a placeholder to avoid ** being affected by the * → *{1} step. - return sql.fragment`replace(replace(replace(replace(ltrim(${value}, '/'), '**', '__DSTAR__'), '*', '*{1}'), '__DSTAR__', '*'), '/', '.')::lquery`; -} +import { + ltreeOperatorExpression, + ltreePathExpression, + ltreeQueryExpression, +} from './qualified-sql'; /** * Creates the ltree connection filter operator factory. @@ -55,10 +33,9 @@ function toQueryExpr(value: SQL, useHelpers: boolean): SQL { */ export function createLtreeOperatorFactory(): ConnectionFilterOperatorFactory { return (build) => { - const ltreeInfo = (build as any).pgLtreeExtensionInfo; + const ltreeInfo: LtreeExtensionInfo | undefined = + (build as any).pgLtreeExtensionInfo; if (!ltreeInfo) return []; - - const useHelpers = hasLtreeHelpers(build); const registrations: ConnectionFilterOperatorRegistration[] = []; registrations.push({ @@ -78,7 +55,12 @@ export function createLtreeOperatorFactory(): ConnectionFilterOperatorFactory { _details: { fieldName: string | null; operatorName: string } ) { const pathVal = sql.value(String(input)); - return sql.fragment`${sqlIdentifier} <@ ${toPathExpr(pathVal, useHelpers)}`; + return ltreeOperatorExpression( + '<@', + sqlIdentifier, + ltreePathExpression(pathVal, ltreeInfo), + ltreeInfo + ); } } satisfies ConnectionFilterOperatorSpec }); @@ -100,7 +82,12 @@ export function createLtreeOperatorFactory(): ConnectionFilterOperatorFactory { _details: { fieldName: string | null; operatorName: string } ) { const pathVal = sql.value(String(input)); - return sql.fragment`${sqlIdentifier} @> ${toPathExpr(pathVal, useHelpers)}`; + return ltreeOperatorExpression( + '@>', + sqlIdentifier, + ltreePathExpression(pathVal, ltreeInfo), + ltreeInfo + ); } } satisfies ConnectionFilterOperatorSpec }); @@ -122,7 +109,12 @@ export function createLtreeOperatorFactory(): ConnectionFilterOperatorFactory { _details: { fieldName: string | null; operatorName: string } ) { const globVal = sql.value(String(input)); - return sql.fragment`${sqlIdentifier} ~ ${toQueryExpr(globVal, useHelpers)}`; + return ltreeOperatorExpression( + '~', + sqlIdentifier, + ltreeQueryExpression(globVal, ltreeInfo), + ltreeInfo + ); } } satisfies ConnectionFilterOperatorSpec }); diff --git a/graphile/graphile-ltree/src/plugins/detect-ltree.ts b/graphile/graphile-ltree/src/plugins/detect-ltree.ts index 4dd689f473..35bbd2f79b 100644 --- a/graphile/graphile-ltree/src/plugins/detect-ltree.ts +++ b/graphile/graphile-ltree/src/plugins/detect-ltree.ts @@ -5,8 +5,12 @@ import type { PgCodec } from '@dataplan/pg'; import type { GraphileConfig } from 'graphile-config'; export interface LtreeExtensionInfo { + serviceName: string; + schemaName: string; ltreeCodec: PgCodec; lqueryCodec: PgCodec | null; + /** Exact schema containing both validated helper functions, when present. */ + helperSchemaName: string | null; } function isLtreeCodec(codec: any): boolean { @@ -23,6 +27,114 @@ function isLqueryCodec(codec: any): boolean { ); } +function codecIdentity(codec: any, typeName: string): { + serviceName: string; + schemaName: string; +} { + const pg = codec?.extensions?.pg; + if (!pg?.serviceName || !pg?.schemaName) { + throw new Error( + `[graphile-ltree] ${typeName} codec is missing exact service/schema metadata` + ); + } + return { serviceName: pg.serviceName, schemaName: pg.schemaName }; +} + +function helperSchemaName(pgRegistry: any, serviceName: string): string | null { + const matches: Record<'to_path' | 'to_query', any[]> = { + to_path: [], + to_query: [], + }; + + for (const resource of Object.values(pgRegistry.pgResources ?? {}) as any[]) { + if (!Array.isArray(resource?.parameters)) continue; + const pg = resource?.extensions?.pg; + const rawFunctionName = pg?.name ?? resource?.name; + if (rawFunctionName !== 'to_path' && rawFunctionName !== 'to_query') continue; + const functionName: 'to_path' | 'to_query' = rawFunctionName; + + const returnMatches = functionName === 'to_path' + ? isLtreeCodec(resource.codec) + : isLqueryCodec(resource.codec); + const parameter = resource.parameters[0]; + const parameterName = parameter?.codec?.extensions?.pg?.name ?? parameter?.codec?.name; + const signatureMatches = + returnMatches && + resource.parameters.length === 1 && + (parameterName === 'text' || parameterName === 'varchar' || parameterName === 'bpchar'); + if (!signatureMatches) continue; + + if (!pg?.serviceName || !pg?.schemaName) { + throw new Error( + `[graphile-ltree] ${functionName} helper is missing exact service/schema metadata` + ); + } + if (pg.serviceName !== serviceName) continue; + matches[functionName].push(resource); + } + + const pathMatches = matches.to_path; + const queryMatches = matches.to_query; + if (pathMatches.length === 0 && queryMatches.length === 0) return null; + if (pathMatches.length !== 1 || queryMatches.length !== 1) { + throw new Error( + `[graphile-ltree] Helper functions for service '${serviceName}' are incomplete ` + + `or ambiguous (to_path=${pathMatches.length}, to_query=${queryMatches.length})` + ); + } + + const pathSchema = pathMatches[0].extensions.pg.schemaName; + const querySchema = queryMatches[0].extensions.pg.schemaName; + if (pathSchema !== querySchema) { + throw new Error( + `[graphile-ltree] Helper functions for service '${serviceName}' resolve to ` + + `different schemas ('${pathSchema}', '${querySchema}')` + ); + } + return pathSchema; +} + +/** Resolve one unambiguous ltree identity from this exact build registry. */ +export function resolveLtreeExtensionInfo(build: any): LtreeExtensionInfo | undefined { + const pgRegistry = build.input?.pgRegistry; + if (!pgRegistry) return undefined; + + const ltreeCodecs = Object.values(pgRegistry.pgCodecs).filter(isLtreeCodec) as PgCodec[]; + const lqueryCodecs = Object.values(pgRegistry.pgCodecs).filter(isLqueryCodec) as PgCodec[]; + if (ltreeCodecs.length === 0) return undefined; + if (ltreeCodecs.length !== 1) { + throw new Error( + `[graphile-ltree] Expected one ltree codec per build, found ${ltreeCodecs.length}` + ); + } + + const ltreeCodec = ltreeCodecs[0]; + const identity = codecIdentity(ltreeCodec, 'ltree'); + const matchingLquery = lqueryCodecs.filter((codec) => { + const candidate = codecIdentity(codec, 'lquery'); + return candidate.serviceName === identity.serviceName && + candidate.schemaName === identity.schemaName; + }); + if (lqueryCodecs.length > 0 && matchingLquery.length !== lqueryCodecs.length) { + throw new Error( + '[graphile-ltree] lquery codec service/schema does not match the ltree codec' + ); + } + if (matchingLquery.length > 1) { + throw new Error( + `[graphile-ltree] Expected at most one matching lquery codec, found ` + + `${matchingLquery.length}` + ); + } + + return { + ...identity, + ltreeCodec, + lqueryCodec: matchingLquery[0] ?? null, + helperSchemaName: helperSchemaName(pgRegistry, identity.serviceName), + }; +} + /** * LtreeExtensionDetectionPlugin * @@ -40,30 +152,8 @@ export const LtreeExtensionDetectionPlugin: GraphileConfig.Plugin = { schema: { hooks: { build(build) { - const pgRegistry = build.input?.pgRegistry; - if (!pgRegistry) { - return build; - } - - let ltreeCodec: PgCodec | null = null; - let lqueryCodec: PgCodec | null = null; - - for (const codec of Object.values(pgRegistry.pgCodecs)) { - if (isLtreeCodec(codec)) { - ltreeCodec = codec; - } else if (isLqueryCodec(codec)) { - lqueryCodec = codec; - } - } - - if (!ltreeCodec) { - return build; - } - - const ltreeInfo: LtreeExtensionInfo = { - ltreeCodec, - lqueryCodec - }; + const ltreeInfo = resolveLtreeExtensionInfo(build); + if (!ltreeInfo) return build; return build.extend( build, diff --git a/graphile/graphile-ltree/src/plugins/folder-filter-operators.ts b/graphile/graphile-ltree/src/plugins/folder-filter-operators.ts index 88f39c3c83..0b1a106688 100644 --- a/graphile/graphile-ltree/src/plugins/folder-filter-operators.ts +++ b/graphile/graphile-ltree/src/plugins/folder-filter-operators.ts @@ -10,31 +10,13 @@ import type { import type { SQL } from 'pg-sql2'; import sql from 'pg-sql2'; +import type { LtreeExtensionInfo } from './detect-ltree'; import { LTREE_SCALAR_NAME } from './ltree-codec'; - -function hasLtreeHelpers(build: any): boolean { - const pgRegistry = build.input?.pgRegistry; - if (!pgRegistry) return false; - for (const resource of Object.values(pgRegistry.pgResources)) { - const r = resource as any; - if (r?.extensions?.pg?.schemaName === 'ltree_helpers') return true; - } - return false; -} - -function slashToLtree(value: SQL, useHelpers: boolean): SQL { - if (useHelpers) { - return sql.fragment`ltree_helpers.to_path(${value})`; - } - return sql.fragment`replace(ltrim(${value}, '/'), '/', '.')::ltree`; -} - -function slashGlobToLquery(value: SQL, useHelpers: boolean): SQL { - if (useHelpers) { - return sql.fragment`ltree_helpers.to_query(${value})`; - } - return sql.fragment`replace(replace(replace(replace(ltrim(${value}, '/'), '**', '__DSTAR__'), '*', '*{1}'), '__DSTAR__', '*'), '/', '.')::lquery`; -} +import { + ltreeOperatorExpression, + ltreePathExpression, + ltreeQueryExpression, +} from './qualified-sql'; /** * Creates folder-oriented connection filter operators for the LTree scalar. @@ -56,10 +38,9 @@ function slashGlobToLquery(value: SQL, useHelpers: boolean): SQL { */ export function createFolderOperatorFactory(): ConnectionFilterOperatorFactory { return (build) => { - const ltreeInfo = (build as any).pgLtreeExtensionInfo; + const ltreeInfo: LtreeExtensionInfo | undefined = + (build as any).pgLtreeExtensionInfo; if (!ltreeInfo) return []; - - const useHelpers = hasLtreeHelpers(build); const registrations: ConnectionFilterOperatorRegistration[] = []; registrations.push({ @@ -79,7 +60,12 @@ export function createFolderOperatorFactory(): ConnectionFilterOperatorFactory { _details: { fieldName: string | null; operatorName: string } ) { const pathVal = sql.value(String(input)); - return sql.fragment`${sqlIdentifier} <@ ${slashToLtree(pathVal, useHelpers)}`; + return ltreeOperatorExpression( + '<@', + sqlIdentifier, + ltreePathExpression(pathVal, ltreeInfo), + ltreeInfo + ); } } satisfies ConnectionFilterOperatorSpec }); @@ -101,7 +87,12 @@ export function createFolderOperatorFactory(): ConnectionFilterOperatorFactory { _details: { fieldName: string | null; operatorName: string } ) { const pathVal = sql.value(String(input)); - return sql.fragment`${sqlIdentifier} @> ${slashToLtree(pathVal, useHelpers)}`; + return ltreeOperatorExpression( + '@>', + sqlIdentifier, + ltreePathExpression(pathVal, ltreeInfo), + ltreeInfo + ); } } satisfies ConnectionFilterOperatorSpec }); @@ -124,7 +115,12 @@ export function createFolderOperatorFactory(): ConnectionFilterOperatorFactory { _details: { fieldName: string | null; operatorName: string } ) { const globVal = sql.value(String(input)); - return sql.fragment`${sqlIdentifier} ~ ${slashGlobToLquery(globVal, useHelpers)}`; + return ltreeOperatorExpression( + '~', + sqlIdentifier, + ltreeQueryExpression(globVal, ltreeInfo), + ltreeInfo + ); } } satisfies ConnectionFilterOperatorSpec }); diff --git a/graphile/graphile-ltree/src/plugins/ltree-codec.ts b/graphile/graphile-ltree/src/plugins/ltree-codec.ts index db39dcd0a9..ce800bc3ac 100644 --- a/graphile/graphile-ltree/src/plugins/ltree-codec.ts +++ b/graphile/graphile-ltree/src/plugins/ltree-codec.ts @@ -56,8 +56,6 @@ export const LtreeCodecPlugin: GraphileConfig.Plugin = { gather: { hooks: { async pgCodecs_findPgCodec(info, event) { - if (event.pgCodec) return; - const { pgType: type, serviceName } = event; const isLtree = type.typname === 'ltree'; @@ -70,7 +68,39 @@ export const LtreeCodecPlugin: GraphileConfig.Plugin = { serviceName, type.typnamespace ); - const schemaName = ns?.nspname || 'pg_catalog'; + if (!ns?.nspname) { + throw new Error( + `[graphile-ltree] Cannot resolve namespace for ${type.typname} ` + + `codec in service '${serviceName}'` + ); + } + const schemaName = ns.nspname; + + if (event.pgCodec) { + const existingPg = event.pgCodec.extensions?.pg; + if ( + (existingPg?.serviceName && existingPg.serviceName !== serviceName) || + (existingPg?.schemaName && existingPg.schemaName !== schemaName) + ) { + throw new Error( + `[graphile-ltree] Existing ${type.typname} codec identity conflicts with ` + + `introspection for service '${serviceName}'` + ); + } + const existingCodec = event.pgCodec as any; + existingCodec.sqlType = sql.identifier(schemaName, type.typname); + existingCodec.extensions = { + ...existingCodec.extensions, + oid: type._id, + pg: { + ...existingPg, + serviceName, + schemaName, + name: type.typname, + }, + }; + return; + } event.pgCodec = { name: type.typname, diff --git a/graphile/graphile-ltree/src/plugins/qualified-sql.ts b/graphile/graphile-ltree/src/plugins/qualified-sql.ts new file mode 100644 index 0000000000..ff383fb238 --- /dev/null +++ b/graphile/graphile-ltree/src/plugins/qualified-sql.ts @@ -0,0 +1,41 @@ +import type { SQL } from 'pg-sql2'; +import sql from 'pg-sql2'; + +import type { LtreeExtensionInfo } from './detect-ltree'; + +export function ltreePathExpression(value: SQL, info: LtreeExtensionInfo): SQL { + if (info.helperSchemaName) { + const toPath = sql.identifier(info.helperSchemaName, 'to_path'); + return sql.fragment`${toPath}(${value})`; + } + const ltreeType = sql.identifier(info.schemaName, 'ltree'); + return sql.fragment`replace(ltrim(${value}, '/'), '/', '.')::${ltreeType}`; +} + +export function ltreeQueryExpression( + value: SQL, + info: LtreeExtensionInfo +): SQL { + if (info.helperSchemaName) { + const toQuery = sql.identifier(info.helperSchemaName, 'to_query'); + return sql.fragment`${toQuery}(${value})`; + } + const lqueryType = sql.identifier(info.schemaName, 'lquery'); + return sql.fragment`replace(replace(replace(replace(ltrim(${value}, '/'), '**', '__DSTAR__'), '*', '*{1}'), '__DSTAR__', '*'), '/', '.')::${lqueryType}`; +} + +export function ltreeOperatorExpression( + operator: '<@' | '@>' | '~', + left: SQL, + right: SQL, + info: LtreeExtensionInfo +): SQL { + const schema = sql.identifier(info.schemaName); + if (operator === '<@') { + return sql.fragment`${left} OPERATOR(${schema}.<@) ${right}`; + } + if (operator === '@>') { + return sql.fragment`${left} OPERATOR(${schema}.@>) ${right}`; + } + return sql.fragment`${left} OPERATOR(${schema}.~) ${right}`; +} diff --git a/graphile/graphile-postgis/__tests__/codec.test.ts b/graphile/graphile-postgis/__tests__/codec.test.ts index 4f1e19e145..bd94c15c93 100644 --- a/graphile/graphile-postgis/__tests__/codec.test.ts +++ b/graphile/graphile-postgis/__tests__/codec.test.ts @@ -1,4 +1,5 @@ import type { PgCodec } from '@dataplan/pg'; +import sql from 'pg-sql2'; import { GisSubtype } from '../src/constants'; import { PostgisCodecPlugin } from '../src/plugins/codec'; @@ -27,16 +28,33 @@ describe('PostgisCodecPlugin', () => { const gatherHook = (PostgisCodecPlugin as { gather: { hooks: { pgCodecs_findPgCodec: Function } } }) .gather.hooks.pgCodecs_findPgCodec; - it('should skip if pgCodec is already set', async () => { - const info = { helpers: { pgIntrospection: { getNamespace: jest.fn() } } }; - const event = { pgCodec: { name: 'existing' }, pgType: { typname: 'geometry' }, serviceName: 'main' }; + it('should bind exact identity when a native pgCodec is already set', async () => { + const info = { + helpers: { + pgIntrospection: { + getNamespace: jest.fn().mockResolvedValue({ _id: '123', nspname: 'postgis_ext' }) + } + } + }; + const event = { + pgCodec: { name: 'geometry' } as PgCodec, + pgType: { typname: 'geometry', typnamespace: '123', _id: '456' }, + serviceName: 'main' + }; + const originalCodec = event.pgCodec; await gatherHook(info, event); - // Should not have called getNamespace since pgCodec was already set - expect(info.helpers.pgIntrospection.getNamespace).not.toHaveBeenCalled(); + expect(event.pgCodec).toBe(originalCodec); + expect(info.helpers.pgIntrospection.getNamespace).toHaveBeenCalledWith('main', '123'); + expect(event.pgCodec.extensions?.pg).toEqual({ + serviceName: 'main', + schemaName: 'postgis_ext', + name: 'geometry' + }); + expect(sql.compile(event.pgCodec.sqlType!).text).toBe('"postgis_ext"."geometry"'); }); - it('should skip if namespace is not found', async () => { + it('should fail closed if namespace is not found', async () => { const info = { helpers: { pgIntrospection: { getNamespace: jest.fn().mockResolvedValue(null) } } }; @@ -46,8 +64,9 @@ describe('PostgisCodecPlugin', () => { serviceName: 'main' }; - await gatherHook(info, event); - expect(event.pgCodec).toBeNull(); + await expect(gatherHook(info, event)).rejects.toThrow( + /Cannot resolve namespace for geometry codec/ + ); }); it('should create geometry codec when type is geometry', async () => { diff --git a/graphile/graphile-postgis/__tests__/connection-filter-operators.test.ts b/graphile/graphile-postgis/__tests__/connection-filter-operators.test.ts index 0fe7ed1388..ccb1aba3be 100644 --- a/graphile/graphile-postgis/__tests__/connection-filter-operators.test.ts +++ b/graphile/graphile-postgis/__tests__/connection-filter-operators.test.ts @@ -343,31 +343,31 @@ describe('PostGIS operator factory (createPostgisOperatorFactory)', () => { it('generates correct SQL for = operator', () => { expect(runOp('exactlyEquals').text).toBe( - '"col" = "public"."st_geomfromgeojson"($1::text)' + '"col" OPERATOR("public".=) "public"."st_geomfromgeojson"($1::text)' ); }); it('generates correct SQL for && operator', () => { expect(runOp('bboxIntersects2D').text).toBe( - '"col" && "public"."st_geomfromgeojson"($1::text)' + '"col" OPERATOR("public".&&) "public"."st_geomfromgeojson"($1::text)' ); }); it('generates correct SQL for ~ operator', () => { expect(runOp('bboxContains').text).toBe( - '"col" ~ "public"."st_geomfromgeojson"($1::text)' + '"col" OPERATOR("public".~) "public"."st_geomfromgeojson"($1::text)' ); }); it('generates correct SQL for ~= operator', () => { expect(runOp('bboxEquals').text).toBe( - '"col" ~= "public"."st_geomfromgeojson"($1::text)' + '"col" OPERATOR("public".~=) "public"."st_geomfromgeojson"($1::text)' ); }); it('generates correct SQL for &&& operator', () => { expect(runOp('bboxIntersectsND').text).toBe( - '"col" &&& "public"."st_geomfromgeojson"($1::text)' + '"col" OPERATOR("public".&&&) "public"."st_geomfromgeojson"($1::text)' ); }); }); diff --git a/graphile/graphile-postgis/__tests__/detect-extension.test.ts b/graphile/graphile-postgis/__tests__/detect-extension.test.ts index cd29d9b90c..9416012dbd 100644 --- a/graphile/graphile-postgis/__tests__/detect-extension.test.ts +++ b/graphile/graphile-postgis/__tests__/detect-extension.test.ts @@ -40,7 +40,7 @@ describe('PostgisExtensionDetectionPlugin', () => { it('should detect PostGIS with only geometry codec (no geography)', () => { const geometryCodec = { name: 'geometry', - extensions: { pg: { name: 'geometry', schemaName: 'public' } } + extensions: { pg: { name: 'geometry', schemaName: 'public', serviceName: 'main' } } }; const build = { input: { @@ -57,17 +57,18 @@ describe('PostgisExtensionDetectionPlugin', () => { expect(result.pgGISExtensionInfo).toBeDefined(); expect(result.pgGISExtensionInfo.geometryCodec).toBe(geometryCodec); expect(result.pgGISExtensionInfo.geographyCodec).toBeNull(); + expect(result.pgGISExtensionInfo.serviceName).toBe('main'); expect(result.pgGISExtensionInfo.schemaName).toBe('public'); }); it('should detect PostGIS when both geometry and geography codecs exist', () => { const geometryCodec = { name: 'geometry', - extensions: { pg: { name: 'geometry', schemaName: 'public' } } + extensions: { pg: { name: 'geometry', schemaName: 'public', serviceName: 'main' } } }; const geographyCodec = { name: 'geography', - extensions: { pg: { name: 'geography', schemaName: 'public' } } + extensions: { pg: { name: 'geography', schemaName: 'public', serviceName: 'main' } } }; const build = { @@ -90,11 +91,11 @@ describe('PostgisExtensionDetectionPlugin', () => { it('should detect custom schema for PostGIS installation', () => { const geometryCodec = { name: 'geometry', - extensions: { pg: { name: 'geometry', schemaName: 'postgis' } } + extensions: { pg: { name: 'geometry', schemaName: 'postgis', serviceName: 'main' } } }; const geographyCodec = { name: 'geography', - extensions: { pg: { name: 'geography', schemaName: 'postgis' } } + extensions: { pg: { name: 'geography', schemaName: 'postgis', serviceName: 'main' } } }; const build = { @@ -113,11 +114,11 @@ describe('PostgisExtensionDetectionPlugin', () => { it('should skip codecs without pg extensions', () => { const geometryCodec = { name: 'geometry', - extensions: { pg: { name: 'geometry', schemaName: 'public' } } + extensions: { pg: { name: 'geometry', schemaName: 'public', serviceName: 'main' } } }; const geographyCodec = { name: 'geography', - extensions: { pg: { name: 'geography', schemaName: 'public' } } + extensions: { pg: { name: 'geography', schemaName: 'public', serviceName: 'main' } } }; const otherCodec = { name: 'custom', @@ -140,5 +141,44 @@ describe('PostgisExtensionDetectionPlugin', () => { const result = buildHook(build); expect(result.pgGISExtensionInfo).toBeDefined(); }); + + it('fails closed when codec identity is missing or inconsistent', () => { + const extend = (base: any, ext: any) => ({ ...base, ...ext }); + expect(() => buildHook({ + input: { + pgRegistry: { + pgCodecs: { + geometry: { + name: 'geometry', + extensions: { pg: { name: 'geometry', schemaName: 'postgis' } } + } + } + } + }, + extend + })).toThrow(/missing exact service\/schema metadata/); + + expect(() => buildHook({ + input: { + pgRegistry: { + pgCodecs: { + geometry: { + name: 'geometry', + extensions: { + pg: { name: 'geometry', schemaName: 'postgis_a', serviceName: 'main' } + } + }, + geography: { + name: 'geography', + extensions: { + pg: { name: 'geography', schemaName: 'postgis_b', serviceName: 'main' } + } + } + } + } + }, + extend + })).toThrow(/different service\/schema identities/); + }); }); }); diff --git a/graphile/graphile-postgis/__tests__/spatial-relations.test.ts b/graphile/graphile-postgis/__tests__/spatial-relations.test.ts index 8e29ba7d7b..441337e28f 100644 --- a/graphile/graphile-postgis/__tests__/spatial-relations.test.ts +++ b/graphile/graphile-postgis/__tests__/spatial-relations.test.ts @@ -1,6 +1,7 @@ import sql from 'pg-sql2'; import { + buildSpatialJoinFragment, collectSpatialRelations, OPERATOR_REGISTRY, parseSpatialRelationTag, @@ -130,6 +131,23 @@ describe('OPERATOR_REGISTRY', () => { } } }); + + it('schema-qualifies the PostGIS infix operator in relation SQL', () => { + const fragment = buildSpatialJoinFragment( + { + ownerAttributeName: 'location', + targetAttributeName: 'geom', + operator: OPERATOR_REGISTRY.st_bbox_intersects, + } as any, + 'postgis_ext', + sql.identifier('owner'), + sql.identifier('target'), + null + ); + expect(sql.compile(fragment).text).toBe( + '"owner"."location" OPERATOR("postgis_ext".&&) "target"."geom"' + ); + }); }); // --------------------------------------------------------------------------- diff --git a/graphile/graphile-postgis/src/plugins/codec.ts b/graphile/graphile-postgis/src/plugins/codec.ts index 59dfde1679..cb9b566b87 100644 --- a/graphile/graphile-postgis/src/plugins/codec.ts +++ b/graphile/graphile-postgis/src/plugins/codec.ts @@ -199,12 +199,12 @@ export const PostgisCodecPlugin: GraphileConfig.Plugin = { gather: { hooks: { async pgCodecs_findPgCodec(info, event) { - if (event.pgCodec) { + const { pgType: type, serviceName } = event; + + if (type.typname !== 'geometry' && type.typname !== 'geography') { return; } - const { pgType: type, serviceName } = event; - // Find the namespace for this type by its OID const typeNamespace = await info.helpers.pgIntrospection.getNamespace( serviceName, @@ -212,6 +212,35 @@ export const PostgisCodecPlugin: GraphileConfig.Plugin = { ); if (!typeNamespace) { + throw new Error( + `[graphile-postgis] Cannot resolve namespace for ${type.typname} ` + + `codec in service '${serviceName}'` + ); + } + + if (event.pgCodec) { + const existingPg = event.pgCodec.extensions?.pg; + if ( + (existingPg?.serviceName && existingPg.serviceName !== serviceName) || + (existingPg?.schemaName && existingPg.schemaName !== typeNamespace.nspname) + ) { + throw new Error( + `[graphile-postgis] Existing ${type.typname} codec identity conflicts ` + + `with introspection for service '${serviceName}'` + ); + } + const existingCodec = event.pgCodec as any; + existingCodec.sqlType = sql.identifier(typeNamespace.nspname, type.typname); + existingCodec.extensions = { + ...existingCodec.extensions, + oid: type._id, + pg: { + ...existingPg, + serviceName, + schemaName: typeNamespace.nspname, + name: type.typname, + }, + }; return; } diff --git a/graphile/graphile-postgis/src/plugins/connection-filter-operators.ts b/graphile/graphile-postgis/src/plugins/connection-filter-operators.ts index 5431c81fc3..d94f18f680 100644 --- a/graphile/graphile-postgis/src/plugins/connection-filter-operators.ts +++ b/graphile/graphile-postgis/src/plugins/connection-filter-operators.ts @@ -18,21 +18,22 @@ import type { PostgisExtensionInfo } from './detect-extension'; * Builds an infix operator SQL fragment from a validated operator string. * Uses explicit template literals for each operator to avoid sql.raw. */ -function buildOperatorExpr(op: string, i: SQL, v: SQL): SQL { +function buildOperatorExpr(schemaName: string, op: string, i: SQL, v: SQL): SQL { + const schema = sql.identifier(schemaName); switch (op) { - case '=': return sql.fragment`${i} = ${v}`; - case '&&': return sql.fragment`${i} && ${v}`; - case '&&&': return sql.fragment`${i} &&& ${v}`; - case '&<': return sql.fragment`${i} &< ${v}`; - case '&<|': return sql.fragment`${i} &<| ${v}`; - case '&>': return sql.fragment`${i} &> ${v}`; - case '|&>': return sql.fragment`${i} |&> ${v}`; - case '<<': return sql.fragment`${i} << ${v}`; - case '<<|': return sql.fragment`${i} <<| ${v}`; - case '>>': return sql.fragment`${i} >> ${v}`; - case '|>>': return sql.fragment`${i} |>> ${v}`; - case '~': return sql.fragment`${i} ~ ${v}`; - case '~=': return sql.fragment`${i} ~= ${v}`; + case '=': return sql.fragment`${i} OPERATOR(${schema}.=) ${v}`; + case '&&': return sql.fragment`${i} OPERATOR(${schema}.&&) ${v}`; + case '&&&': return sql.fragment`${i} OPERATOR(${schema}.&&&) ${v}`; + case '&<': return sql.fragment`${i} OPERATOR(${schema}.&<) ${v}`; + case '&<|': return sql.fragment`${i} OPERATOR(${schema}.&<|) ${v}`; + case '&>': return sql.fragment`${i} OPERATOR(${schema}.&>) ${v}`; + case '|&>': return sql.fragment`${i} OPERATOR(${schema}.|&>) ${v}`; + case '<<': return sql.fragment`${i} OPERATOR(${schema}.<<) ${v}`; + case '<<|': return sql.fragment`${i} OPERATOR(${schema}.<<|) ${v}`; + case '>>': return sql.fragment`${i} OPERATOR(${schema}.>>) ${v}`; + case '|>>': return sql.fragment`${i} OPERATOR(${schema}.|>>) ${v}`; + case '~': return sql.fragment`${i} OPERATOR(${schema}.~) ${v}`; + case '~=': return sql.fragment`${i} OPERATOR(${schema}.~=) ${v}`; default: throw new Error(`Unexpected PostGIS SQL operator: ${op}`); } @@ -289,7 +290,8 @@ export function createPostgisOperatorFactory(): ConnectionFilterOperatorFactory operatorName, description, baseType: baseType as 'geometry' | 'geography', - resolve: (i: SQL, v: SQL) => buildOperatorExpr(capturedOp, i, v) + resolve: (i: SQL, v: SQL) => + buildOperatorExpr(schemaName, capturedOp, i, v) }); } } diff --git a/graphile/graphile-postgis/src/plugins/detect-extension.ts b/graphile/graphile-postgis/src/plugins/detect-extension.ts index b91f0ea182..85dba50051 100644 --- a/graphile/graphile-postgis/src/plugins/detect-extension.ts +++ b/graphile/graphile-postgis/src/plugins/detect-extension.ts @@ -8,6 +8,62 @@ import type { PostgisExtensionInfo } from '../types'; export type { PostgisExtensionInfo } from '../types'; +function codecIdentity(codec: any, typeName: string): { + serviceName: string; + schemaName: string; +} { + const pg = codec?.extensions?.pg; + if (!pg?.serviceName || !pg?.schemaName) { + throw new Error( + `[graphile-postgis] ${typeName} codec is missing exact service/schema metadata` + ); + } + return { serviceName: pg.serviceName, schemaName: pg.schemaName }; +} + +/** Resolve one unambiguous PostGIS installation identity for this build. */ +export function resolvePostgisExtensionInfo(build: any): PostgisExtensionInfo | undefined { + const pgRegistry = build.input?.pgRegistry; + if (!pgRegistry) return undefined; + + const geometryCodecs: PgCodec[] = []; + const geographyCodecs: PgCodec[] = []; + for (const codec of Object.values(pgRegistry.pgCodecs) as PgCodec[]) { + const name = codec?.extensions?.pg?.name; + if (name === 'geometry') geometryCodecs.push(codec); + if (name === 'geography') geographyCodecs.push(codec); + } + if (geometryCodecs.length === 0 && geographyCodecs.length === 0) return undefined; + if (geometryCodecs.length > 1 || geographyCodecs.length > 1) { + throw new Error( + `[graphile-postgis] Ambiguous codecs in one build ` + + `(geometry=${geometryCodecs.length}, geography=${geographyCodecs.length})` + ); + } + + const geometryCodec = geometryCodecs[0] ?? null; + const geographyCodec = geographyCodecs[0] ?? null; + const primary = geometryCodec ?? geographyCodec!; + const identity = codecIdentity(primary, geometryCodec ? 'geometry' : 'geography'); + if (geometryCodec && geographyCodec) { + const geographyIdentity = codecIdentity(geographyCodec, 'geography'); + if ( + geographyIdentity.serviceName !== identity.serviceName || + geographyIdentity.schemaName !== identity.schemaName + ) { + throw new Error( + '[graphile-postgis] geometry/geography codecs resolve to different service/schema identities' + ); + } + } + + return { + ...identity, + geometryCodec, + geographyCodec, + }; +} + /** * PostgisExtensionDetectionPlugin * @@ -25,44 +81,8 @@ export const PostgisExtensionDetectionPlugin: GraphileConfig.Plugin = { schema: { hooks: { build(build) { - const pgRegistry = build.input?.pgRegistry; - if (!pgRegistry) { - return build; - } - - let geometryCodec: PgCodec | null = null; - let geographyCodec: PgCodec | null = null; - let schemaName: string = 'public'; - - // Search through codecs for geometry and geography types - for (const codec of Object.values(pgRegistry.pgCodecs)) { - const pg = codec?.extensions?.pg; - if (!pg) continue; - - if (pg.name === 'geometry') { - geometryCodec = codec; - schemaName = pg.schemaName || 'public'; - } else if (pg.name === 'geography') { - geographyCodec = codec; - if (!geometryCodec) { - schemaName = pg.schemaName || 'public'; - } - } - } - - // PostGIS is detected when at least one of geometry or geography - // codecs is present. Some databases use only geography columns - // (e.g. use_geography: true in SearchSpatial), so PostGraphile may - // introspect geography but not geometry. - if (!geometryCodec && !geographyCodec) { - return build; - } - - const postgisInfo: PostgisExtensionInfo = { - schemaName, - geometryCodec, - geographyCodec - }; + const postgisInfo = resolvePostgisExtensionInfo(build); + if (!postgisInfo) return build; return build.extend(build, { pgGISExtensionInfo: postgisInfo, diff --git a/graphile/graphile-postgis/src/plugins/spatial-relations.ts b/graphile/graphile-postgis/src/plugins/spatial-relations.ts index 20adc5132c..68ecb1ba8e 100644 --- a/graphile/graphile-postgis/src/plugins/spatial-relations.ts +++ b/graphile/graphile-postgis/src/plugins/spatial-relations.ts @@ -451,7 +451,7 @@ function spatialFilterTypeName(build: any, rel: SpatialRelationInfo): string { * Build the SQL fragment that joins the inner (target) row to the outer * (owner) row using the resolved PostGIS predicate. */ -function buildSpatialJoinFragment( +export function buildSpatialJoinFragment( rel: SpatialRelationInfo, schemaName: string, outerAlias: SQL, @@ -467,8 +467,8 @@ function buildSpatialJoinFragment( const ownerExpr = sql`${outerAlias}.${sql.identifier(rel.ownerAttributeName)}`; const targetExpr = sql`${innerAlias}.${sql.identifier(rel.targetAttributeName)}`; if (rel.operator.kind === 'infix') { - // Only `&&` today — simple inline (symmetric). - return sql`${ownerExpr} && ${targetExpr}`; + // Only `&&` today. Bind it to this build's exact PostGIS namespace. + return sql`${ownerExpr} OPERATOR(${sql.identifier(schemaName)}.&&) ${targetExpr}`; } const fn = sql.identifier(schemaName, rel.operator.pgToken); if (rel.operator.parametric) { diff --git a/graphile/graphile-postgis/src/types.ts b/graphile/graphile-postgis/src/types.ts index 60defc8ea8..aa56511f48 100644 --- a/graphile/graphile-postgis/src/types.ts +++ b/graphile/graphile-postgis/src/types.ts @@ -22,6 +22,8 @@ export interface GisFieldValue { * PostGIS extension detection result stored on the build object. */ export interface PostgisExtensionInfo { + /** Exact Graphile PostgreSQL service that owns these codecs. */ + serviceName: string; /** The schema name where PostGIS is installed (e.g. 'public') */ schemaName: string; /** The geometry codec from the registry (null if only geography columns are used) */ diff --git a/graphile/graphile-search/package.json b/graphile/graphile-search/package.json index 160885140f..1ab2a88dc7 100644 --- a/graphile/graphile-search/package.json +++ b/graphile/graphile-search/package.json @@ -29,6 +29,7 @@ "url": "https://github.com/constructive-io/constructive/issues" }, "dependencies": { + "@pgsql/quotes": "^18.2.4", "graphile-plugin-utils": "workspace:^" }, "devDependencies": { diff --git a/graphile/graphile-search/src/__tests__/extension-schema-qualification.test.ts b/graphile/graphile-search/src/__tests__/extension-schema-qualification.test.ts new file mode 100644 index 0000000000..91fab9f486 --- /dev/null +++ b/graphile/graphile-search/src/__tests__/extension-schema-qualification.test.ts @@ -0,0 +1,248 @@ +import sql from 'pg-sql2'; + +import { createPgvectorAdapter } from '../adapters/pgvector'; +import { createTrgmAdapter } from '../adapters/trgm'; +import { createTrgmOperatorFactories } from '../codecs/operator-factories'; +import { VectorCodecPlugin } from '../codecs/vector-codec'; +import { + collectSearchExtensionSchemas, + requireBuildExtensionSchema, + resolveBuildExtensionSchema, + type SearchExtensionSchemas, +} from '../extension-metadata'; + +const extensionBinding = ( + overrides: Partial = {} +): SearchExtensionSchemas => ({ + serviceName: 'tenant_service', + pgTrgmSchema: 'extension_tools', + pgvectorSchema: 'extension_tools', + ...overrides, +}); + +const introspection = (extensions: any[]) => + ({ + extensions, + getNamespace: ({ id }: { id: string }) => + id === '910' ? { nspname: 'extension_tools' } : undefined, + }) as any; + +describe('search extension schema binding', () => { + it('collects exact pg_trgm and pgvector schemas from one service introspection', () => { + expect( + collectSearchExtensionSchemas( + introspection([ + { extname: 'pg_trgm', extnamespace: '910' }, + { extname: 'vector', extnamespace: '910' }, + ]), + 'tenant_service' + ) + ).toEqual(extensionBinding()); + }); + + it('fails closed on ambiguous, unresolved, or cross-service schemas', () => { + expect(() => + collectSearchExtensionSchemas( + introspection([ + { extname: 'pg_trgm', extnamespace: '910' }, + { extname: 'pg_trgm', extnamespace: '910' }, + ]), + 'tenant_service' + ) + ).toThrow(/ambiguous pg_trgm/); + + expect(() => + collectSearchExtensionSchemas( + introspection([{ extname: 'pg_trgm', extnamespace: '999' }]), + 'tenant_service' + ) + ).toThrow(/cannot resolve the namespace/); + + expect(() => + requireBuildExtensionSchema( + { + pgSearchExtensionSchemasByService: new Map([ + [ + 'a', + extensionBinding({ serviceName: 'a', pgTrgmSchema: 'ext_a' }), + ], + [ + 'b', + extensionBinding({ serviceName: 'b', pgTrgmSchema: 'ext_b' }), + ], + ]), + }, + 'pg_trgm' + ) + ).toThrow(/ambiguous schemas/); + + expect( + resolveBuildExtensionSchema( + { + pgSearchExtensionSchemasByService: new Map([ + ['tenant_service', extensionBinding({ pgTrgmSchema: null })], + ]), + }, + 'pg_trgm' + ) + ).toBeNull(); + }); +}); + +describe('pg_trgm SQL qualification', () => { + const adapter = createTrgmAdapter({ requireIntentionalSearch: false }); + + it('binds metadata to the eligible attribute and qualifies functions', () => { + const codec = { + name: 'documents', + attributes: { + title: { + codec: { name: 'text' }, + extensions: { searchExtensionSchemas: extensionBinding() }, + }, + }, + }; + const [column] = adapter.detectColumns(codec, {}); + const result = adapter.buildFilterApply( + sql, + sql.identifier('documents'), + column, + { value: 'memory density', threshold: 0.2 }, + {} + ); + expect(sql.compile(result!.whereClause!).text).toContain( + '"extension_tools"."similarity"("documents"."title", $1)' + ); + + const registrations = createTrgmOperatorFactories()({ + sql, + pgSearchExtensionSchemasByService: new Map([ + ['tenant_service', extensionBinding()], + ]), + getTypeByName: () => ({ name: 'TrgmSearchInput' }), + } as any); + const similar = registrations.find( + (entry) => entry.operatorName === 'similarTo' + )!; + const fragment = similar.spec.resolve!( + sql.identifier('title'), + sql.null, + { value: 'memory', threshold: 0.3 }, + null, + { fieldName: 'title', operatorName: 'similarTo' } + ); + expect(sql.compile(fragment!).text).toContain( + '"extension_tools"."similarity"' + ); + }); + + it('fails closed when an eligible attribute has no bound schema', () => { + expect(() => + adapter.detectColumns( + { + name: 'documents', + attributes: { title: { codec: { name: 'text' } } }, + }, + {} + ) + ).toThrow(/missing service-bound extension schema/); + }); +}); + +describe('pgvector SQL qualification', () => { + const vectorCodec = { + name: 'vector', + extensions: { + pg: { + serviceName: 'tenant_service', + schemaName: 'extension_tools', + name: 'vector', + }, + }, + }; + + it('qualifies and annotates a native vector codec during gather', async () => { + const gatherHook = (VectorCodecPlugin as any).gather.hooks + .pgCodecs_findPgCodec; + const event: any = { + pgCodec: { + name: 'vector', + sqlType: sql.fragment`vector`, + extensions: undefined, + }, + pgType: { typname: 'vector', typnamespace: '910', _id: '912' }, + serviceName: 'tenant_service', + }; + const originalCodec = event.pgCodec; + await gatherHook( + { + helpers: { + pgIntrospection: { + getNamespace: jest + .fn() + .mockResolvedValue({ nspname: 'extension_tools' }), + }, + }, + }, + event + ); + + expect(event.pgCodec).toBe(originalCodec); + expect(event.pgCodec.extensions.pg).toEqual({ + serviceName: 'tenant_service', + schemaName: 'extension_tools', + name: 'vector', + }); + expect(sql.compile(event.pgCodec.sqlType).text).toBe( + '"extension_tools"."vector"' + ); + }); + + it('qualifies the vector cast and distance operator', () => { + const adapter = createPgvectorAdapter(); + const [column] = adapter.detectColumns( + { + name: 'documents', + attributes: { + embedding: { + codec: vectorCodec, + extensions: { searchExtensionSchemas: extensionBinding() }, + }, + }, + }, + {} + ); + const result = adapter.buildFilterApply( + sql, + sql.identifier('documents'), + column, + { vector: [1, 0, 0], metric: 'COSINE' }, + {} + ); + const compiled = sql.compile(result!.scoreExpression); + expect(compiled.text).toContain('::"extension_tools"."vector"'); + expect(compiled.text).toContain('OPERATOR("extension_tools".<=>)'); + }); + + it('fails closed when codec and extension identities disagree', () => { + const adapter = createPgvectorAdapter(); + expect(() => + adapter.detectColumns( + { + name: 'documents', + attributes: { + embedding: { + codec: vectorCodec, + extensions: { + searchExtensionSchemas: extensionBinding({ + pgvectorSchema: 'other_extension_schema', + }), + }, + }, + }, + }, + {} + ) + ).toThrow(/does not match extension/); + }); +}); diff --git a/graphile/graphile-search/src/__tests__/search-config.test.ts b/graphile/graphile-search/src/__tests__/search-config.test.ts index fd7a559161..2c09b143c6 100644 --- a/graphile/graphile-search/src/__tests__/search-config.test.ts +++ b/graphile/graphile-search/src/__tests__/search-config.test.ts @@ -13,6 +13,31 @@ import { createPgvectorAdapter } from '../adapters/pgvector'; import { createTsvectorAdapter } from '../adapters/tsvector'; import { createUnifiedSearchPlugin } from '../plugin'; +const VECTOR_ADAPTER_IDENTITY = { + serviceName: 'main', + extensionSchema: 'extension_tools', +}; + +const vectorAttribute = () => ({ + codec: { + name: 'vector', + extensions: { + pg: { + serviceName: 'main', + schemaName: 'extension_tools', + name: 'vector', + }, + }, + }, + extensions: { + searchExtensionSchemas: { + serviceName: 'main', + pgTrgmSchema: 'extension_tools', + pgvectorSchema: 'extension_tools', + }, + }, +}); + // ─── pgvector adapter: chunk detection ──────────────────────────────────────── describe('pgvector adapter — chunk querying (Phase E)', () => { @@ -24,7 +49,7 @@ describe('pgvector adapter — chunk querying (Phase E)', () => { name: 'documents', attributes: { id: { codec: { name: 'uuid' } }, - embedding: { codec: { name: 'vector' } }, + embedding: vectorAttribute(), }, extensions: { tags: {} }, }; @@ -32,7 +57,7 @@ describe('pgvector adapter — chunk querying (Phase E)', () => { const columns = adapter.detectColumns(codec, {}); expect(columns).toHaveLength(1); expect(columns[0].attributeName).toBe('embedding'); - expect(columns[0].adapterData).toBeUndefined(); + expect(columns[0].adapterData).toEqual(VECTOR_ADAPTER_IDENTITY); }); it('includes chunksInfo when @hasChunks smart tag has metadata', () => { @@ -40,7 +65,7 @@ describe('pgvector adapter — chunk querying (Phase E)', () => { name: 'documents', attributes: { id: { codec: { name: 'uuid' } }, - embedding: { codec: { name: 'vector' } }, + embedding: vectorAttribute(), }, extensions: { tags: { @@ -58,6 +83,7 @@ describe('pgvector adapter — chunk querying (Phase E)', () => { expect(columns).toHaveLength(1); expect(columns[0].attributeName).toBe('embedding'); expect(columns[0].adapterData).toEqual({ + ...VECTOR_ADAPTER_IDENTITY, chunksInfo: { chunksSchema: 'app_public', chunksTableName: 'documents_chunks', @@ -75,7 +101,7 @@ describe('pgvector adapter — chunk querying (Phase E)', () => { const codec = { name: 'documents', attributes: { - embedding: { codec: { name: 'vector' } }, + embedding: vectorAttribute(), }, extensions: { tags: { @@ -93,6 +119,7 @@ describe('pgvector adapter — chunk querying (Phase E)', () => { const columns = adapter.detectColumns(codec, {}); expect(columns).toHaveLength(1); expect(columns[0].adapterData).toEqual({ + ...VECTOR_ADAPTER_IDENTITY, chunksInfo: { chunksSchema: 'private_schema', chunksTableName: 'doc_chunks', @@ -110,7 +137,7 @@ describe('pgvector adapter — chunk querying (Phase E)', () => { const codec = { name: 'documents', attributes: { - embedding: { codec: { name: 'vector' } }, + embedding: vectorAttribute(), }, extensions: { tags: { @@ -121,6 +148,7 @@ describe('pgvector adapter — chunk querying (Phase E)', () => { const columns = adapter.detectColumns(codec, {}); expect(columns[0].adapterData).toEqual({ + ...VECTOR_ADAPTER_IDENTITY, chunksInfo: { chunksSchema: null, chunksTableName: 'my_chunks', @@ -138,7 +166,7 @@ describe('pgvector adapter — chunk querying (Phase E)', () => { const codec = { name: 'documents', attributes: { - embedding: { codec: { name: 'vector' } }, + embedding: vectorAttribute(), }, extensions: { tags: { @@ -150,6 +178,7 @@ describe('pgvector adapter — chunk querying (Phase E)', () => { const columns = adapter.detectColumns(codec, {}); expect(columns[0].adapterData).toEqual({ + ...VECTOR_ADAPTER_IDENTITY, chunksInfo: { chunksSchema: 'my_schema', chunksTableName: 'my_chunks', @@ -167,7 +196,7 @@ describe('pgvector adapter — chunk querying (Phase E)', () => { const codec = { name: 'documents', attributes: { - embedding: { codec: { name: 'vector' } }, + embedding: vectorAttribute(), }, extensions: { tags: { hasChunks: true }, @@ -176,14 +205,14 @@ describe('pgvector adapter — chunk querying (Phase E)', () => { const columns = adapter.detectColumns(codec, {}); expect(columns).toHaveLength(1); - expect(columns[0].adapterData).toBeUndefined(); + expect(columns[0].adapterData).toEqual(VECTOR_ADAPTER_IDENTITY); }); it('ignores invalid JSON in @hasChunks string', () => { const codec = { name: 'documents', attributes: { - embedding: { codec: { name: 'vector' } }, + embedding: vectorAttribute(), }, extensions: { tags: { hasChunks: 'not-valid-json' }, @@ -192,7 +221,7 @@ describe('pgvector adapter — chunk querying (Phase E)', () => { const columns = adapter.detectColumns(codec, {}); expect(columns).toHaveLength(1); - expect(columns[0].adapterData).toBeUndefined(); + expect(columns[0].adapterData).toEqual(VECTOR_ADAPTER_IDENTITY); }); it('does not detect chunks when enableChunkQuerying is false', () => { @@ -200,7 +229,7 @@ describe('pgvector adapter — chunk querying (Phase E)', () => { const codec = { name: 'documents', attributes: { - embedding: { codec: { name: 'vector' } }, + embedding: vectorAttribute(), }, extensions: { tags: { @@ -211,7 +240,7 @@ describe('pgvector adapter — chunk querying (Phase E)', () => { const columns = noChunksAdapter.detectColumns(codec, {}); expect(columns).toHaveLength(1); - expect(columns[0].adapterData).toBeUndefined(); + expect(columns[0].adapterData).toEqual(VECTOR_ADAPTER_IDENTITY); }); }); @@ -220,7 +249,7 @@ describe('pgvector adapter — chunk querying (Phase E)', () => { // Mock sql object that mimics pg-sql2 behavior const mockSql = { - identifier: (name: string) => `"${name}"`, + identifier: (...names: string[]) => names.map((name) => `"${name}"`).join('.'), value: (val: any) => `'${val}'`, raw: (s: string) => s, fragment: (strings: TemplateStringsArray, ...values: any[]) => { @@ -251,7 +280,10 @@ describe('pgvector adapter — chunk querying (Phase E)', () => { const result = adapter.buildFilterApply( sql, 'tbl' as any, - { attributeName: 'embedding' }, + { + attributeName: 'embedding', + adapterData: VECTOR_ADAPTER_IDENTITY, + }, { vector: [1, 0, 0], metric: 'COSINE' }, {}, ); @@ -269,6 +301,7 @@ describe('pgvector adapter — chunk querying (Phase E)', () => { { attributeName: 'embedding', adapterData: { + ...VECTOR_ADAPTER_IDENTITY, chunksInfo: { chunksSchema: null, chunksTableName: 'documents_chunks', @@ -296,6 +329,7 @@ describe('pgvector adapter — chunk querying (Phase E)', () => { { attributeName: 'embedding', adapterData: { + ...VECTOR_ADAPTER_IDENTITY, chunksInfo: { chunksSchema: null, chunksTableName: 'documents_chunks', @@ -323,6 +357,7 @@ describe('pgvector adapter — chunk querying (Phase E)', () => { { attributeName: 'embedding', adapterData: { + ...VECTOR_ADAPTER_IDENTITY, chunksInfo: { chunksSchema: 'app_private', chunksTableName: 'doc_chunks', diff --git a/graphile/graphile-search/src/__tests__/sql-qualification.test.ts b/graphile/graphile-search/src/__tests__/sql-qualification.test.ts new file mode 100644 index 0000000000..41999db3e4 --- /dev/null +++ b/graphile/graphile-search/src/__tests__/sql-qualification.test.ts @@ -0,0 +1,45 @@ +import sql from 'pg-sql2'; + +import { createBm25Adapter } from '../adapters/bm25'; + +describe('BM25 SQL qualification', () => { + it('qualifies extension functions/operators and quotes physical index names', () => { + const store = new Map([ + [ + 'tenant-a.documents.content', + { + extensionSchema: 'extension-tools', + schemaName: 'tenant-a', + tableName: 'documents', + columnName: 'content', + indexName: 'documents"content_idx', + }, + ], + ]); + const adapter = createBm25Adapter({ bm25IndexStore: store }); + const [column] = adapter.detectColumns( + { + name: 'documents', + extensions: { + pg: { schemaName: 'tenant-a', name: 'documents' }, + }, + attributes: { + content: { codec: { name: 'text' } }, + }, + }, + {} + ); + const result = adapter.buildFilterApply( + sql, + sql.identifier('documents'), + column, + { query: 'memory density' }, + {} + ); + const compiled = sql.compile(result!.scoreExpression); + + expect(compiled.text).toContain('"extension-tools"."to_bm25query"'); + expect(compiled.text).toContain('OPERATOR("extension-tools".<@>)'); + expect(compiled.values).toContain('"tenant-a"."documents""content_idx"'); + }); +}); diff --git a/graphile/graphile-search/src/adapters/bm25.ts b/graphile/graphile-search/src/adapters/bm25.ts index d5ebd22254..a6daf64fc8 100644 --- a/graphile/graphile-search/src/adapters/bm25.ts +++ b/graphile/graphile-search/src/adapters/bm25.ts @@ -13,6 +13,7 @@ * LEAST(parent_score, chunk_score) (lower = better for BM25). */ +import { QuoteUtils } from '@pgsql/quotes'; import type { SQL } from 'pg-sql2'; import { bm25IndexStore as moduleBm25IndexStore } from '../codecs/bm25-codec'; @@ -23,6 +24,7 @@ import { type ChunksInfo,getChunksInfo } from './chunks'; * BM25 index info discovered during gather phase. */ export interface Bm25IndexInfo { + extensionSchema: string; schemaName: string; tableName: string; columnName: string; @@ -182,9 +184,15 @@ export function createBm25Adapter( const columnExpr = sql`${alias}.${sql.identifier(column.attributeName)}`; // Use quoteQualifiedIdentifier to produce the qualified index name - const qualifiedIndexName = `"${bm25Index.schemaName}"."${bm25Index.indexName}"`; - const bm25queryExpr = sql`to_bm25query(${sql.value(query)}, ${sql.value(qualifiedIndexName)})`; - const scoreExpr = sql`(${columnExpr} <@> ${bm25queryExpr})`; + const qualifiedIndexName = QuoteUtils.quoteQualifiedIdentifier( + bm25Index.schemaName, + bm25Index.indexName + ); + const toBm25Query = sql.identifier(bm25Index.extensionSchema, 'to_bm25query'); + const bm25queryExpr = sql`${toBm25Query}(${sql.value(query)}, ${sql.value(qualifiedIndexName)})`; + const scoreExpr = sql`(${columnExpr} OPERATOR(${sql.identifier( + bm25Index.extensionSchema + )}.<@>) ${bm25queryExpr})`; // Check for chunk-aware querying const chunksInfo = columnData.chunksInfo; @@ -200,9 +208,14 @@ export function createBm25Adapter( // BM25 on chunks requires an index name on the chunks table. // We construct it from the chunks table schema + a conventional index name. // The BM25 index on chunks is named: {chunks_table}_{content_field}_bm25_idx - const chunksIndexName = `"${chunksInfo.chunksSchema || bm25Index.schemaName}"."${chunksInfo.chunksTableName}_${chunksInfo.contentField}_bm25_idx"`; - const chunkBm25queryExpr = sql`to_bm25query(${sql.value(query)}, ${sql.value(chunksIndexName)})`; - const chunkScoreExpr = sql`(${chunksAlias}.${chunkContentField} <@> ${chunkBm25queryExpr})`; + const chunksIndexName = QuoteUtils.quoteQualifiedIdentifier( + chunksInfo.chunksSchema || bm25Index.schemaName, + `${chunksInfo.chunksTableName}_${chunksInfo.contentField}_bm25_idx` + ); + const chunkBm25queryExpr = sql`${toBm25Query}(${sql.value(query)}, ${sql.value(chunksIndexName)})`; + const chunkScoreExpr = sql`(${chunksAlias}.${chunkContentField} OPERATOR(${sql.identifier( + bm25Index.extensionSchema + )}.<@>) ${chunkBm25queryExpr})`; // Subquery: MIN(bm25_score) across chunks (lower = better for BM25) const chunkScoreSubquery = sql`( diff --git a/graphile/graphile-search/src/adapters/pgvector.ts b/graphile/graphile-search/src/adapters/pgvector.ts index 038509bfab..3807107ff8 100644 --- a/graphile/graphile-search/src/adapters/pgvector.ts +++ b/graphile/graphile-search/src/adapters/pgvector.ts @@ -8,9 +8,16 @@ import type { SQL } from 'pg-sql2'; +import type { SearchExtensionSchemas } from '../extension-metadata'; import type { FilterApplyResult,SearchableColumn, SearchAdapter } from '../types'; import { type ChunksInfo,getChunksInfo } from './chunks'; +interface PgvectorColumnData { + serviceName: string; + extensionSchema: string; + chunksInfo?: ChunksInfo; +} + /** * Build a distance expression for the given metric. * Uses explicit SQL template literals for each operator to avoid sql.raw. @@ -20,15 +27,16 @@ function buildDistanceExpr( columnExpr: SQL, vectorExpr: SQL, metric: string, + extensionSchema: string, ): SQL { switch (metric) { case 'L2': - return sql`(${columnExpr} <-> ${vectorExpr})`; + return sql`(${columnExpr} OPERATOR(${sql.identifier(extensionSchema)}.<->) ${vectorExpr})`; case 'IP': - return sql`(${columnExpr} <#> ${vectorExpr})`; + return sql`(${columnExpr} OPERATOR(${sql.identifier(extensionSchema)}.<#>) ${vectorExpr})`; case 'COSINE': default: - return sql`(${columnExpr} <=> ${vectorExpr})`; + return sql`(${columnExpr} OPERATOR(${sql.identifier(extensionSchema)}.<=>) ${vectorExpr})`; } } @@ -92,9 +100,33 @@ export function createPgvectorAdapter( codec.attributes as Record )) { if (isVectorCodec(attribute.codec)) { + const binding: SearchExtensionSchemas | undefined = + attribute?.extensions?.searchExtensionSchemas; + const codecPg = attribute.codec?.extensions?.pg; + if (!binding?.pgvectorSchema || !codecPg?.schemaName || !codecPg?.serviceName) { + const tableName = codec?.extensions?.pg?.name ?? codec?.name ?? ''; + throw new Error( + `[graphile-search] pgvector column '${tableName}.${attributeName}' is ` + + 'missing exact codec/service extension metadata' + ); + } + if ( + codecPg.schemaName !== binding.pgvectorSchema || + codecPg.serviceName !== binding.serviceName + ) { + throw new Error( + `[graphile-search] pgvector column '${attributeName}' codec identity ` + + `'${codecPg.serviceName}/${codecPg.schemaName}' does not match extension ` + + `'${binding.serviceName}/${binding.pgvectorSchema}'` + ); + } columns.push({ attributeName, - adapterData: chunksInfo ? { chunksInfo } : undefined, + adapterData: { + serviceName: binding.serviceName, + extensionSchema: binding.pgvectorSchema, + ...(chunksInfo ? { chunksInfo } : {}), + } satisfies PgvectorColumnData, }); } } @@ -195,12 +227,21 @@ export function createPgvectorAdapter( const { vector, metric, distance, includeChunks } = filterValue; if (!vector || !Array.isArray(vector) || vector.length === 0) return null; + const adapterData = column.adapterData as PgvectorColumnData | undefined; + if (!adapterData?.extensionSchema || !adapterData.serviceName) { + throw new Error( + `[graphile-search] pgvector column '${column.attributeName}' has no bound ` + + 'extension schema' + ); + } const resolvedMetric = metric || defaultMetric; const vectorString = `[${vector.join(',')}]`; - const vectorExpr = sql`${sql.value(vectorString)}::vector`; + const vectorExpr = sql`${sql.value(vectorString)}::${sql.identifier( + adapterData.extensionSchema, + 'vector' + )}`; // Check if this column has chunks info and chunk querying is requested - const adapterData = column.adapterData as { chunksInfo?: ChunksInfo } | undefined; const chunksInfo = adapterData?.chunksInfo; if (chunksInfo && (includeChunks !== false)) { @@ -217,7 +258,13 @@ export function createPgvectorAdapter( const chunksAlias = sql.identifier('__chunks'); // Subquery: SELECT MIN(distance) FROM chunks WHERE chunks.parent_fk = parent.pk - const chunkDistanceExpr = buildDistanceExpr(sql, sql`${chunksAlias}.${chunkEmbedding}`, vectorExpr, resolvedMetric); + const chunkDistanceExpr = buildDistanceExpr( + sql, + sql`${chunksAlias}.${chunkEmbedding}`, + vectorExpr, + resolvedMetric, + adapterData.extensionSchema + ); const chunkDistanceSubquery = sql`( SELECT MIN(${chunkDistanceExpr}) FROM ${chunksTableRef} AS ${chunksAlias} @@ -226,7 +273,13 @@ export function createPgvectorAdapter( // Also compute direct parent distance if the parent has an embedding const parentColumnExpr = sql`${alias}.${sql.identifier(column.attributeName)}`; - const parentDistanceExpr = buildDistanceExpr(sql, parentColumnExpr, vectorExpr, resolvedMetric); + const parentDistanceExpr = buildDistanceExpr( + sql, + parentColumnExpr, + vectorExpr, + resolvedMetric, + adapterData.extensionSchema + ); // Use LEAST of parent distance and closest chunk distance // COALESCE handles cases where parent or chunks may not have embeddings @@ -248,7 +301,13 @@ export function createPgvectorAdapter( // Standard (non-chunk) query const columnExpr = sql`${alias}.${sql.identifier(column.attributeName)}`; - const distanceExpr = buildDistanceExpr(sql, columnExpr, vectorExpr, resolvedMetric); + const distanceExpr = buildDistanceExpr( + sql, + columnExpr, + vectorExpr, + resolvedMetric, + adapterData.extensionSchema + ); let whereClause: SQL | null = null; if (distance !== undefined && distance !== null) { diff --git a/graphile/graphile-search/src/adapters/trgm.ts b/graphile/graphile-search/src/adapters/trgm.ts index 103e6c7bd4..80ebf81741 100644 --- a/graphile/graphile-search/src/adapters/trgm.ts +++ b/graphile/graphile-search/src/adapters/trgm.ts @@ -12,6 +12,7 @@ import type { SQL } from 'pg-sql2'; +import type { SearchExtensionSchemas } from '../extension-metadata'; import type { FilterApplyResult,SearchableColumn, SearchAdapter } from '../types'; import { type ChunksInfo,getChunksInfo } from './chunks'; @@ -48,6 +49,12 @@ export interface TrgmAdapterOptions { requireIntentionalSearch?: boolean; } +interface TrgmColumnData { + serviceName: string; + extensionSchema: string; + chunksInfo?: ChunksInfo; +} + export function createTrgmAdapter( options: TrgmAdapterOptions = {} ): SearchAdapter { @@ -89,12 +96,39 @@ export function createTrgmAdapter( codec.attributes as Record )) { if (isTextCodec(attribute.codec)) { + const binding: SearchExtensionSchemas | undefined = + attribute?.extensions?.searchExtensionSchemas; + if (!binding) { + const tableName = codec?.extensions?.pg?.name ?? codec?.name ?? ''; + throw new Error( + `[graphile-search] pg_trgm column '${tableName}.${attributeName}' is ` + + 'missing service-bound extension schema metadata' + ); + } + if (!binding.pgTrgmSchema) { + const explicitlyRequired = + requireIntentionalSearch === false || + codec?.extensions?.tags?.trgmSearch === true || + attribute?.extensions?.tags?.trgmSearch === true; + if (explicitlyRequired) { + const tableName = codec?.extensions?.pg?.name ?? codec?.name ?? ''; + throw new Error( + `[graphile-search] pg_trgm is required for '${tableName}.${attributeName}' ` + + `but is not installed for service '${binding.serviceName}'` + ); + } + continue; + } // Store chunks info if available and chunks have trigram search const chunksInfo = getChunksInfo(codec); const hasChunkTrgm = chunksInfo?.searchIndexes.includes('trigram'); columns.push({ attributeName, - adapterData: hasChunkTrgm ? chunksInfo : undefined, + adapterData: { + serviceName: binding.serviceName, + extensionSchema: binding.pgTrgmSchema, + ...(hasChunkTrgm ? { chunksInfo } : {}), + } satisfies TrgmColumnData, }); } } @@ -152,12 +186,20 @@ export function createTrgmAdapter( const { value, threshold, includeChunks } = filterValue; if (!value || typeof value !== 'string' || value.trim().length === 0) return null; + const columnData = column.adapterData as TrgmColumnData | undefined; + if (!columnData?.extensionSchema || !columnData.serviceName) { + throw new Error( + `[graphile-search] pg_trgm column '${column.attributeName}' has no bound ` + + 'extension schema' + ); + } const th = threshold != null ? threshold : defaultThreshold; const columnExpr = sql`${alias}.${sql.identifier(column.attributeName)}`; - const similarityExpr = sql`similarity(${columnExpr}, ${sql.value(value)})`; + const similarity = sql.identifier(columnData.extensionSchema, 'similarity'); + const similarityExpr = sql`${similarity}(${columnExpr}, ${sql.value(value)})`; // Check for chunk-aware querying - const chunksInfo = column.adapterData as ChunksInfo | undefined; + const chunksInfo = columnData.chunksInfo; if (chunksInfo && chunksInfo.searchIndexes.includes('trigram') && (includeChunks !== false)) { const chunksTableRef = chunksInfo.chunksSchema ? sql`${sql.identifier(chunksInfo.chunksSchema)}.${sql.identifier(chunksInfo.chunksTableName)}` @@ -169,10 +211,10 @@ export function createTrgmAdapter( // Subquery: MAX(similarity) across chunks (higher = better for trgm) const chunkSimilaritySubquery = sql`( - SELECT MAX(similarity(${chunksAlias}.${chunkContentField}, ${sql.value(value)})) + SELECT MAX(${similarity}(${chunksAlias}.${chunkContentField}, ${sql.value(value)})) FROM ${chunksTableRef} AS ${chunksAlias} WHERE ${chunksAlias}.${parentFk} = ${parentId} - AND similarity(${chunksAlias}.${chunkContentField}, ${sql.value(value)}) > ${sql.value(th)} + AND ${similarity}(${chunksAlias}.${chunkContentField}, ${sql.value(value)}) > ${sql.value(th)} )`; // Combined: GREATEST of parent similarity and best chunk similarity diff --git a/graphile/graphile-search/src/codecs/bm25-codec.ts b/graphile/graphile-search/src/codecs/bm25-codec.ts index b48beceeda..bd365217e7 100644 --- a/graphile/graphile-search/src/codecs/bm25-codec.ts +++ b/graphile/graphile-search/src/codecs/bm25-codec.ts @@ -21,6 +21,8 @@ import sql from 'pg-sql2'; * Represents a discovered BM25 index in the database. */ export interface Bm25IndexInfo { + /** Schema containing pg_textsearch functions and operators. */ + extensionSchema: string; /** Schema name (e.g. 'public') */ schemaName: string; /** Table name (e.g. 'documents') */ @@ -52,6 +54,7 @@ export let bm25ExtensionDetected = false; */ const BM25_DISCOVERY_SQL = ` SELECT + en.nspname AS extension_schema, n.nspname AS schema_name, c.relname AS table_name, a.attname AS column_name, @@ -62,6 +65,8 @@ const BM25_DISCOVERY_SQL = ` JOIN pg_class c ON c.oid = ix.indrelid JOIN pg_namespace n ON n.oid = c.relnamespace JOIN pg_attribute a ON a.attrelid = c.oid AND a.attnum = ANY(ix.indkey) + JOIN pg_extension e ON e.extname = 'pg_textsearch' + JOIN pg_namespace en ON en.oid = e.extnamespace WHERE am.amname = 'bm25' `; @@ -152,6 +157,7 @@ export const Bm25CodecPlugin: GraphileConfig.Plugin = { for (const row of result.rows) { const key = `${row.schema_name}.${row.table_name}.${row.column_name}`; bm25IndexStore.set(key, { + extensionSchema: row.extension_schema, schemaName: row.schema_name, tableName: row.table_name, columnName: row.column_name, diff --git a/graphile/graphile-search/src/codecs/operator-factories.ts b/graphile/graphile-search/src/codecs/operator-factories.ts index a8e8a80b0f..bab9cf9259 100644 --- a/graphile/graphile-search/src/codecs/operator-factories.ts +++ b/graphile/graphile-search/src/codecs/operator-factories.ts @@ -11,6 +11,7 @@ import type { ConnectionFilterOperatorFactory } from 'graphile-connection-filter'; import type { SQL } from 'pg-sql2'; +import { resolveBuildExtensionSchema } from '../extension-metadata'; /** * Creates the `matches` filter operator factory for full-text search. * Declared here so it's registered via the declarative @@ -59,6 +60,10 @@ export function createMatchesOperatorFactory( export function createTrgmOperatorFactories(): ConnectionFilterOperatorFactory { return (build) => { const { sql } = build; + const extensionSchema = resolveBuildExtensionSchema(build, 'pg_trgm'); + if (!extensionSchema) return []; + const similarity = sql.identifier(extensionSchema, 'similarity'); + const wordSimilarity = sql.identifier(extensionSchema, 'word_similarity'); return [ { @@ -82,7 +87,7 @@ export function createTrgmOperatorFactories(): ConnectionFilterOperatorFactory { return null; } const th = threshold != null ? threshold : 0.3; - return sql`similarity(${sqlIdentifier}, ${sql.value(value)}) > ${sql.value(th)}`; + return sql`${similarity}(${sqlIdentifier}, ${sql.value(value)}) > ${sql.value(th)}`; }, }, }, @@ -107,7 +112,7 @@ export function createTrgmOperatorFactories(): ConnectionFilterOperatorFactory { return null; } const th = threshold != null ? threshold : 0.3; - return sql`word_similarity(${sql.value(value)}, ${sqlIdentifier}) > ${sql.value(th)}`; + return sql`${wordSimilarity}(${sql.value(value)}, ${sqlIdentifier}) > ${sql.value(th)}`; }, }, }, diff --git a/graphile/graphile-search/src/codecs/vector-codec.ts b/graphile/graphile-search/src/codecs/vector-codec.ts index e764ba1238..539f22dc87 100644 --- a/graphile/graphile-search/src/codecs/vector-codec.ts +++ b/graphile/graphile-search/src/codecs/vector-codec.ts @@ -26,8 +26,6 @@ export const VectorCodecPlugin: GraphileConfig.Plugin = { gather: { hooks: { async pgCodecs_findPgCodec(info, event) { - if (event.pgCodec) return; - const { pgType: type, serviceName } = event; if (type.typname !== 'vector') return; @@ -35,10 +33,41 @@ export const VectorCodecPlugin: GraphileConfig.Plugin = { serviceName, type.typnamespace ); - if (!typeNamespace) return; + if (!typeNamespace?.nspname) { + throw new Error( + `[graphile-search] Cannot resolve the vector type namespace for ` + + `service '${serviceName}'` + ); + } const schemaName = typeNamespace.nspname; + if (event.pgCodec) { + const existingPg = event.pgCodec.extensions?.pg; + if ( + (existingPg?.serviceName && existingPg.serviceName !== serviceName) || + (existingPg?.schemaName && existingPg.schemaName !== schemaName) + ) { + throw new Error( + `[graphile-search] Existing vector codec identity conflicts with ` + + `introspection for service '${serviceName}'` + ); + } + const existingCodec = event.pgCodec as any; + existingCodec.sqlType = sql.identifier(schemaName, 'vector'); + existingCodec.extensions = { + ...existingCodec.extensions, + oid: type._id, + pg: { + ...existingPg, + serviceName, + schemaName, + name: 'vector', + }, + }; + return; + } + event.pgCodec = { name: 'vector', sqlType: sql.identifier(schemaName, 'vector'), diff --git a/graphile/graphile-search/src/extension-metadata.ts b/graphile/graphile-search/src/extension-metadata.ts new file mode 100644 index 0000000000..d19166aa23 --- /dev/null +++ b/graphile/graphile-search/src/extension-metadata.ts @@ -0,0 +1,232 @@ +import 'graphile-build'; +import 'graphile-build-pg'; + +import type { GraphileConfig } from 'graphile-config'; +import { gatherConfig } from 'graphile-build'; + +type Introspection = Parameters< + GraphileConfig.GatherHooks['pgIntrospection_introspection'] +>[0]['introspection']; + +/** Extension namespaces discovered for one exact Graphile PostgreSQL service. */ +export interface SearchExtensionSchemas { + serviceName: string; + pgTrgmSchema: string | null; + pgvectorSchema: string | null; +} + +declare global { + namespace GraphileConfig { + interface GatherHelpers { + unifiedSearchExtensionMetadata: Record; + } + } + + namespace DataplanPg { + interface PgCodecExtensions { + /** Exact extension schemas for the service that owns this record codec. */ + searchExtensionSchemas?: SearchExtensionSchemas; + } + + interface PgCodecAttributeExtensions { + /** Exact extension schemas bound from this service's introspection generation. */ + searchExtensionSchemas?: SearchExtensionSchemas; + } + } + + namespace GraphileBuild { + interface Build { + /** Per-service extension schemas for this build only. */ + pgSearchExtensionSchemasByService?: ReadonlyMap< + string, + SearchExtensionSchemas + >; + } + } +} + +function extensionSchema( + introspection: Introspection, + extensionName: string, + serviceName: string +): string | null { + const matches = introspection.extensions.filter( + (extension) => extension.extname === extensionName + ); + if (matches.length > 1) { + throw new Error( + `[graphile-search] Service '${serviceName}' has ambiguous ${extensionName} ` + + `extension metadata (${matches.length} entries)` + ); + } + if (matches.length === 0) return null; + + const extension = matches[0]; + if (extension.extnamespace == null) { + throw new Error( + `[graphile-search] Service '${serviceName}' has ${extensionName} without an ` + + 'introspected extension namespace' + ); + } + const namespace = introspection.getNamespace({ id: extension.extnamespace }); + if (!namespace?.nspname) { + throw new Error( + `[graphile-search] Service '${serviceName}' cannot resolve the namespace for ` + + `${extensionName}` + ); + } + return namespace.nspname; +} + +/** Resolve extension schemas exclusively from the current service introspection. */ +export function collectSearchExtensionSchemas( + introspection: Introspection, + serviceName: string +): SearchExtensionSchemas { + return Object.freeze({ + serviceName, + pgTrgmSchema: extensionSchema(introspection, 'pg_trgm', serviceName), + pgvectorSchema: extensionSchema(introspection, 'vector', serviceName), + }); +} + +/** Gather exact optional-extension schemas while service identity is explicit. */ +export const SearchExtensionMetadataGather = gatherConfig({ + namespace: 'unifiedSearchExtensionMetadata', + initialState: () => ({ + schemasByService: new Map(), + }), + helpers: {}, + hooks: { + pgIntrospection_introspection(info, event) { + const { introspection, serviceName } = event; + info.state.schemasByService.set( + serviceName, + collectSearchExtensionSchemas(introspection, serviceName) + ); + }, + + pgCodecs_PgCodec(info, event) { + if (!event.pgClass) return; + const binding = info.state.schemasByService.get(event.serviceName); + if (!binding) { + throw new Error( + `[graphile-search] No extension metadata was gathered for service ` + + `'${event.serviceName}'` + ); + } + event.pgCodec.extensions ??= Object.create(null); + event.pgCodec.extensions.searchExtensionSchemas = binding; + }, + + pgCodecs_attribute(info, event) { + const binding = info.state.schemasByService.get(event.serviceName); + if (!binding) { + throw new Error( + `[graphile-search] No extension metadata was gathered for service ` + + `'${event.serviceName}'` + ); + } + event.attribute.extensions ??= Object.create(null); + event.attribute.extensions.searchExtensionSchemas = binding; + }, + }, +}); + +/** Build an immutable, consistency-checked service map from bound attributes. */ +export function extensionSchemasByService( + build: any +): ReadonlyMap { + const schemasByService = new Map(); + const codecs = build.input?.pgRegistry?.pgCodecs; + if (!codecs) return schemasByService; + + const addBinding = (binding: SearchExtensionSchemas): void => { + const existing = schemasByService.get(binding.serviceName); + if ( + existing && + (existing.pgTrgmSchema !== binding.pgTrgmSchema || + existing.pgvectorSchema !== binding.pgvectorSchema) + ) { + throw new Error( + `[graphile-search] Conflicting extension metadata for service ` + + `'${binding.serviceName}' in one build` + ); + } + schemasByService.set(binding.serviceName, binding); + }; + + for (const codec of Object.values(codecs) as any[]) { + const codecBinding: SearchExtensionSchemas | undefined = + codec?.extensions?.searchExtensionSchemas; + if (codecBinding) addBinding(codecBinding); + if (!codec?.attributes) continue; + for (const attribute of Object.values(codec.attributes) as any[]) { + const binding: SearchExtensionSchemas | undefined = + attribute?.extensions?.searchExtensionSchemas; + if (binding) addBinding(binding); + } + } + return schemasByService; +} + +/** Resolve one unambiguous extension namespace for a build-wide operator factory. */ +export function resolveBuildExtensionSchema( + build: any, + extension: 'pg_trgm' | 'vector' +): string | null { + const schemasByService: ReadonlyMap = + build.pgSearchExtensionSchemasByService ?? extensionSchemasByService(build); + if (schemasByService.size === 0) { + const codecs = build.input?.pgRegistry?.pgCodecs; + const hasServiceBoundCodec = + codecs && + Object.values(codecs).some( + (codec: any) => + codec?.attributes != null || + codec?.extensions?.pg?.serviceName != null + ); + if (!hasServiceBoundCodec) return null; + throw new Error( + `[graphile-search] ${extension} requires service-bound extension metadata` + ); + } + + const field = extension === 'pg_trgm' ? 'pgTrgmSchema' : 'pgvectorSchema'; + const schemas = new Set(); + let missingCount = 0; + for (const binding of schemasByService.values()) { + const schemaName = binding[field]; + if (!schemaName) { + missingCount++; + continue; + } + schemas.add(schemaName); + } + if (schemas.size === 0) return null; + if (missingCount > 0) { + throw new Error( + `[graphile-search] ${extension} is present for only part of this multi-service build` + ); + } + if (schemas.size !== 1) { + throw new Error( + `[graphile-search] ${extension} has ambiguous schemas across this build: ` + + [...schemas].sort().join(', ') + ); + } + return schemas.values().next().value!; +} + +export function requireBuildExtensionSchema( + build: any, + extension: 'pg_trgm' | 'vector' +): string { + const schemaName = resolveBuildExtensionSchema(build, extension); + if (!schemaName) { + throw new Error( + `[graphile-search] ${extension} is required by this feature but is not installed` + ); + } + return schemaName; +} diff --git a/graphile/graphile-search/src/index.ts b/graphile/graphile-search/src/index.ts index b28afee219..4a1eebf4e5 100644 --- a/graphile/graphile-search/src/index.ts +++ b/graphile/graphile-search/src/index.ts @@ -30,6 +30,13 @@ * ``` */ +export type { SearchExtensionSchemas } from './extension-metadata'; +export { + collectSearchExtensionSchemas, + requireBuildExtensionSchema, + resolveBuildExtensionSchema, +} from './extension-metadata'; + // Core plugin export { createUnifiedSearchPlugin } from './plugin'; diff --git a/graphile/graphile-search/src/plugin.ts b/graphile/graphile-search/src/plugin.ts index c67f796be3..e0cb078947 100644 --- a/graphile/graphile-search/src/plugin.ts +++ b/graphile/graphile-search/src/plugin.ts @@ -26,6 +26,10 @@ import { TYPES } from '@dataplan/pg'; import type { GraphileConfig } from 'graphile-config'; import { getQueryBuilder } from 'graphile-plugin-utils'; +import { + extensionSchemasByService, + SearchExtensionMetadataGather, +} from './extension-metadata'; import type { SearchableColumn, SearchAdapter, UnifiedSearchOptions } from './types'; // ─── TypeScript Namespace Augmentations ────────────────────────────────────── @@ -260,6 +264,8 @@ export function createUnifiedSearchPlugin( 'VectorCodecPlugin', ], + gather: SearchExtensionMetadataGather, + // ─── Custom Inflection Methods ───────────────────────────────────── inflection: { add: { @@ -328,6 +334,16 @@ export function createUnifiedSearchPlugin( }, hooks: { + build(build) { + return build.extend( + build, + { + pgSearchExtensionSchemasByService: extensionSchemasByService(build), + }, + 'UnifiedSearchPlugin adding per-service extension schemas' + ); + }, + /** * Register all adapter-specific GraphQL types during init. */ diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index ca6ae59d08..f658b401ac 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -461,6 +461,9 @@ importers: '@dataplan/pg': specifier: 1.1.1 version: 1.1.1(@dataplan/json@1.0.1(grafast@1.1.2(graphql@16.13.0)))(grafast@1.1.2(graphql@16.13.0))(graphile-config@1.1.0)(graphql@16.13.0)(pg-sql2@5.0.1)(pg@8.21.0) + '@pgsql/quotes': + specifier: ^18.2.4 + version: 18.2.4 grafast: specifier: 1.1.2 version: 1.1.2(graphql@16.13.0) @@ -684,6 +687,9 @@ importers: '@dataplan/pg': specifier: 1.1.1 version: 1.1.1(@dataplan/json@1.0.1(grafast@1.1.2(graphql@16.13.0)))(grafast@1.1.2(graphql@16.13.0))(graphile-config@1.1.0)(graphql@16.13.0)(pg-sql2@5.0.1)(pg@8.21.0) + '@pgsql/quotes': + specifier: ^18.2.4 + version: 18.2.4 accept-language-parser: specifier: ^1.5.0 version: 1.5.0 @@ -749,6 +755,9 @@ importers: '@dataplan/pg': specifier: 1.1.1 version: 1.1.1(@dataplan/json@1.0.1(grafast@1.1.2(graphql@16.13.0)))(grafast@1.1.2(graphql@16.13.0))(graphile-config@1.1.0)(graphql@16.13.0)(pg-sql2@5.0.1)(pg@8.21.0) + '@pgsql/quotes': + specifier: ^18.2.4 + version: 18.2.4 grafast: specifier: 1.1.2 version: 1.1.2(graphql@16.13.0) @@ -1230,6 +1239,9 @@ importers: '@dataplan/pg': specifier: 1.1.1 version: 1.1.1(@dataplan/json@1.0.1(grafast@1.1.2(graphql@16.13.0)))(grafast@1.1.2(graphql@16.13.0))(graphile-config@1.1.0)(graphql@16.13.0)(pg-sql2@5.0.1)(pg@8.21.0) + '@pgsql/quotes': + specifier: ^18.2.4 + version: 18.2.4 graphile-build: specifier: 5.1.1 version: 5.1.1(grafast@1.1.2(graphql@16.13.0))(graphile-config@1.1.0)(graphql@16.13.0)