refactor: reduce cognitive complexity in DBSchema and PostgresAdapter… · DBDiff/DBDiff@d46614e · GitHub
Skip to content

Commit d46614e

Browse files
claudeclaude
committed
refactor: reduce cognitive complexity in DBSchema and PostgresAdapter (S3776)
DBSchema::getDiff (19 -> ~10) Extract the two blocks added for #165 into their own methods: selectTablesNeedingDiff() for the pre-scan partition and bulkFetchSchemas() for the batched load. getDiff() reads as a sequence of phases again, and each helper is well under the limit on its own. PostgresAdapter::assembleConstraints (18 -> ~12) This class sits on the 20-method ceiling (S2166), so the complexity had to come out in place rather than into a helper. Both nested loop pairs are flattened: - FK/UNIQUE/PK rows group under one table+constraint key joined by NUL, which no Postgres identifier can contain, so the emit loop is single-level and rebuilds the map from each row's own table_name/constraint_name. - Named NOT NULL constraints are collected as a flat list instead of a [table][name] map. Each row already carries both names, and the query yields exactly one row per constraint, so nothing needed the nesting. Output is unchanged: assembled constraints still come out grouped by table in the order the ORDER BY produces. Adds a test that named NOT NULL constraints sharing a name across tables stay scoped to their own table, matching the existing coverage for the other constraint kinds.
1 parent ecb9eb7 commit d46614e

3 files changed

Lines changed: 126 additions & 72 deletions

File tree

‎src/DB/Adapters/PostgresAdapter.php‎

Lines changed: 31 additions & 29 deletions

‎src/DB/Schema/DBSchema.php‎

Lines changed: 64 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -87,46 +87,8 @@ function getDiff() {
8787

8888
$commonTables = array_values(array_intersect($sourceTables, $targetTables));
8989

90-
// Pre-scan: fetch a hash of every table's schema in two batch queries
91-
// (one per DB side). Tables whose hashes match are identical and can be
92-
// skipped entirely, avoiding the 7-14 per-table queries that otherwise
93-
// fire for every common table — critical for large Supabase databases.
94-
$sourceHashes = $this->manager->getSchemaHashMap('source', $commonTables);
95-
$targetHashes = $this->manager->getSchemaHashMap('target', $commonTables);
96-
97-
// Partition: unchanged tables (skipped) vs changed tables (need full diff)
98-
$tablesNeedingDiff = [];
99-
$skipped = 0;
100-
foreach ($commonTables as $table) {
101-
if (isset($sourceHashes[$table], $targetHashes[$table])
102-
&& $sourceHashes[$table] === $targetHashes[$table]) {
103-
$skipped++;
104-
} else {
105-
$tablesNeedingDiff[] = $table;
106-
}
107-
}
108-
109-
if ($skipped > 0) {
110-
Logger::info("Pre-scan: skipped $skipped / " . count($commonTables) . " unchanged tables");
111-
}
112-
113-
// Batch-fetch full schemas for all changed tables in 7 queries per side
114-
// instead of 8 queries per table per side (O(1) vs O(N) round-trips).
115-
$sourceBulk = [];
116-
$targetBulk = [];
117-
if (!empty($tablesNeedingDiff)) {
118-
$adapter = $this->manager->getAdapter();
119-
if ($adapter instanceof BulkSchemaAdapterInterface) {
120-
$sourceBulk = $adapter->getBulkTableSchema(
121-
$this->manager->getDB('source'), $tablesNeedingDiff
122-
);
123-
$targetBulk = $adapter->getBulkTableSchema(
124-
$this->manager->getDB('target'), $tablesNeedingDiff
125-
);
126-
$n = count($tablesNeedingDiff);
127-
Logger::info("Batch schema fetch: loaded $n changed table(s) in 14 queries");
128-
}
129-
}
90+
$tablesNeedingDiff = $this->selectTablesNeedingDiff($commonTables);
91+
[$sourceBulk, $targetBulk] = $this->bulkFetchSchemas($tablesNeedingDiff);
13092

13193
foreach ($tablesNeedingDiff as $table) {
13294
$tableDiff = $tableSchema->getDiff(
@@ -158,6 +120,68 @@ function getDiff() {
158120
return $diffs;
159121
}
160122

123+
/**
124+
* Pre-scan: fetch a hash of every table's schema in two batch queries (one
125+
* per DB side) and keep only the tables whose hashes differ. Matching
126+
* hashes mean the table is identical on both sides, so it can skip the
127+
* per-table queries that would otherwise fire — critical for large
128+
* Supabase databases, where most tables are unchanged.
129+
*
130+
* Adapters that return no hashes simply yield every table, so this is
131+
* always an optimisation and never a filter.
132+
*
133+
* @param string[] $commonTables Tables present on both sides.
134+
* @return string[] Tables that still need a full schema diff.
135+
*/
136+
private function selectTablesNeedingDiff(array $commonTables): array {
137+
$sourceHashes = $this->manager->getSchemaHashMap('source', $commonTables);
138+
$targetHashes = $this->manager->getSchemaHashMap('target', $commonTables);
139+
140+
$needingDiff = [];
141+
foreach ($commonTables as $table) {
142+
$identical = isset($sourceHashes[$table], $targetHashes[$table])
143+
&& $sourceHashes[$table] === $targetHashes[$table];
144+
if (!$identical) {
145+
$needingDiff[] = $table;
146+
}
147+
}
148+
149+
$skipped = count($commonTables) - count($needingDiff);
150+
if ($skipped > 0) {
151+
Logger::info("Pre-scan: skipped $skipped / " . count($commonTables) . " unchanged tables");
152+
}
153+
154+
return $needingDiff;
155+
}
156+
157+
/**
158+
* Batch-fetch the full schema of every changed table in 7 queries per side
159+
* instead of 8 queries per table per side — O(1) round-trips instead of
160+
* O(N). Adapters without bulk support return empty maps, and TableSchema
161+
* then falls back to querying each table individually.
162+
*
163+
* @param string[] $tables Tables needing a full diff.
164+
* @return array{array<string,array>, array<string,array>} [source, target]
165+
*/
166+
private function bulkFetchSchemas(array $tables): array {
167+
if (empty($tables)) {
168+
return [[], []];
169+
}
170+
171+
$adapter = $this->manager->getAdapter();
172+
if (!$adapter instanceof BulkSchemaAdapterInterface) {
173+
return [[], []];
174+
}
175+
176+
$source = $adapter->getBulkTableSchema($this->manager->getDB('source'), $tables);
177+
$target = $adapter->getBulkTableSchema($this->manager->getDB('target'), $tables);
178+
179+
$n = count($tables);
180+
Logger::info("Batch schema fetch: loaded $n changed table(s) in 14 queries");
181+
182+
return [$source, $target];
183+
}
184+
161185
/**
162186
* Diff views between source and target databases.
163187
*/

‎tests/Unit/PostgresBulkSchemaTest.php‎

Lines changed: 31 additions & 3 deletions

0 commit comments

Comments
 (0)