Revert "Add support for search_path in PostgreSQL DSN and TOML config for non-public schemas (#244)"

tianzhou · tianzhou · commit a30c3b106733 · 2026-02-22T20:19:22.000-08:00
This reverts commit 8ecb7bd.
diff --git a/dbhub.toml.example b/dbhub.toml.example
@@ -32,17 +32,6 @@ dsn = "postgres://postgres:postgres@localhost:5432/myapp"
 # password = "p@ss:word/123"
 # sslmode = "disable"  # Optional: "disable" or "require"
 
-# PostgreSQL with non-public schema (individual parameters)
-# [[sources]]
-# id = "schema_pg"
-# type = "postgres"
-# host = "localhost"
-# database = "mydb"
-# user = "user"
-# password = "pass"
-# search_path = "myschema"          # Sets session search_path and default schema for discovery
-# # search_path = "myschema,public" # Comma-separated: first schema is used as discovery default
-
 # Development PostgreSQL (shared dev server)
 # [[sources]]
 # id = "dev_pg"
diff --git a/src/config/__tests__/toml-loader.test.ts b/src/config/__tests__/toml-loader.test.ts
@@ -1048,53 +1048,6 @@ query_timeout = 120
 
       expect(dsn).toBe('postgres://user:pass@localhost:9999/db');
     });
-
-    it('should append search_path param for PostgreSQL sources', () => {
-      const source: SourceConfig = {
-        id: 'test',
-        type: 'postgres',
-        host: 'localhost',
-        database: 'testdb',
-        user: 'testuser',
-        password: 'testpass',
-        search_path: 'myschema',
-      };
-
-      const dsn = buildDSNFromSource(source);
-
-      expect(dsn).toBe('postgres://testuser:testpass@localhost:5432/testdb?search_path=myschema');
-    });
-
-    it('should URL-encode comma-separated search_path values', () => {
-      const source: SourceConfig = {
-        id: 'test',
-        type: 'postgres',
-        host: 'localhost',
-        database: 'testdb',
-        user: 'testuser',
-        password: 'testpass',
-        search_path: 'myschema,public',
-      };
-
-      const dsn = buildDSNFromSource(source);
-
-      expect(dsn).toBe('postgres://testuser:testpass@localhost:5432/testdb?search_path=myschema%2Cpublic');
-    });
-
-    it('should not add search_path param when not specified', () => {
-      const source: SourceConfig = {
-        id: 'test',
-        type: 'postgres',
-        host: 'localhost',
-        database: 'testdb',
-        user: 'testuser',
-        password: 'testpass',
-      };
-
-      const dsn = buildDSNFromSource(source);
-
-      expect(dsn).not.toContain('search_path');
-    });
   });
 
   describe('Integration scenarios', () => {
diff --git a/src/config/toml-loader.ts b/src/config/toml-loader.ts
@@ -5,21 +5,13 @@ import toml from "@iarna/toml";
 import type { SourceConfig, TomlConfig, ToolConfig } from "../types/config.js";
 import { parseCommandLineArgs } from "./env.js";
 import { parseConnectionInfoFromDSN, getDefaultPortForType } from "../utils/dsn-obfuscate.js";
-import {
-  BUILTIN_TOOLS,
-  BUILTIN_TOOL_EXECUTE_SQL,
-  BUILTIN_TOOL_SEARCH_OBJECTS,
-} from "../tools/builtin-tools.js";
+import { BUILTIN_TOOLS, BUILTIN_TOOL_EXECUTE_SQL, BUILTIN_TOOL_SEARCH_OBJECTS } from "../tools/builtin-tools.js";
 
 /**
  * Load and parse TOML configuration file
  * Returns the parsed sources array, tools array, and the source of the config file
  */
-export function loadTomlConfig(): {
-  sources: SourceConfig[];
-  tools?: TomlConfig["tools"];
-  source: string;
-} | null {
+export function loadTomlConfig(): { sources: SourceConfig[]; tools?: TomlConfig['tools']; source: string } | null {
   const configPath = resolveTomlConfigPath();
   if (!configPath) {
     return null;
@@ -48,7 +40,9 @@ export function loadTomlConfig(): {
     };
   } catch (error) {
     if (error instanceof Error) {
-      throw new Error(`Failed to load TOML configuration from ${configPath}: ${error.message}`);
+      throw new Error(
+        `Failed to load TOML configuration from ${configPath}: ${error.message}`
+      );
     }
     throw error;
   }
@@ -65,7 +59,9 @@ function resolveTomlConfigPath(): string | null {
   if (args.config) {
     const configPath = expandHomeDir(args.config);
     if (!fs.existsSync(configPath)) {
-      throw new Error(`Configuration file specified by --config flag not found: ${configPath}`);
+      throw new Error(
+        `Configuration file specified by --config flag not found: ${configPath}`
+      );
     }
     return configPath;
   }
@@ -150,7 +146,9 @@ function validateToolsConfig(
 
   for (const tool of tools) {
     if (!tool.name) {
-      throw new Error(`Configuration file ${configPath}: all tools must have a 'name' field`);
+      throw new Error(
+        `Configuration file ${configPath}: all tools must have a 'name' field`
+      );
     }
 
     if (!tool.source) {
@@ -270,7 +268,11 @@ function validateSourceConfig(source: SourceConfig, configPath: string): void {
 
   // Validate SSH port if provided
   if (source.ssh_port !== undefined) {
-    if (typeof source.ssh_port !== "number" || source.ssh_port <= 0 || source.ssh_port > 65535) {
+    if (
+      typeof source.ssh_port !== "number" ||
+      source.ssh_port <= 0 ||
+      source.ssh_port > 65535
+    ) {
       throw new Error(
         `Configuration file ${configPath}: source '${source.id}' has invalid ssh_port. ` +
           `Must be between 1 and 65535.`
@@ -365,7 +367,10 @@ function validateSourceConfig(source: SourceConfig, configPath: string): void {
 /**
  * Process source configurations (expand paths, populate fields from DSN)
  */
-function processSourceConfigs(sources: SourceConfig[], configPath: string): SourceConfig[] {
+function processSourceConfigs(
+  sources: SourceConfig[],
+  configPath: string
+): SourceConfig[] {
   return sources.map((source) => {
     const processed = { ...source };
 
@@ -427,20 +432,24 @@ function expandHomeDir(filePath: string): string {
  * Similar to buildDSNFromEnvParams in env.ts but for TOML sources
  */
 export function buildDSNFromSource(source: SourceConfig): string {
-  // If DSN is already provided, use it directly
+  // If DSN is already provided, use it
   if (source.dsn) {
     return source.dsn;
   }
 
   // Validate required fields
   if (!source.type) {
-    throw new Error(`Source '${source.id}': 'type' field is required when 'dsn' is not provided`);
+    throw new Error(
+      `Source '${source.id}': 'type' field is required when 'dsn' is not provided`
+    );
   }
 
   // Handle SQLite
   if (source.type === "sqlite") {
     if (!source.database) {
-      throw new Error(`Source '${source.id}': 'database' field is required for SQLite`);
+      throw new Error(
+        `Source '${source.id}': 'database' field is required for SQLite`
+      );
     }
     return `sqlite:///${source.database}`;
   }
@@ -492,13 +501,6 @@ export function buildDSNFromSource(source: SourceConfig): string {
     }
   }
 
-  // Add PostgreSQL-specific parameters
-  if (source.type === "postgres") {
-    if (source.search_path) {
-      queryParams.push(`search_path=${encodeURIComponent(source.search_path)}`);
-    }
-  }
-
   // Add sslmode for network databases (not sqlite)
   if (source.sslmode && source.type !== "sqlite") {
     queryParams.push(`sslmode=${source.sslmode}`);
diff --git a/src/connectors/__tests__/postgres.integration.test.ts b/src/connectors/__tests__/postgres.integration.test.ts
@@ -462,113 +462,6 @@ describe('PostgreSQL Connector Integration Tests', () => {
 
   });
 
-  describe('search_path support', () => {
-    it('should discover tables in non-public schema by default when search_path is set', async () => {
-      // Create a non-public schema with a table
-      await postgresTest.connector.executeSQL('CREATE SCHEMA IF NOT EXISTS notpublic', {});
-      await postgresTest.connector.executeSQL(`
-        CREATE TABLE IF NOT EXISTS notpublic.widgets (
-          id SERIAL PRIMARY KEY,
-          name VARCHAR(100) NOT NULL
-        )
-      `, {});
-
-      const baseUri = postgresTest.connectionString;
-      const uriWithSearchPath = baseUri.includes('?')
-        ? `${baseUri}&search_path=notpublic`
-        : `${baseUri}?search_path=notpublic`;
-
-      const connector = new PostgresConnector();
-      try {
-        await connector.connect(uriWithSearchPath);
-
-        // getTables() without explicit schema should use notpublic (the defaultSchema)
-        const tables = await connector.getTables();
-        expect(tables).toContain('widgets');
-        // Should NOT contain tables from public schema
-        expect(tables).not.toContain('users');
-        expect(tables).not.toContain('orders');
-      } finally {
-        await connector.disconnect();
-      }
-    });
-
-    it('should use first schema from comma-separated search_path as discovery default', async () => {
-      await postgresTest.connector.executeSQL('CREATE SCHEMA IF NOT EXISTS notpublic', {});
-      await postgresTest.connector.executeSQL(`
-        CREATE TABLE IF NOT EXISTS notpublic.widgets (
-          id SERIAL PRIMARY KEY,
-          name VARCHAR(100) NOT NULL
-        )
-      `, {});
-
-      const baseUri = postgresTest.connectionString;
-      const uriWithSearchPath = baseUri.includes('?')
-        ? `${baseUri}&search_path=notpublic,public`
-        : `${baseUri}?search_path=notpublic,public`;
-
-      const connector = new PostgresConnector();
-      try {
-        await connector.connect(uriWithSearchPath);
-
-        // First schema in the list (notpublic) should be the default
-        const tables = await connector.getTables();
-        expect(tables).toContain('widgets');
-        expect(tables).not.toContain('users');
-      } finally {
-        await connector.disconnect();
-      }
-    });
-
-    it('should allow explicit schema parameter to override defaultSchema', async () => {
-      await postgresTest.connector.executeSQL('CREATE SCHEMA IF NOT EXISTS notpublic', {});
-      await postgresTest.connector.executeSQL(`
-        CREATE TABLE IF NOT EXISTS notpublic.widgets (
-          id SERIAL PRIMARY KEY,
-          name VARCHAR(100) NOT NULL
-        )
-      `, {});
-
-      const baseUri = postgresTest.connectionString;
-      const uriWithSearchPath = baseUri.includes('?')
-        ? `${baseUri}&search_path=notpublic`
-        : `${baseUri}?search_path=notpublic`;
-
-      const connector = new PostgresConnector();
-      try {
-        await connector.connect(uriWithSearchPath);
-
-        // Explicit schema='public' should override defaultSchema
-        const publicTables = await connector.getTables('public');
-        expect(publicTables).toContain('users');
-        expect(publicTables).toContain('orders');
-        expect(publicTables).not.toContain('widgets');
-      } finally {
-        await connector.disconnect();
-      }
-    });
-
-    it('should set search_path session variable after connecting', async () => {
-      await postgresTest.connector.executeSQL('CREATE SCHEMA IF NOT EXISTS notpublic', {});
-
-      const baseUri = postgresTest.connectionString;
-      const uriWithSearchPath = baseUri.includes('?')
-        ? `${baseUri}&search_path=notpublic`
-        : `${baseUri}?search_path=notpublic`;
-
-      const connector = new PostgresConnector();
-      try {
-        await connector.connect(uriWithSearchPath);
-
-        const result = await connector.executeSQL('SHOW search_path', {});
-        expect(result.rows).toHaveLength(1);
-        expect(result.rows[0].search_path).toContain('notpublic');
-      } finally {
-        await connector.disconnect();
-      }
-    });
-  });
-
   describe('SDK-Level Readonly Mode Tests', () => {
     it('should set default_transaction_read_only at connection level', async () => {
       const readonlyConnector = new PostgresConnector();
diff --git a/src/connectors/postgres/index.ts b/src/connectors/postgres/index.ts
diff --git a/src/types/config.ts b/src/types/config.ts
