Skip to content

Commit ac233ee

Browse files
committed
fix(postgres): bind every placeholder in the table-stats query
getTableStats built a query with NINE `?` placeholders — the two size subtractions use two each — while the binding loop ran `for (i = 1; i <= 7)`. Parameters 8 and 9 were never set, so Postgres rejected every call with No value specified for parameter 8 That silently disabled table-growth snapshots for every table on every Postgres connection: TableGrowthMonitoringService logs the failure per table and carries on, so the scheduled job "succeeded" while capturing nothing. It shows up in the backend log as a steady stream of ✗ Failed to capture snapshot for table: dba_batch_job_execution - No value specified for parameter 8. The query now binds one value and references it through a CTE, so the count cannot drift again — counting placeholders by hand is precisely what failed here. Verified against a live Postgres 18: the old form reports 9 parameters via pg_prepared_statements, the new one reports 1 and returns correct sizes for dba_batch_job_execution, the exact table from the log. MySQL's getTableStats was checked and is unaffected (2 placeholders, 2 bound). Tests: the existing getTableStats_returnsStats passed throughout the bug, because a mocked PreparedStatement does not enforce that placeholders are bound. The new test compares the two directly — it fails against the pre-fix provider with "Wanted 9 times" while the other 10 tests still pass, which is exactly why this reached production.
1 parent 8a8483e commit ac233ee

2 files changed

Lines changed: 56 additions & 10 deletions

File tree

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

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -223,21 +223,31 @@ public TableStats getTableStats(Connection connection, String database, String t
223223
TableStats stats = new TableStats();
224224
stats.setTableName(tableName);
225225

226+
// One placeholder, resolved once in a CTE, instead of repeating `?::regclass`
227+
// in every expression. The previous form had NINE placeholders — the two
228+
// subtractions use two each — while the binding loop ran `i <= 7`, so
229+
// parameters 8 and 9 were never set and every call threw
230+
// `No value specified for parameter 8`. That silently broke table-growth
231+
// snapshots for every table on every Postgres connection
232+
// (TableGrowthMonitoringService logs it per table and carries on).
233+
//
234+
// Counting placeholders by hand is exactly what failed here, so the count is
235+
// now impossible to get wrong: bind one value and reference it by name.
226236
String query = """
237+
WITH t AS (SELECT ?::regclass AS rel)
227238
SELECT
228-
pg_size_pretty(pg_total_relation_size(?::regclass)) as total_size,
229-
pg_total_relation_size(?::regclass) as total_bytes,
230-
pg_size_pretty(pg_relation_size(?::regclass)) as data_size,
231-
pg_relation_size(?::regclass) as data_bytes,
232-
pg_size_pretty(pg_total_relation_size(?::regclass) - pg_relation_size(?::regclass)) as index_size,
233-
(pg_total_relation_size(?::regclass) - pg_relation_size(?::regclass)) as index_bytes,
234-
obj_description(?::regclass, 'pg_class') as comment
239+
pg_size_pretty(pg_total_relation_size(rel)) as total_size,
240+
pg_total_relation_size(rel) as total_bytes,
241+
pg_size_pretty(pg_relation_size(rel)) as data_size,
242+
pg_relation_size(rel) as data_bytes,
243+
pg_size_pretty(pg_total_relation_size(rel) - pg_relation_size(rel)) as index_size,
244+
(pg_total_relation_size(rel) - pg_relation_size(rel)) as index_bytes,
245+
obj_description(rel, 'pg_class') as comment
246+
FROM t
235247
""";
236248

237249
try (PreparedStatement stmt = connection.prepareStatement(query)) {
238-
for (int i = 1; i <= 7; i++) {
239-
stmt.setString(i, tableName);
240-
}
250+
stmt.setString(1, tableName);
241251

242252
try (ResultSet rs = stmt.executeQuery()) {
243253
if (rs.next()) {

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,12 @@
1010
import java.sql.*;
1111
import java.util.List;
1212

13+
import org.mockito.ArgumentCaptor;
14+
1315
import static org.junit.jupiter.api.Assertions.*;
16+
import static org.mockito.ArgumentMatchers.anyInt;
1417
import static org.mockito.ArgumentMatchers.anyString;
18+
import static org.mockito.ArgumentMatchers.eq;
1519
import static org.mockito.Mockito.*;
1620

1721
@ExtendWith(MockitoExtension.class)
@@ -171,6 +175,38 @@ void getTableStats_returnsStats() throws SQLException {
171175
assertEquals(122880L, stats.getSizeBytes());
172176
}
173177

178+
@Test
179+
void getTableStats_bindsEveryPlaceholderInTheStatsQuery() throws SQLException {
180+
// The stats query carried NINE `?` placeholders (the two size subtractions use
181+
// two each) while the binding loop ran `i <= 7`, so parameters 8 and 9 were
182+
// never set and Postgres rejected every call with
183+
// No value specified for parameter 8
184+
// silently killing table-growth snapshots for every table.
185+
//
186+
// getTableStats_returnsStats above passed throughout, because a mocked
187+
// PreparedStatement does not enforce that placeholders are bound. This test
188+
// compares the two directly, so the count can never drift again.
189+
PreparedStatement rowCountStatement = mock(PreparedStatement.class);
190+
ResultSet rowCountResultSet = mock(ResultSet.class);
191+
ArgumentCaptor<String> sqlCaptor = ArgumentCaptor.forClass(String.class);
192+
193+
when(connection.prepareStatement(anyString())).thenReturn(preparedStatement, rowCountStatement);
194+
when(preparedStatement.executeQuery()).thenReturn(resultSet);
195+
when(rowCountStatement.executeQuery()).thenReturn(rowCountResultSet);
196+
when(resultSet.next()).thenReturn(true);
197+
when(rowCountResultSet.next()).thenReturn(true);
198+
when(rowCountResultSet.getObject("row_count")).thenReturn(1000L);
199+
200+
provider.getTableStats(connection, "public", "users");
201+
202+
verify(connection, atLeastOnce()).prepareStatement(sqlCaptor.capture());
203+
String statsQuery = sqlCaptor.getAllValues().get(0);
204+
int placeholders = (int) statsQuery.chars().filter(c -> c == '?').count();
205+
206+
assertTrue(placeholders > 0, "stats query should still be parameterised");
207+
verify(preparedStatement, times(placeholders)).setString(anyInt(), eq("users"));
208+
}
209+
174210
@Test
175211
void scanSchema_returnsSchemaMetadata() throws SQLException {
176212
when(connection.createStatement()).thenReturn(statement);

0 commit comments

Comments
 (0)