Skip to content

Commit 05ddf85

Browse files
committed
fix(postgres): only honour a search_path that was deliberately set
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().
1 parent f63eea0 commit 05ddf85

2 files changed

Lines changed: 115 additions & 6 deletions

File tree

backend/src/main/java/com/dbaagent/provider/postgres/PostgresIntrospectionProvider.java

Lines changed: 50 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -821,18 +821,52 @@ private Long getExactTableRowCount(Connection connection, String schemaName, Str
821821

822822
/**
823823
* The schema this session's catalog queries resolve against — the first
824-
* existing entry in the search_path. Falls back to {@code public} so a
825-
* connection whose search_path names only missing schemas behaves exactly
826-
* as it did before, rather than tagging every object with a null schema.
824+
* existing entry in the search_path — but only when that search_path was
825+
* deliberately set. Falls back to {@code public} otherwise.
826+
*
827+
* <p>Two distinct fallbacks, for two distinct reasons:
828+
*
829+
* <ol>
830+
* <li>{@code current_schema()} is null when the search_path names only
831+
* schemas that do not exist. Tagging every object with a null schema
832+
* would be worse than the previous behaviour.
833+
* <li><b>The search_path is still the untouched Postgres default.</b> Every
834+
* Postgres ships {@code "$user", public} — RDS and Aurora included — and
835+
* that leading {@code "$user"} is inert only while no schema matches the
836+
* connecting role's name. Create one (per-tenant layouts, or the per-user
837+
* pattern the Postgres docs recommend and which spread after PG15
838+
* hardened {@code public}) and {@code current_schema()} silently becomes
839+
* that schema. A connection that had been reading {@code public} would
840+
* start reading an empty user schema and report a healthy, empty brain —
841+
* the very failure honouring search_path exists to fix, inverted.
842+
* </ol>
843+
*
844+
* <p>The second case is detected rather than merely logged: an operator who has
845+
* not touched search_path gets the historical {@code public} behaviour bit for
846+
* bit, and only an explicit setting — {@code ALTER ROLE … SET search_path},
847+
* or the JDBC {@code currentSchema} parameter — moves this provider off it.
848+
* That keeps the blast radius of this feature to connections that asked for it.
849+
*
850+
* <p>Deliberately narrow: the check is for the default search_path *exactly*.
851+
* Someone who writes {@code "$user", marts} has stated an intent, and it is
852+
* honoured.
827853
*/
828854
private String resolveSchema(Connection connection) {
829855
try (Statement stmt = connection.createStatement();
830-
ResultSet rs = stmt.executeQuery("SELECT current_schema()")) {
856+
ResultSet rs = stmt.executeQuery(
857+
"SELECT current_schema(), current_setting('search_path'), current_user")) {
831858
if (rs.next()) {
832859
String schema = rs.getString(1);
833-
if (schema != null && !schema.isBlank()) {
834-
return schema;
860+
if (schema == null || schema.isBlank()) {
861+
return DEFAULT_SCHEMA;
862+
}
863+
if (schema.equals(rs.getString(3)) && isUntouchedDefaultSearchPath(rs.getString(2))) {
864+
log.debug("search_path is the Postgres default and '{}' matches the connecting "
865+
+ "role, so it was selected by the implicit \"$user\" entry rather than "
866+
+ "configured — introspecting '{}' as before", schema, DEFAULT_SCHEMA);
867+
return DEFAULT_SCHEMA;
835868
}
869+
return schema;
836870
}
837871
} catch (SQLException e) {
838872
log.debug("Could not resolve current_schema(), falling back to {}: {}",
@@ -841,6 +875,16 @@ private String resolveSchema(Connection connection) {
841875
return DEFAULT_SCHEMA;
842876
}
843877

878+
/**
879+
* True when search_path is exactly what Postgres ships, ignoring spacing.
880+
* Anything else — including a reordered or extended path that still mentions
881+
* {@code "$user"} — counts as configured and is honoured.
882+
*/
883+
private boolean isUntouchedDefaultSearchPath(String searchPath) {
884+
return searchPath != null
885+
&& "\"$user\",public".equals(searchPath.replace(" ", ""));
886+
}
887+
844888
/**
845889
* Say which schema this pass is reading, so a switch is never silent.
846890
*

backend/src/test/java/com/dbaagent/provider/postgres/PostgresIntrospectionProviderTest.java

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,12 @@ void setUp() throws SQLException {
5959
lenient().when(connection.createStatement()).thenReturn(schemaStatement, statement);
6060
lenient().when(schemaStatement.executeQuery(anyString())).thenReturn(schemaResultSet);
6161
lenient().when(schemaResultSet.next()).thenReturn(true);
62+
// resolveSchema() reads current_schema(), search_path, current_user.
63+
// A schema that differs from the role is an ordinary resolution, so these
64+
// fixtures land on "public" exactly as they did before search_path support.
6265
lenient().when(schemaResultSet.getString(1)).thenReturn("public");
66+
lenient().when(schemaResultSet.getString(2)).thenReturn("\"$user\", public");
67+
lenient().when(schemaResultSet.getString(3)).thenReturn("app_user");
6368
}
6469

6570
@Test
@@ -198,6 +203,66 @@ void getTableStats_returnsStats() throws SQLException {
198203
assertEquals(122880L, stats.getSizeBytes());
199204
}
200205

206+
/** Runs getTableColumns and returns the schema it bound (parameter 4). */
207+
private String schemaUsedByGetTableColumns() throws SQLException {
208+
when(connection.prepareStatement(anyString())).thenReturn(preparedStatement);
209+
when(preparedStatement.executeQuery()).thenReturn(resultSet);
210+
when(resultSet.next()).thenReturn(false);
211+
212+
provider.getTableColumns(connection, "db", "orders");
213+
214+
ArgumentCaptor<String> bound = ArgumentCaptor.forClass(String.class);
215+
verify(preparedStatement, atLeastOnce()).setString(eq(4), bound.capture());
216+
return bound.getValue();
217+
}
218+
219+
@Test
220+
void schemaChosenByTheImplicitDollarUserEntryIsIgnored() throws SQLException {
221+
// Every Postgres ships search_path = "$user", public. That leading "$user"
222+
// is inert only while no schema matches the connecting role — create one and
223+
// current_schema() silently becomes it. Honouring that would move an
224+
// untouched RDS/Aurora connection off `public` onto an empty user schema and
225+
// report a healthy, empty brain. An operator who never configured a
226+
// search_path must keep the historical behaviour exactly.
227+
when(schemaResultSet.getString(1)).thenReturn("app_user"); // current_schema()
228+
when(schemaResultSet.getString(2)).thenReturn("\"$user\", public"); // untouched default
229+
when(schemaResultSet.getString(3)).thenReturn("app_user"); // current_user
230+
231+
assertEquals("public", schemaUsedByGetTableColumns(),
232+
"an implicit \"$user\" match must not move introspection off public");
233+
}
234+
235+
@Test
236+
void deliberatelyConfiguredSearchPathIsHonoured() throws SQLException {
237+
// ALTER ROLE <user> IN DATABASE <db> SET search_path = marts, public;
238+
when(schemaResultSet.getString(1)).thenReturn("marts");
239+
when(schemaResultSet.getString(3)).thenReturn("app_user");
240+
// lenient: the guard short-circuits on schema != current_user, so the
241+
// search_path is never read here. Stated anyway to describe the scenario.
242+
lenient().when(schemaResultSet.getString(2)).thenReturn("marts, public");
243+
244+
assertEquals("marts", schemaUsedByGetTableColumns());
245+
}
246+
247+
@Test
248+
void userSchemaIsHonouredWhenTheSearchPathWasSetDeliberately() throws SQLException {
249+
// "$user" present but the path is NOT the shipped default — that is a stated
250+
// intent, so it is honoured rather than second-guessed.
251+
when(schemaResultSet.getString(1)).thenReturn("app_user");
252+
when(schemaResultSet.getString(2)).thenReturn("\"$user\", marts");
253+
when(schemaResultSet.getString(3)).thenReturn("app_user");
254+
255+
assertEquals("app_user", schemaUsedByGetTableColumns());
256+
}
257+
258+
@Test
259+
void nullCurrentSchemaFallsBackToPublic() throws SQLException {
260+
// search_path naming only missing schemas makes current_schema() NULL.
261+
when(schemaResultSet.getString(1)).thenReturn(null);
262+
263+
assertEquals("public", schemaUsedByGetTableColumns());
264+
}
265+
201266
@Test
202267
void getTableColumns_qualifiesThePrimaryKeySubqueryBySchema() throws SQLException {
203268
// Postgres auto-names primary keys "<table>_pkey", so joining

0 commit comments

Comments
 (0)