From cff1e7389e3cdd815a3cd451e0a70bf1f11d140b Mon Sep 17 00:00:00 2001 From: Krishna Sasank Talasila <606482+geekypunk@users.noreply.github.com> Date: Sun, 9 Aug 2026 17:29:32 +0000 Subject: [PATCH 1/5] fix(postgres): introspect the session search_path, not a hardcoded 'public' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every catalog query in PostgresIntrospectionProvider filtered on the literal 'public', and the Java side tagged every discovered object with a DEFAULT_SCHEMA constant of the same value. Any database that keeps its tables elsewhere was therefore completely invisible to DeepSQL. The failure was silent and looked like success. On a dbt warehouse whose 37 tables live in `marts` and whose `public` holds nothing but extension views, connection init ran all nine stages in 1.1 seconds and wrote "All set! Brain is ready." at 100% — having produced an empty schema snapshot ("tables":[], totalTables 0), zero rag_documents, zero column profiles and zero semantic models. Nothing errored, because nothing was found to process. All 16 predicates now filter on current_schema(), and the schema tags come from a resolveSchema(Connection) helper reading the same value, so the target schema becomes a property of the connection — set it on the role, or with the JDBC currentSchema parameter — instead of a compile-time constant. It falls back to 'public' when current_schema() cannot be read, so existing connections behave exactly as before. getDefaultSchema() deliberately keeps returning 'public'. Its four callers (SchemaIntrospectionService, PgVectorSearchService, AzureSearchService, TrainingService) use it to decide whether a name needs qualifying; returning the session schema there would strip the qualifier off precisely the names that need it, turning marts.dim_person into a bare dim_person. Verified against a live 25GB warehouse: the same connection that previously snapshotted 0 tables now snapshots 37 tables and 837 columns, and init proceeds through real data sampling instead of completing instantly. Co-Authored-By: Claude Opus 5 (1M context) --- .../PostgresIntrospectionProvider.java | 101 +++++++++++++----- 1 file changed, 73 insertions(+), 28 deletions(-) diff --git a/backend/src/main/java/com/dbaagent/provider/postgres/PostgresIntrospectionProvider.java b/backend/src/main/java/com/dbaagent/provider/postgres/PostgresIntrospectionProvider.java index 592bf8b..4b298e7 100644 --- a/backend/src/main/java/com/dbaagent/provider/postgres/PostgresIntrospectionProvider.java +++ b/backend/src/main/java/com/dbaagent/provider/postgres/PostgresIntrospectionProvider.java @@ -12,6 +12,20 @@ /** * PostgreSQL implementation of IntrospectionProvider. * Uses pg_catalog and information_schema for schema introspection. + * + *

Every catalog query filters on {@code current_schema()} rather than a + * hardcoded {@code 'public'}. Databases that keep their tables anywhere else — + * a dbt warehouse in {@code marts}, a tenant schema, anything — were previously + * invisible: introspection returned zero tables, so brain initialization + * "COMPLETED" in about a second having learned nothing, and the schema snapshot + * was persisted as an empty table list. Honouring the session search_path makes + * the target schema a property of the connection (set it on the role, or via the + * JDBC {@code currentSchema} parameter) instead of a compile-time constant. + * + *

{@link #getDefaultSchema()} deliberately still reports {@code public}: its + * callers use it to decide whether a name needs qualifying, and reporting the + * session schema there would strip the schema off exactly the names that need + * it most ({@code marts.dim_person} → {@code dim_person}). */ @Slf4j @Component @@ -55,25 +69,27 @@ private List getTablesAndViews(Connection connection) throws SQL JOIN pg_namespace n ON n.nspname = t.schemaname JOIN pg_class c ON c.relnamespace = n.oid AND c.relname = t.tablename LEFT JOIN pg_stat_all_tables s ON s.relid = c.oid - WHERE t.schemaname = 'public' AND c.relkind IN ('r', 'p') + WHERE t.schemaname = current_schema() AND c.relkind IN ('r', 'p') UNION ALL SELECT v.viewname as name, 'view' as type, 0 as row_count - FROM pg_views v WHERE v.schemaname = 'public' + FROM pg_views v WHERE v.schemaname = current_schema() ORDER BY type, name """; + String schema = resolveSchema(connection); + try (Statement stmt = connection.createStatement(); ResultSet rs = stmt.executeQuery(query)) { while (rs.next()) { DatabaseObject obj = new DatabaseObject(); obj.setName(rs.getString("name")); - obj.setSchema(DEFAULT_SCHEMA); + obj.setSchema(schema); obj.setType(rs.getString("type")); Long estimatedRowCount = getNullableLong(rs, "row_count"); obj.setRowCount("table".equals(obj.getType()) - ? resolveTableRowCount(connection, DEFAULT_SCHEMA, obj.getName(), estimatedRowCount) + ? resolveTableRowCount(connection, schema, obj.getName(), estimatedRowCount) : estimatedRowCount); - obj.setColumns(getTableColumns(connection, DEFAULT_SCHEMA, obj.getName())); + obj.setColumns(getTableColumns(connection, schema, obj.getName())); objects.add(obj); } } @@ -87,16 +103,18 @@ private List getFunctions(Connection connection) throws SQLExcep SELECT p.proname as name, pg_get_functiondef(p.oid) as definition FROM pg_proc p JOIN pg_namespace n ON p.pronamespace = n.oid - WHERE n.nspname = 'public' AND p.prokind = 'f' + WHERE n.nspname = current_schema() AND p.prokind = 'f' ORDER BY p.proname """; + String schema = resolveSchema(connection); + try (Statement stmt = connection.createStatement(); ResultSet rs = stmt.executeQuery(query)) { while (rs.next()) { DatabaseObject obj = new DatabaseObject(); obj.setName(rs.getString("name")); - obj.setSchema(DEFAULT_SCHEMA); + obj.setSchema(schema); obj.setType("function"); obj.setDefinition(rs.getString("definition")); objects.add(obj); @@ -112,16 +130,18 @@ private List getProcedures(Connection connection) throws SQLExce SELECT p.proname as name, pg_get_functiondef(p.oid) as definition FROM pg_proc p JOIN pg_namespace n ON p.pronamespace = n.oid - WHERE n.nspname = 'public' AND p.prokind = 'p' + WHERE n.nspname = current_schema() AND p.prokind = 'p' ORDER BY p.proname """; + String schema = resolveSchema(connection); + try (Statement stmt = connection.createStatement(); ResultSet rs = stmt.executeQuery(query)) { while (rs.next()) { DatabaseObject obj = new DatabaseObject(); obj.setName(rs.getString("name")); - obj.setSchema(DEFAULT_SCHEMA); + obj.setSchema(schema); obj.setType("procedure"); obj.setDefinition(rs.getString("definition")); objects.add(obj); @@ -151,7 +171,7 @@ LEFT JOIN ( try (PreparedStatement stmt = connection.prepareStatement(query)) { stmt.setString(1, tableName); stmt.setString(2, tableName); - stmt.setString(3, DEFAULT_SCHEMA); + stmt.setString(3, resolveSchema(connection)); try (ResultSet rs = stmt.executeQuery()) { while (rs.next()) { ColumnInfo col = new ColumnInfo(); @@ -257,11 +277,12 @@ WITH t AS (SELECT ?::regclass AS rel) stats.setIndexSize(rs.getLong("index_bytes")); stats.setSizeBytes(rs.getLong("total_bytes")); stats.setIndexSizeBytes(rs.getLong("index_bytes")); + String schema = resolveSchema(connection); stats.setRowCount(resolveTableRowCount( connection, - DEFAULT_SCHEMA, + schema, tableName, - getEstimatedTableRowCount(connection, DEFAULT_SCHEMA, tableName) + getEstimatedTableRowCount(connection, schema, tableName) )); } } @@ -288,14 +309,15 @@ public SchemaMetadata scanSchema(Connection connection, String database) throws "JOIN pg_namespace n ON n.nspname = t.schemaname " + "JOIN pg_class c ON c.relnamespace = n.oid AND c.relname = t.tablename " + "LEFT JOIN pg_stat_all_tables s ON s.relid = c.oid " + - "WHERE t.schemaname = 'public' AND c.relkind IN ('r', 'p') " + + "WHERE t.schemaname = current_schema() AND c.relkind IN ('r', 'p') " + "UNION ALL " + "SELECT v.viewname as tablename, 'view' as type, 0 as size_bytes, 0 as row_count " + "FROM pg_views v " + - "WHERE v.schemaname = 'public' " + + "WHERE v.schemaname = current_schema() " + "ORDER BY tablename"; Map tableMap = new HashMap<>(); + String schemaName = resolveSchema(connection); try (Statement stmt = connection.createStatement()) { applyStatementSettings(stmt); @@ -303,12 +325,12 @@ public SchemaMetadata scanSchema(Connection connection, String database) throws while (rs.next()) { TableMetadata table = new TableMetadata(); table.setName(rs.getString("tablename")); - table.setSchema(DEFAULT_SCHEMA); + table.setSchema(schemaName); table.setType(rs.getString("type")); table.setSizeBytes(rs.getLong("size_bytes")); Long estimatedRowCount = getNullableLong(rs, "row_count"); table.setRowCount("table".equals(table.getType()) - ? resolveTableRowCount(connection, DEFAULT_SCHEMA, table.getName(), estimatedRowCount) + ? resolveTableRowCount(connection, schemaName, table.getName(), estimatedRowCount) : estimatedRowCount); schema.getTables().add(table); tableMap.put(table.getName(), table); @@ -349,9 +371,9 @@ private void scanPostgreSQLColumnsBatch(Connection connection, Map getForeignKeys(Connection connection, String d JOIN information_schema.constraint_column_usage ccu ON tc.constraint_name = ccu.constraint_name WHERE tc.constraint_type = 'FOREIGN KEY' - AND tc.table_schema = 'public' + AND tc.table_schema = current_schema() ORDER BY tc.table_name, tc.constraint_name """; @@ -502,7 +524,7 @@ public List getColumnDetails(Connection connection, String databas data_type, character_maximum_length, numeric_precision, numeric_scale, udt_name FROM information_schema.columns - WHERE table_schema = 'public' AND table_name = ? + WHERE table_schema = current_schema() AND table_name = ? ORDER BY ordinal_position """; @@ -547,7 +569,7 @@ public List getConstraintDetails(Connection connection, String LEFT JOIN information_schema.constraint_column_usage ccu ON tc.constraint_name = ccu.constraint_name AND tc.constraint_type = 'FOREIGN KEY' - WHERE tc.table_schema = 'public' AND tc.table_name = ? + WHERE tc.table_schema = current_schema() AND tc.table_name = ? ORDER BY tc.constraint_name """; @@ -582,11 +604,12 @@ public List getConstraintDetails(Connection connection, String @Override public Long getTableRowCount(Connection connection, String database, String tableName) throws SQLException { + String schema = resolveSchema(connection); return resolveTableRowCount( connection, - DEFAULT_SCHEMA, + schema, tableName, - getEstimatedTableRowCount(connection, DEFAULT_SCHEMA, tableName) + getEstimatedTableRowCount(connection, schema, tableName) ); } @@ -763,6 +786,28 @@ private Long getExactTableRowCount(Connection connection, String schemaName, Str return null; } + /** + * The schema this session's catalog queries resolve against — the first + * existing entry in the search_path. Falls back to {@code public} so a + * connection whose search_path names only missing schemas behaves exactly + * as it did before, rather than tagging every object with a null schema. + */ + private String resolveSchema(Connection connection) { + try (Statement stmt = connection.createStatement(); + ResultSet rs = stmt.executeQuery("SELECT current_schema()")) { + if (rs.next()) { + String schema = rs.getString(1); + if (schema != null && !schema.isBlank()) { + return schema; + } + } + } catch (SQLException e) { + log.debug("Could not resolve current_schema(), falling back to {}: {}", + DEFAULT_SCHEMA, e.getMessage()); + } + return DEFAULT_SCHEMA; + } + private String quoteIdentifier(String identifier) { return "\"" + identifier.replace("\"", "\"\"") + "\""; } @@ -802,7 +847,7 @@ public List> getAllTablesWithMetadata(Connection connection, JOIN pg_namespace n ON n.nspname = t.schemaname JOIN pg_class c ON c.relnamespace = n.oid AND c.relname = t.tablename LEFT JOIN pg_stat_all_tables s ON s.relid = c.oid - WHERE t.schemaname = 'public' AND c.relkind IN ('r', 'p') + WHERE t.schemaname = current_schema() AND c.relkind IN ('r', 'p') ORDER BY t.tablename """; From f5196eb94eead00780d941640c7e82749258d970 Mon Sep 17 00:00:00 2001 From: geekypunk Date: Sun, 9 Aug 2026 14:58:23 -0500 Subject: [PATCH 2/5] fix(postgres): make a schema switch visible instead of silent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Postgres ships `search_path = "$user", public` everywhere, RDS and Aurora included, and the "$user" entry is inert only while no schema matches the connecting role's name. Create one — per-tenant layouts, or the per-user pattern the Postgres docs recommend and which spread after PG15 hardened `public` — and current_schema() silently becomes that schema. Verified on a stock instance: SHOW search_path; -> "$user", public SELECT current_schema(); -> public CREATE SCHEMA postgres; -- schema named after the connecting role SELECT current_schema(); -> postgres So a connection that had been reading `public` can start reading an empty user schema and report a healthy, empty brain — the exact failure the search_path change was written to fix, inverted. resolveSchema()'s fallback does not catch it, because current_schema() returned a perfectly valid schema. Two guards, both diagnostic only: - announceSchema() logs the schema at INFO when it is not `public`, and DEBUG when it is, so the unchanged historical case stays quiet while a switch is always on the record. Called from getDatabaseObjects and scanSchema rather than from resolveSchema, which runs once per table via getTableColumns — a log there would emit a line per table. - warnIfEmptyWhilePublicHasTables() fires on the fingerprint of an accidental "$user" match: nothing found in the resolved schema while `public`, where this provider used to look unconditionally, still holds tables. It names the ALTER ROLE ... SET search_path remedy. Best-effort; a failed count never breaks introspection. Default RDS/Aurora installs are unaffected either way: with no role-named schema current_schema() is `public`, exactly as before. --- .../PostgresIntrospectionProvider.java | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/backend/src/main/java/com/dbaagent/provider/postgres/PostgresIntrospectionProvider.java b/backend/src/main/java/com/dbaagent/provider/postgres/PostgresIntrospectionProvider.java index 4b298e7..05a9352 100644 --- a/backend/src/main/java/com/dbaagent/provider/postgres/PostgresIntrospectionProvider.java +++ b/backend/src/main/java/com/dbaagent/provider/postgres/PostgresIntrospectionProvider.java @@ -77,6 +77,7 @@ private List getTablesAndViews(Connection connection) throws SQL """; String schema = resolveSchema(connection); + announceSchema(schema); try (Statement stmt = connection.createStatement(); ResultSet rs = stmt.executeQuery(query)) { @@ -93,6 +94,7 @@ private List getTablesAndViews(Connection connection) throws SQL objects.add(obj); } } + warnIfEmptyWhilePublicHasTables(connection, schema, objects.size()); return objects; } @@ -318,6 +320,7 @@ public SchemaMetadata scanSchema(Connection connection, String database) throws Map tableMap = new HashMap<>(); String schemaName = resolveSchema(connection); + announceSchema(schemaName); try (Statement stmt = connection.createStatement()) { applyStatementSettings(stmt); @@ -338,6 +341,8 @@ public SchemaMetadata scanSchema(Connection connection, String database) throws } } + warnIfEmptyWhilePublicHasTables(connection, schemaName, schema.getTables().size()); + // Batch load all columns and indexes in single queries (eliminates N+1) scanPostgreSQLColumnsBatch(connection, tableMap); scanPostgreSQLIndexesBatch(connection, tableMap); @@ -808,6 +813,65 @@ private String resolveSchema(Connection connection) { return DEFAULT_SCHEMA; } + /** + * Say which schema this pass is reading, so a switch is never silent. + * + *

Postgres ships {@code search_path = "$user", public} everywhere, RDS and + * Aurora included. The {@code "$user"} entry is inert only while no schema + * matches the connecting role's name — create one (per-tenant layouts, or the + * per-user pattern the Postgres docs recommend and which spread after PG15 + * hardened {@code public}) and {@code current_schema()} silently becomes that + * schema. A connection that had been reading {@code public} would then read an + * empty user schema and report a healthy, empty brain: the very failure this + * class was changed to fix, inverted. + * + *

Logged at INFO only when it is not {@code public}, because {@code public} + * is the unchanged historical case and every connection would otherwise emit a + * line per introspection pass. + * + *

Called from the two pass-level entry points rather than from + * {@link #resolveSchema}, which runs once per table via + * {@link #getTableColumns} — logging there would produce one line per table. + */ + private void announceSchema(String schema) { + if (!DEFAULT_SCHEMA.equals(schema)) { + log.info("Introspecting schema '{}' (from the session search_path, not '{}')", + schema, DEFAULT_SCHEMA); + } else { + log.debug("Introspecting schema '{}'", schema); + } + } + + /** + * Warn on the fingerprint of an accidental schema switch: nothing found here, + * while {@code public} — where this provider used to look unconditionally — + * still holds tables. + * + *

Without this the outcome is a successful-looking run over an empty schema. + * Best-effort: a failure to count is never allowed to break introspection. + */ + private void warnIfEmptyWhilePublicHasTables(Connection connection, String schema, int found) { + if (found > 0 || DEFAULT_SCHEMA.equals(schema)) { + return; + } + try (PreparedStatement stmt = connection.prepareStatement( + "SELECT count(*) FROM pg_tables WHERE schemaname = ?")) { + stmt.setString(1, DEFAULT_SCHEMA); + try (ResultSet rs = stmt.executeQuery()) { + if (rs.next() && rs.getLong(1) > 0) { + log.warn("Schema '{}' contains no tables, but '{}' has {}. The session " + + "search_path resolves to '{}' — if that is not intended, check for a " + + "schema named after the connecting role (search_path starts with " + + "\"$user\"), or set it explicitly: " + + "ALTER ROLE IN DATABASE SET search_path = , public;", + schema, DEFAULT_SCHEMA, rs.getLong(1), schema); + } + } + } catch (SQLException e) { + log.debug("Could not compare '{}' against '{}': {}", schema, DEFAULT_SCHEMA, e.getMessage()); + } + } + private String quoteIdentifier(String identifier) { return "\"" + identifier.replace("\"", "\"\"") + "\""; } From 2a23bc4b79d86b979c444c99a2ef132c1f10c528 Mon Sep 17 00:00:00 2001 From: geekypunk Date: Sun, 9 Aug 2026 14:58:33 -0500 Subject: [PATCH 3/5] test(postgres): stub current_schema() so the provider fixtures pass again resolveSchema() issues `SELECT current_schema()` on its own Statement before each method's real query. Seven tests predated that call and broke: those stubbing only prepareStatement got the unstubbed-mock default null from createStatement() NullPointerException: Cannot invoke "java.sql.Statement.executeQuery(String)" because "stmt" is null at resolveSchema(...:797) and those stubbing createStatement handed resolveSchema the shared ResultSet, so its getString(1) tripped strict stubbing against getString("tablename"). resolveSchema() is the first createStatement() caller in every method that reaches the database, so returning a dedicated schemaStatement first and the shared statement afterwards routes each to the right place. It answers "public", keeping these fixtures on the historical schema so they go on asserting the behaviour they were written for rather than the search_path change itself. setUp() uses lenient() because the pure-Java tests (getDatabaseType, getDefaultSchema) never touch the Connection and strict stubbing would fail them over an unused stub. Three tests re-stub createStatement locally and had to prepend schemaStatement themselves; getForeignKeys_returnsRelationships is left alone because getForeignKeys does not call resolveSchema. 11/11 pass. --- .../PostgresIntrospectionProviderTest.java | 31 +++++++++++++++++-- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/backend/src/test/java/com/dbaagent/provider/postgres/PostgresIntrospectionProviderTest.java b/backend/src/test/java/com/dbaagent/provider/postgres/PostgresIntrospectionProviderTest.java index 2e7ea5c..497be80 100644 --- a/backend/src/test/java/com/dbaagent/provider/postgres/PostgresIntrospectionProviderTest.java +++ b/backend/src/test/java/com/dbaagent/provider/postgres/PostgresIntrospectionProviderTest.java @@ -35,9 +35,31 @@ class PostgresIntrospectionProviderTest { @Mock private ResultSet resultSet; + // resolveSchema() issues `SELECT current_schema()` on its own Statement before + // any method's real query. These mocks answer that call so the fixtures below + // keep testing what they were written to test. + @Mock + private Statement schemaStatement; + + @Mock + private ResultSet schemaResultSet; + @BeforeEach - void setUp() { + void setUp() throws SQLException { provider = new PostgresIntrospectionProvider(); + + // lenient(): the pure-Java tests (getDatabaseType, getDefaultSchema, …) never + // touch the Connection, and strict stubbing would fail them over an unused stub. + // + // resolveSchema() is the FIRST createStatement() caller in every method that + // reaches the database, so returning schemaStatement first and the shared + // statement afterwards routes each to the right place. Answering "public" + // keeps these fixtures on the historical schema, so they go on asserting the + // behaviour they were written for rather than the search_path change itself. + lenient().when(connection.createStatement()).thenReturn(schemaStatement, statement); + lenient().when(schemaStatement.executeQuery(anyString())).thenReturn(schemaResultSet); + lenient().when(schemaResultSet.next()).thenReturn(true); + lenient().when(schemaResultSet.getString(1)).thenReturn("public"); } @Test @@ -47,7 +69,8 @@ void getDatabaseType_returnsPostgres() { @Test void getDatabaseObjects_returnsTables() throws SQLException { - when(connection.createStatement()).thenReturn(statement); + // schemaStatement first: resolveSchema() runs before the objects query. + when(connection.createStatement()).thenReturn(schemaStatement, statement); when(statement.executeQuery(anyString())).thenReturn(resultSet); when(connection.prepareStatement(anyString())).thenReturn(preparedStatement); when(preparedStatement.executeQuery()).thenReturn(resultSet); @@ -209,7 +232,8 @@ void getTableStats_bindsEveryPlaceholderInTheStatsQuery() throws SQLException { @Test void scanSchema_returnsSchemaMetadata() throws SQLException { - when(connection.createStatement()).thenReturn(statement); + // schemaStatement first: resolveSchema() runs before the tables query. + when(connection.createStatement()).thenReturn(schemaStatement, statement); when(statement.executeQuery(anyString())).thenReturn(resultSet); when(resultSet.next()) @@ -281,6 +305,7 @@ void scanSchema_fallsBackToExactCountWhenEstimateMissing() throws SQLException { ResultSet foreignKeysResultSet = mock(ResultSet.class); when(connection.createStatement()).thenReturn( + schemaStatement, // resolveSchema() runs before the tables query statement, exactCountStatement, columnsStatement, From f63eea0158a2c7d3fcb74dc09315b72b439bbbdb Mon Sep 17 00:00:00 2001 From: geekypunk Date: Sun, 9 Aug 2026 15:06:48 -0500 Subject: [PATCH 4/5] fix(postgres): schema-qualify the primary-key subquery in getTableColumns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PK subquery joined tc.constraint_name = ku.constraint_name and filtered on ku.table_name alone, with no schema predicate anywhere. Postgres auto-names primary keys "_pkey", so two schemas holding a same-named table collide by construction — the join cross-matches them. Measured against two schemas each with an `orders` table (s_a's PK is `other`, s_b's is `name`), asking for s_b.orders: column_name | is_primary_key name | t name | t <- duplicated row other | t <- false positive: `other` is s_a's PK, not s_b's other | t <- duplicated row Ground truth from pg_index: s_b.orders has exactly one PK column, `name`. So the provider reported every column as a primary key and returned each one twice — duplicate ColumnInfo entries, and a table with no real single PK. Adding tc.table_schema = ku.table_schema and ku.table_schema = ? returns the correct `name | t`, `other | f`. This predates the search_path change and was mostly latent while the provider only ever read `public`: a collision needed some other schema to hold a same-named table. Now that it targets whatever the session resolves to, that is no longer the exception — staging/marts/public each holding `orders` or `customers` is the normal shape of the dbt warehouse this change was made for. So the earlier commit does not introduce this bug, but it does move it from unlikely to expected, which is why it is fixed on the same branch. Also hoists resolveSchema() out of the argument list: this method runs once per table, and each call costs a round-trip. Tests: 12/12. A mocked ResultSet cannot exercise SQL semantics, so the new test asserts the two schema predicates are present and that every placeholder is bound — enough to stop the qualification being dropped again. The correctness evidence is the measurement above, run against Postgres 18. --- .../PostgresIntrospectionProvider.java | 36 ++++++++++++++++--- .../PostgresIntrospectionProviderTest.java | 32 +++++++++++++++++ 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/backend/src/main/java/com/dbaagent/provider/postgres/PostgresIntrospectionProvider.java b/backend/src/main/java/com/dbaagent/provider/postgres/PostgresIntrospectionProvider.java index 05a9352..43fa2bb 100644 --- a/backend/src/main/java/com/dbaagent/provider/postgres/PostgresIntrospectionProvider.java +++ b/backend/src/main/java/com/dbaagent/provider/postgres/PostgresIntrospectionProvider.java @@ -156,6 +156,26 @@ SELECT p.proname as name, pg_get_functiondef(p.oid) as definition public List getTableColumns(Connection connection, String database, String tableName) throws SQLException { List columns = new ArrayList<>(); + // The PK subquery must be schema-qualified on BOTH the constraint join and + // the table lookup. Without it, two schemas holding a same-named table + // collide by construction: Postgres auto-names primary keys + // "
_pkey", so `tc.constraint_name = ku.constraint_name` alone + // cross-joins the schemas. + // + // Measured against two schemas each holding an `orders` table, asking for + // s_b.orders whose only PK is `name`: + // name | t + // name | t <- duplicated row + // other | t <- false positive; `other` is s_a's PK, not s_b's + // other | t <- duplicated row + // i.e. every column reported as a primary key, and each one twice. With the + // predicates below the same query returns `name | t`, `other | f`. + // + // The bug predates the search_path change but was mostly latent while this + // provider only ever read `public`. Now that it targets whatever the session + // resolves to, same-named tables across schemas — the normal shape of a dbt + // warehouse (staging/marts/public each with `orders`) — are the expected + // case rather than the exception. String query = """ SELECT c.column_name, c.data_type, c.is_nullable, c.column_default, CASE WHEN pk.column_name IS NOT NULL THEN true ELSE false END as is_primary_key @@ -163,17 +183,25 @@ public List getTableColumns(Connection connection, String database, LEFT JOIN ( SELECT ku.column_name FROM information_schema.table_constraints tc - JOIN information_schema.key_column_usage ku ON tc.constraint_name = ku.constraint_name - WHERE tc.constraint_type = 'PRIMARY KEY' AND ku.table_name = ? + JOIN information_schema.key_column_usage ku + ON tc.constraint_name = ku.constraint_name + AND tc.table_schema = ku.table_schema + WHERE tc.constraint_type = 'PRIMARY KEY' + AND ku.table_name = ? AND ku.table_schema = ? ) pk ON c.column_name = pk.column_name WHERE c.table_name = ? AND c.table_schema = ? ORDER BY c.ordinal_position """; + // Resolved once: this method runs per table, and resolveSchema() costs a + // round-trip each call. + String schema = resolveSchema(connection); + try (PreparedStatement stmt = connection.prepareStatement(query)) { stmt.setString(1, tableName); - stmt.setString(2, tableName); - stmt.setString(3, resolveSchema(connection)); + stmt.setString(2, schema); + stmt.setString(3, tableName); + stmt.setString(4, schema); try (ResultSet rs = stmt.executeQuery()) { while (rs.next()) { ColumnInfo col = new ColumnInfo(); diff --git a/backend/src/test/java/com/dbaagent/provider/postgres/PostgresIntrospectionProviderTest.java b/backend/src/test/java/com/dbaagent/provider/postgres/PostgresIntrospectionProviderTest.java index 497be80..a5a4536 100644 --- a/backend/src/test/java/com/dbaagent/provider/postgres/PostgresIntrospectionProviderTest.java +++ b/backend/src/test/java/com/dbaagent/provider/postgres/PostgresIntrospectionProviderTest.java @@ -198,6 +198,38 @@ void getTableStats_returnsStats() throws SQLException { assertEquals(122880L, stats.getSizeBytes()); } + @Test + void getTableColumns_qualifiesThePrimaryKeySubqueryBySchema() throws SQLException { + // Postgres auto-names primary keys "
_pkey", so joining + // tc.constraint_name = ku.constraint_name WITHOUT a schema predicate + // cross-joins any two schemas holding a same-named table. Measured against + // two `orders` tables (s_a PK `other`, s_b PK `name`), asking for s_b: + // name|t name|t other|t other|t + // — every column a primary key, and each row duplicated. Schema-qualified + // it returns name|t, other|f. + // + // A mocked ResultSet cannot exercise SQL semantics, so this asserts the + // predicates are present and every placeholder is bound — enough to stop + // the qualification being dropped again. + ArgumentCaptor sqlCaptor = ArgumentCaptor.forClass(String.class); + when(connection.prepareStatement(anyString())).thenReturn(preparedStatement); + when(preparedStatement.executeQuery()).thenReturn(resultSet); + when(resultSet.next()).thenReturn(false); + + provider.getTableColumns(connection, "public", "orders"); + + verify(connection).prepareStatement(sqlCaptor.capture()); + String sql = sqlCaptor.getValue(); + + assertTrue(sql.contains("tc.table_schema = ku.table_schema"), + "the constraint join must be schema-qualified, or
_pkey collides across schemas"); + assertTrue(sql.contains("ku.table_schema = ?"), + "the PK lookup must be restricted to the target schema"); + + int placeholders = (int) sql.chars().filter(c -> c == '?').count(); + verify(preparedStatement, times(placeholders)).setString(anyInt(), anyString()); + } + @Test void getTableStats_bindsEveryPlaceholderInTheStatsQuery() throws SQLException { // The stats query carried NINE `?` placeholders (the two size subtractions use From 05ddf85ecbf78373c8c45c8f0e3d66928ff0c9d2 Mon Sep 17 00:00:00 2001 From: geekypunk Date: Sun, 9 Aug 2026 15:31:35 -0500 Subject: [PATCH 5/5] fix(postgres): only honour a search_path that was deliberately set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Honouring current_schema() unconditionally changed behaviour for every Postgres connection in order to fix the subset that keeps tables outside `public`. That is the wrong trade, because the risk is not hypothetical: Postgres ships `search_path = "$user", public` everywhere, RDS and Aurora included, and the leading "$user" is inert only while no schema matches the connecting role's name. SHOW search_path; -> "$user", public SELECT current_schema(); -> public CREATE SCHEMA postgres; -- named after the connecting role SELECT current_schema(); -> postgres Per-tenant layouts, and the per-user pattern the Postgres docs recommend and which spread after PG15 hardened `public`, make that live. An untouched connection would then read an empty user schema and report a healthy, empty brain — the failure this feature exists to fix, inverted. The warning added earlier makes that visible after the fact; it does not prevent it. resolveSchema() now reads search_path and current_user alongside current_schema(), and ignores a schema that was selected by the implicit "$user" entry: current_schema() equal to current_user while search_path is still exactly what Postgres ships. Such a connection introspects `public`, bit for bit as before. The blast radius is now the connections that asked for this. Nothing changes unless an operator sets a search_path — `ALTER ROLE … SET search_path`, or the JDBC currentSchema parameter — which is how the reported warehouse enabled it, so that case is unaffected. The check is deliberately narrow: it matches the shipped default exactly. A path of `"$user", marts` states an intent and is honoured, verified by test. No new configuration: DatabaseConnection carries no schema field, so a config route would mean entity, DTO, API and UI. The database-side search_path is already the opt-in. Tests: 16/16, covering the implicit "$user" match, a configured path, "$user" inside a non-default path, and a NULL current_schema(). --- .../PostgresIntrospectionProvider.java | 56 ++++++++++++++-- .../PostgresIntrospectionProviderTest.java | 65 +++++++++++++++++++ 2 files changed, 115 insertions(+), 6 deletions(-) diff --git a/backend/src/main/java/com/dbaagent/provider/postgres/PostgresIntrospectionProvider.java b/backend/src/main/java/com/dbaagent/provider/postgres/PostgresIntrospectionProvider.java index 43fa2bb..bddece3 100644 --- a/backend/src/main/java/com/dbaagent/provider/postgres/PostgresIntrospectionProvider.java +++ b/backend/src/main/java/com/dbaagent/provider/postgres/PostgresIntrospectionProvider.java @@ -821,18 +821,52 @@ private Long getExactTableRowCount(Connection connection, String schemaName, Str /** * The schema this session's catalog queries resolve against — the first - * existing entry in the search_path. Falls back to {@code public} so a - * connection whose search_path names only missing schemas behaves exactly - * as it did before, rather than tagging every object with a null schema. + * existing entry in the search_path — but only when that search_path was + * deliberately set. Falls back to {@code public} otherwise. + * + *

Two distinct fallbacks, for two distinct reasons: + * + *

    + *
  1. {@code current_schema()} is null when the search_path names only + * schemas that do not exist. Tagging every object with a null schema + * would be worse than the previous behaviour. + *
  2. The search_path is still the untouched Postgres default. Every + * Postgres ships {@code "$user", public} — RDS and Aurora included — and + * that leading {@code "$user"} is inert only while no schema matches the + * connecting role's name. Create one (per-tenant layouts, or the per-user + * pattern the Postgres docs recommend and which spread after PG15 + * hardened {@code public}) and {@code current_schema()} silently becomes + * that schema. A connection that had been reading {@code public} would + * start reading an empty user schema and report a healthy, empty brain — + * the very failure honouring search_path exists to fix, inverted. + *
+ * + *

The second case is detected rather than merely logged: an operator who has + * not touched search_path gets the historical {@code public} behaviour bit for + * bit, and only an explicit setting — {@code ALTER ROLE … SET search_path}, + * or the JDBC {@code currentSchema} parameter — moves this provider off it. + * That keeps the blast radius of this feature to connections that asked for it. + * + *

Deliberately narrow: the check is for the default search_path *exactly*. + * Someone who writes {@code "$user", marts} has stated an intent, and it is + * honoured. */ private String resolveSchema(Connection connection) { try (Statement stmt = connection.createStatement(); - ResultSet rs = stmt.executeQuery("SELECT current_schema()")) { + ResultSet rs = stmt.executeQuery( + "SELECT current_schema(), current_setting('search_path'), current_user")) { if (rs.next()) { String schema = rs.getString(1); - if (schema != null && !schema.isBlank()) { - return schema; + if (schema == null || schema.isBlank()) { + return DEFAULT_SCHEMA; + } + if (schema.equals(rs.getString(3)) && isUntouchedDefaultSearchPath(rs.getString(2))) { + log.debug("search_path is the Postgres default and '{}' matches the connecting " + + "role, so it was selected by the implicit \"$user\" entry rather than " + + "configured — introspecting '{}' as before", schema, DEFAULT_SCHEMA); + return DEFAULT_SCHEMA; } + return schema; } } catch (SQLException e) { log.debug("Could not resolve current_schema(), falling back to {}: {}", @@ -841,6 +875,16 @@ private String resolveSchema(Connection connection) { return DEFAULT_SCHEMA; } + /** + * True when search_path is exactly what Postgres ships, ignoring spacing. + * Anything else — including a reordered or extended path that still mentions + * {@code "$user"} — counts as configured and is honoured. + */ + private boolean isUntouchedDefaultSearchPath(String searchPath) { + return searchPath != null + && "\"$user\",public".equals(searchPath.replace(" ", "")); + } + /** * Say which schema this pass is reading, so a switch is never silent. * diff --git a/backend/src/test/java/com/dbaagent/provider/postgres/PostgresIntrospectionProviderTest.java b/backend/src/test/java/com/dbaagent/provider/postgres/PostgresIntrospectionProviderTest.java index a5a4536..31c9a92 100644 --- a/backend/src/test/java/com/dbaagent/provider/postgres/PostgresIntrospectionProviderTest.java +++ b/backend/src/test/java/com/dbaagent/provider/postgres/PostgresIntrospectionProviderTest.java @@ -59,7 +59,12 @@ void setUp() throws SQLException { lenient().when(connection.createStatement()).thenReturn(schemaStatement, statement); lenient().when(schemaStatement.executeQuery(anyString())).thenReturn(schemaResultSet); lenient().when(schemaResultSet.next()).thenReturn(true); + // resolveSchema() reads current_schema(), search_path, current_user. + // A schema that differs from the role is an ordinary resolution, so these + // fixtures land on "public" exactly as they did before search_path support. lenient().when(schemaResultSet.getString(1)).thenReturn("public"); + lenient().when(schemaResultSet.getString(2)).thenReturn("\"$user\", public"); + lenient().when(schemaResultSet.getString(3)).thenReturn("app_user"); } @Test @@ -198,6 +203,66 @@ void getTableStats_returnsStats() throws SQLException { assertEquals(122880L, stats.getSizeBytes()); } + /** Runs getTableColumns and returns the schema it bound (parameter 4). */ + private String schemaUsedByGetTableColumns() throws SQLException { + when(connection.prepareStatement(anyString())).thenReturn(preparedStatement); + when(preparedStatement.executeQuery()).thenReturn(resultSet); + when(resultSet.next()).thenReturn(false); + + provider.getTableColumns(connection, "db", "orders"); + + ArgumentCaptor bound = ArgumentCaptor.forClass(String.class); + verify(preparedStatement, atLeastOnce()).setString(eq(4), bound.capture()); + return bound.getValue(); + } + + @Test + void schemaChosenByTheImplicitDollarUserEntryIsIgnored() throws SQLException { + // Every Postgres ships search_path = "$user", public. That leading "$user" + // is inert only while no schema matches the connecting role — create one and + // current_schema() silently becomes it. Honouring that would move an + // untouched RDS/Aurora connection off `public` onto an empty user schema and + // report a healthy, empty brain. An operator who never configured a + // search_path must keep the historical behaviour exactly. + when(schemaResultSet.getString(1)).thenReturn("app_user"); // current_schema() + when(schemaResultSet.getString(2)).thenReturn("\"$user\", public"); // untouched default + when(schemaResultSet.getString(3)).thenReturn("app_user"); // current_user + + assertEquals("public", schemaUsedByGetTableColumns(), + "an implicit \"$user\" match must not move introspection off public"); + } + + @Test + void deliberatelyConfiguredSearchPathIsHonoured() throws SQLException { + // ALTER ROLE IN DATABASE SET search_path = marts, public; + when(schemaResultSet.getString(1)).thenReturn("marts"); + when(schemaResultSet.getString(3)).thenReturn("app_user"); + // lenient: the guard short-circuits on schema != current_user, so the + // search_path is never read here. Stated anyway to describe the scenario. + lenient().when(schemaResultSet.getString(2)).thenReturn("marts, public"); + + assertEquals("marts", schemaUsedByGetTableColumns()); + } + + @Test + void userSchemaIsHonouredWhenTheSearchPathWasSetDeliberately() throws SQLException { + // "$user" present but the path is NOT the shipped default — that is a stated + // intent, so it is honoured rather than second-guessed. + when(schemaResultSet.getString(1)).thenReturn("app_user"); + when(schemaResultSet.getString(2)).thenReturn("\"$user\", marts"); + when(schemaResultSet.getString(3)).thenReturn("app_user"); + + assertEquals("app_user", schemaUsedByGetTableColumns()); + } + + @Test + void nullCurrentSchemaFallsBackToPublic() throws SQLException { + // search_path naming only missing schemas makes current_schema() NULL. + when(schemaResultSet.getString(1)).thenReturn(null); + + assertEquals("public", schemaUsedByGetTableColumns()); + } + @Test void getTableColumns_qualifiesThePrimaryKeySubqueryBySchema() throws SQLException { // Postgres auto-names primary keys "

_pkey", so joining