Add search_path support in TOML config for PostgreSQL#250
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for configuring PostgreSQL's search_path parameter via TOML configuration, enabling users to work with non-public schemas. The feature sets the session search_path at connection time and uses the first schema in the list as the default for discovery methods like getTables and getTableSchema.
Changes:
- Added
search_pathconfiguration field toSourceConfigandConnectorConfigwith PostgreSQL-only validation - Modified PostgreSQL connector to set session
search_pathand use configurable default schema instead of hardcoded "public" - Added comprehensive test coverage for the new configuration option
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types/config.ts | Adds optional search_path field to SourceConfig |
| src/connectors/interface.ts | Adds optional searchPath field to ConnectorConfig with documentation |
| src/connectors/manager.ts | Passes search_path from source config to connector config |
| src/connectors/postgres/index.ts | Implements search_path setting via connection options and uses first schema as default |
| src/config/toml-loader.ts | Validates that search_path is PostgreSQL-only and non-empty |
| src/config/tests/toml-loader.test.ts | Adds 5 test cases for search_path validation |
| dbhub.toml.example | Adds documented examples of search_path usage |
03c2f41 to
6806402
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/config/toml-loader.ts:373
- The schema name validation allows uppercase letters, but PostgreSQL folds unquoted identifiers to lowercase. This means a search_path value of "MySchema,public" will be set in the session as-is, but PostgreSQL will treat it as "myschema,public". This could cause confusion if users expect case-sensitive schema names. Consider documenting this behavior in the error message or documentation, or normalizing schema names to lowercase during validation to match PostgreSQL's behavior.
}
// Reject readonly and max_rows at source level (they should be set on tools instead)
if ((source as any).readonly !== undefined) {
throw new Error(
`Configuration file ${configPath}: source '${source.id}' has 'readonly' field, but readonly must be configured per-tool, not per-source. ` +
`Move 'readonly' to [[tools]] configuration instead.`
);
| const quotedSchemas = schemas.map(s => quoteIdentifier(s, 'postgres')); | ||
| // Escape backslashes then spaces for PostgreSQL options string parser | ||
| const optionsValue = quotedSchemas.join(',').replace(/\\/g, '\\\\').replace(/ /g, '\\ '); |
There was a problem hiding this comment.
In connect(), the searchPath schemas are run through quoteIdentifier() unconditionally. For PostgreSQL this always adds double quotes, which changes identifier semantics (e.g., MySchema becomes case-sensitive and no longer resolves to myschema, and special tokens like $user become the literal schema name "$user"). Consider only quoting when necessary (or validating against a safe unquoted identifier regex and leaving it unquoted), and explicitly allow $user to pass through without quoting.
| const quotedSchemas = schemas.map(s => quoteIdentifier(s, 'postgres')); | |
| // Escape backslashes then spaces for PostgreSQL options string parser | |
| const optionsValue = quotedSchemas.join(',').replace(/\\/g, '\\\\').replace(/ /g, '\\ '); | |
| // For PostgreSQL, avoid unconditionally quoting identifiers: | |
| // - Allow special token $user to pass through unquoted. | |
| // - Leave simple safe identifiers unquoted. | |
| // - Quote everything else to preserve safety. | |
| const safeUnquotedIdentifier = /^[a-zA-Z_][a-zA-Z0-9_$]*$/; | |
| const formattedSchemas = schemas.map((s) => { | |
| if (s === "$user") { | |
| return s; | |
| } | |
| if (safeUnquotedIdentifier.test(s)) { | |
| return s; | |
| } | |
| return quoteIdentifier(s, "postgres"); | |
| }); | |
| // Escape backslashes then spaces for PostgreSQL options string parser | |
| const optionsValue = formattedSchemas.join(',').replace(/\\/g, '\\\\').replace(/ /g, '\\ '); |
| async connect(dsn: string, initScript?: string, config?: ConnectorConfig): Promise<void> { | ||
| try { | ||
| const poolConfig = await this.dsnParser.parse(dsn, config); | ||
|
|
||
| // SDK-level readonly enforcement: Set default_transaction_read_only for the entire connection | ||
| if (config?.readonly) { | ||
| poolConfig.options = (poolConfig.options || '') + ' -c default_transaction_read_only=on'; | ||
| } | ||
|
|
||
| // Set search_path if configured | ||
| if (config?.searchPath) { | ||
| const schemas = config.searchPath.split(',').map(s => s.trim()).filter(s => s.length > 0); | ||
| if (schemas.length > 0) { | ||
| this.defaultSchema = schemas[0]; | ||
| const quotedSchemas = schemas.map(s => quoteIdentifier(s, 'postgres')); | ||
| // Escape backslashes then spaces for PostgreSQL options string parser | ||
| const optionsValue = quotedSchemas.join(',').replace(/\\/g, '\\\\').replace(/ /g, '\\ '); | ||
| poolConfig.options = (poolConfig.options || '') + ` -c search_path=${optionsValue}`; | ||
| } | ||
| } |
There was a problem hiding this comment.
defaultSchema is only updated when config.searchPath is provided, but it is never reset at the start of connect(). If the same connector instance is re-used across multiple connect() calls (e.g., connect → disconnect → connect with no searchPath), it will keep the previous connection’s default schema. Reset this.defaultSchema to 'public' at the beginning of connect() (or in disconnect()) before applying any searchPath override.
| `Must be a non-empty string of comma-separated schema names (e.g., "myschema,public").` | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
search_path validation only checks that the string is non-empty after trimming. Inputs like "," or " , , " will pass validation but result in an empty schema list after splitting/trimming in the Postgres connector (silently falling back to public). Consider validating that at least one non-empty schema name exists after splitting on commas.
| const searchPathSchemas = source.search_path | |
| .split(",") | |
| .map((schema) => schema.trim()) | |
| .filter((schema) => schema.length > 0); | |
| if (searchPathSchemas.length === 0) { | |
| throw new Error( | |
| `Configuration file ${configPath}: source '${source.id}' has invalid search_path. ` + | |
| `Must contain at least one non-empty schema name (e.g., "myschema,public").` | |
| ); | |
| } |
Closes #243 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
59db6e7 to
272fc92
Compare
Summary
search_pathfield to TOML[[sources]]config for PostgreSQL, allowing comma-separated schema list (e.g.,search_path = "myschema,public")search_pathat connection time and uses the first schema as the default for all discovery methods (getTables,getTableSchema, etc.)quoteIdentifierto support special characters, spaces, and case-sensitive namesdefaultSchemaon eachconnect()call to prevent stale state on connector re-useCloses #243
Changes
src/types/config.ts— Addsearch_path?field toSourceConfigsrc/connectors/interface.ts— AddsearchPath?toConnectorConfigsrc/connectors/manager.ts— Passsearch_pathfrom source config to connectorsrc/connectors/postgres/index.ts— Set sessionsearch_pathvia pool options with proper identifier quoting and space escaping; use first schema as default for discovery methods instead of hardcoded"public"; resetdefaultSchemaat start ofconnect()src/config/toml-loader.ts— Validatesearch_pathis PostgreSQL-only and non-emptydocs/config/toml.mdx— Addsearch_pathsection and quick reference entrydbhub.toml.example— Add documented example and quick referencesrc/config/__tests__/toml-loader.test.ts— 5 new validation testssrc/connectors/__tests__/postgres.integration.test.ts— Integration tests for search_path including special character schema namesUsage
Test plan
pnpm run build)"My Schema")🤖 Generated with Claude Code