Merge branch 'master' into fix/graceful-errors · DBDiff/DBDiff@5f41edf · GitHub
Skip to content

Commit 5f41edf

Browse files
jasdeepkhalsajasdeepkhalsa
authored
Merge branch 'master' into fix/graceful-errors
2 parents 6a1f24b + 7a47b7a commit 5f41edf

24 files changed

Lines changed: 977 additions & 61 deletions

‎src/DB/Schema/ColumnDependantPlan.php‎

Lines changed: 1 addition & 1 deletion

‎src/DB/Schema/DBSchema.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,8 @@ function getDiff() {
175175
// A view the migration replaces keeps its grants.
176176
PostgresViewGrants::apply($diffs, $this->manager->getDB('source'), $this->manager->getDB('target'));
177177
ColumnDependantPlan::apply($diffs);
178+
// A changed view others read is replaced around them.
179+
ViewReplacementPlan::apply($diffs, $this->manager->getDB('source'), $this->manager->getDB('target'));
178180
EnumSwapPlan::apply($diffs, $this->manager->getDB('source'), $this->manager->getDB('target'));
179181
CreationOrderPlan::apply($diffs, $this->manager->getDB('source'), $this->manager->getDB('target'));
180182
// Last, knowing what the changes above drop and recreate.

‎src/DB/Schema/MultiSchemaDiff.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,8 @@ public function getDiff(array $schemas, callable $diffOne, bool $compareSchemas
6060

6161
$this->rankTables($schemaDiff, AddTable::class, 'source');
6262
$this->rankTables($schemaDiff, DropTable::class, 'target');
63+
// A view in one schema can read a changed view in another.
64+
ViewReplacementPlan::apply($schemaDiff, $this->manager->getDB('source'), $this->manager->getDB('target'));
6365

6466
return ['schema' => $schemaDiff, 'data' => $dataDiff];
6567
}

‎src/DB/Schema/ViewReplacementPlan.php‎

Lines changed: 391 additions & 0 deletions
Large diffs are not rendered by default.

‎src/DB/Support/PostgresColumnDependants.php‎

Lines changed: 42 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,17 +40,42 @@ final class PostgresColumnDependants {
4040
* @return array{views: array<int, array<string, mixed>>, policies: array<int, array<string, string>>, triggers: array<int, array<string, string>>, generated: array<int, array<string, mixed>>, defaultGrantees: string[]}
4141
*/
4242
public static function find(Connection $connection, string $table, string $column, bool $regenerate = false): array {
43-
$views = self::views($connection, $table, $column);
43+
$seed = self::targetColumnCte($connection);
44+
$views = self::views($connection, $seed, 'tc.rel_oid = d.refobjid AND d.refobjsubid = tc.attnum', [$table, $column]);
4445
$viewOids = array_map(fn(array $v) => (int) $v['oid'], $views);
4546

46-
$dependants = [
47+
return self::withDefaultGrantees($connection, [
4748
'views' => array_map([self::class, 'withoutOid'], $views),
48-
'policies' => self::policies($connection, $table, $column, $viewOids),
49+
'policies' => self::policies($connection, $seed, [$table, $column], $viewOids),
4950
'triggers' => self::triggers($connection, $table, $column),
5051
'generated' => self::generatedColumns($connection, $table, $column, $regenerate),
5152
'defaultGrantees' => [],
52-
];
53+
]);
54+
}
55+
56+
/**
57+
* What stands in the way of dropping a view or materialised view: the
58+
* views built on it, at any depth, and the policies reading it or them.
59+
* Each comes with everything needed to put it back, as find() gives it.
60+
*
61+
* @return array{views: array<int, array<string, mixed>>, policies: array<int, array<string, string>>, triggers: array{}, generated: array{}, defaultGrantees: string[]}
62+
*/
63+
public static function ofRelation(Connection $connection, string $schema, string $name): array {
64+
$seed = 'target_col AS (SELECT to_regclass(?)::oid AS rel_oid, NULL::int2 AS attnum)';
65+
$bindings = [PostgresSchemaHelper::qualifiedName($schema, $name)];
66+
$views = self::views($connection, $seed, 'tc.rel_oid = d.refobjid', $bindings);
67+
$viewOids = array_map(fn(array $v) => (int) $v['oid'], $views);
5368

69+
return self::withDefaultGrantees($connection, [
70+
'views' => array_map([self::class, 'withoutOid'], $views),
71+
'policies' => self::policies($connection, $seed, $bindings, $viewOids, true),
72+
'triggers' => [],
73+
'generated' => [],
74+
'defaultGrantees' => [],
75+
]);
76+
}
77+
78+
private static function withDefaultGrantees(Connection $connection, array $dependants): array {
5479
if ($dependants['views'] !== []) {
5580
$dependants['defaultGrantees'] = array_column($connection->select(
5681
"SELECT DISTINCT CASE WHEN a.grantee = 0 THEN 'PUBLIC'
@@ -137,15 +162,19 @@ private static function targetColumnCte(Connection $connection): string {
137162
* Views and materialised views reading the column, directly or through
138163
* another view, with everything needed to put each back as it was.
139164
*/
140-
private static function views(Connection $connection, string $table, string $column): array {
165+
/**
166+
* @param string $seed the `target_col (rel_oid, attnum)` CTE
167+
* @param string $match how a view's rewrite rule reading it is recognised
168+
*/
169+
private static function views(Connection $connection, string $seed, string $match, array $bindings): array {
141170
$rows = $connection->select(
142-
"WITH RECURSIVE " . self::targetColumnCte($connection) . ",
171+
"WITH RECURSIVE " . $seed . ",
143172
deps AS (
144173
SELECT DISTINCT dependent.oid AS view_oid, 1 AS depth
145174
FROM pg_depend d
146175
JOIN pg_rewrite r ON r.oid = d.objid
147176
JOIN pg_class dependent ON dependent.oid = r.ev_class
148-
JOIN target_col tc ON tc.rel_oid = d.refobjid AND d.refobjsubid = tc.attnum
177+
JOIN target_col tc ON $match
149178
WHERE d.classid = 'pg_rewrite'::regclass
150179
AND d.refclassid = 'pg_class'::regclass
151180
AND dependent.oid <> tc.rel_oid
@@ -197,7 +226,7 @@ private static function views(Connection $connection, string $table, string $col
197226
JOIN pg_class c ON c.oid = deepest.view_oid
198227
JOIN pg_namespace n ON n.oid = c.relnamespace
199228
ORDER BY deepest.depth, n.nspname, c.relname",
200-
[$table, $column]
229+
$bindings
201230
);
202231

203232
$views = [];
@@ -230,11 +259,12 @@ private static function views(Connection $connection, string $table, string $col
230259
*
231260
* @param int[] $viewOids
232261
*/
233-
private static function policies(Connection $connection, string $table, string $column, array $viewOids): array {
262+
private static function policies(Connection $connection, string $seed, array $bindings, array $viewOids, bool $wholeRelation = false): array {
234263
$viewList = $viewOids === [] ? '0' : implode(',', array_map('intval', $viewOids));
264+
$reads = $wholeRelation ? 'tc.rel_oid = d.refobjid' : 'tc.rel_oid = d.refobjid AND tc.attnum = d.refobjsubid';
235265

236266
$rows = $connection->select(
237-
"WITH " . self::targetColumnCte($connection) . "
267+
"WITH " . $seed . "
238268
SELECT DISTINCT n.nspname AS schema, c.relname AS table_name, p.polname AS name,
239269
p.polpermissive AS permissive,
240270
CASE p.polcmd WHEN 'r' THEN 'SELECT' WHEN 'a' THEN 'INSERT'
@@ -249,12 +279,12 @@ private static function policies(Connection $connection, string $table, string $
249279
JOIN pg_policy p ON p.oid = d.objid
250280
JOIN pg_class c ON c.oid = p.polrelid
251281
JOIN pg_namespace n ON n.oid = c.relnamespace
252-
LEFT JOIN target_col tc ON tc.rel_oid = d.refobjid AND tc.attnum = d.refobjsubid
282+
LEFT JOIN target_col tc ON $reads
253283
WHERE d.classid = 'pg_policy'::regclass
254284
AND d.refclassid = 'pg_class'::regclass
255285
AND (tc.rel_oid IS NOT NULL OR d.refobjid IN ($viewList))
256286
ORDER BY n.nspname, c.relname, p.polname",
257-
[$table, $column]
287+
$bindings
258288
);
259289

260290
$policies = [];

‎src/DB/Support/PostgresComments.php‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,38 @@ private static function recreated(array $diffs): array
108108
|| $diff instanceof AlterDomain || $diff instanceof AlterCompositeType) {
109109
$relations[$diff->name] = true;
110110
}
111+
if ($diff instanceof AlterView || $diff instanceof AlterMatView) {
112+
[$bracketKeys, $bracketRelations] = self::putBackAround($diff);
113+
$keys += $bracketKeys;
114+
$relations += $bracketRelations;
115+
}
116+
}
117+
return [$keys, $relations];
118+
}
119+
120+
/**
121+
* What a replaced view's bracket drops and puts back — its triggers, the
122+
* views reading it and theirs, the policies — see ViewReplacementPlan.
123+
*
124+
* @return array{0: array<string, true>, 1: array<string, true>}
125+
*/
126+
private static function putBackAround(AlterView|AlterMatView $diff): array {
127+
$keys = [];
128+
$relations = [];
129+
foreach ($diff->replacement as $replacement) {
130+
foreach ($replacement['triggers'] as $trigger) {
131+
$keys["trigger|{$diff->name}.{$trigger['name']}"] = true;
132+
}
133+
foreach ($replacement['recreate']['views'] ?? [] as $view) {
134+
$keys['view|' . $view['name']] = true;
135+
$relations[$view['name']] = true;
136+
foreach ($view['triggers'] ?? [] as $trigger) {
137+
$keys["trigger|{$view['name']}.{$trigger['name']}"] = true;
138+
}
139+
}
140+
foreach ($replacement['recreate']['policies'] ?? [] as $policy) {
141+
$keys["policy|{$policy['table']}.{$policy['name']}"] = true;
142+
}
111143
}
112144
return [$keys, $relations];
113145
}

‎src/DB/Support/PostgresExpressionEquivalence.php‎

Lines changed: 54 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use DBDiff\Diff\AlterView;
99
use DBDiff\Diff\AlterDomain;
1010
use DBDiff\Diff\AlterTableChangeColumn;
11+
use DBDiff\Logger;
1112
use Illuminate\Database\Connection;
1213

1314
/**
@@ -50,11 +51,13 @@ final class PostgresExpressionEquivalence {
5051
*/
5152
public static function dropEquivalent(array $diffs, Connection $source, Connection $target): array {
5253
self::$rendered = [];
54+
PostgresRenderers::reset();
5355
self::renderViewsAhead($diffs, $source, $target);
5456
try {
5557
return array_values(array_filter($diffs, fn($diff) => self::isRealChange($diff, $source, $target)));
5658
} finally {
5759
self::$rendered = [];
60+
PostgresRenderers::reset();
5861
}
5962
}
6063

@@ -81,25 +84,32 @@ public static function dropEquivalent(array $diffs, Connection $source, Connecti
8184
* @param array<int, object> $diffs
8285
*/
8386
private static function renderViewsAhead(array $diffs, Connection $source, Connection $target): void {
84-
$want = [spl_object_id($source) => [], spl_object_id($target) => []];
85-
$crossVersion = null;
87+
$pairs = [];
8688
foreach ($diffs as $diff) {
8789
$pair = self::definitions($diff);
88-
if ($pair === null || $pair[0] !== 'view') {
89-
continue;
90-
}
91-
[, , $sourceDef, $targetDef] = $pair;
92-
$sourceBody = self::viewParts($sourceDef)[1] ?? null;
93-
$targetBody = self::viewParts($targetDef)[1] ?? null;
94-
if ($sourceBody !== null) {
95-
$want[spl_object_id($source)][$sourceBody] = true;
90+
if ($pair !== null && $pair[0] === 'view') {
91+
$pairs[] = [self::viewParts($pair[2])[1] ?? null, self::viewParts($pair[3])[1] ?? null];
9692
}
97-
if ($targetBody !== null) {
98-
$want[spl_object_id($target)][$targetBody] = true;
93+
}
94+
if ($pairs === []) {
95+
return;
96+
}
97+
// Each side renders its own; and where it cannot (read-only, no TEMP,
98+
// another major version), both go to the one server that renders the
99+
// comparison — see PostgresRenderers::choose().
100+
$renderer = PostgresRenderers::choose($source, $target);
101+
$want = [spl_object_id($source) => [], spl_object_id($target) => []];
102+
$add = function (Connection $on, ?string $body) use (&$want) {
103+
if ($body !== null) {
104+
$want[spl_object_id($on)][$body] = true;
99105
}
100-
$crossVersion ??= self::major($source) !== self::major($target);
101-
if ($crossVersion && $sourceBody !== null) {
102-
$want[spl_object_id($target)][$sourceBody] = true;
106+
};
107+
foreach ($pairs as [$sourceBody, $targetBody]) {
108+
$add($source, $sourceBody);
109+
$add($target, $targetBody);
110+
if ($renderer instanceof Connection) {
111+
$add($renderer, $sourceBody);
112+
$add($renderer, $targetBody);
103113
}
104114
}
105115
foreach ([$source, $target] as $connection) {
@@ -166,24 +176,37 @@ private static function isRealChange(object $diff, Connection $source, Connectio
166176
return true;
167177
}
168178
[$kind, $table, $sourceDef, $targetDef] = $pair;
169-
$a = self::canonical($source, $kind, $table, $sourceDef);
170-
$b = self::canonical($target, $kind, $table, $targetDef);
171-
// Across major versions the same definition prints differently —
172-
// PostgreSQL 16 dropped the table qualifiers 15 writes in a view
173-
// (`SELECT id` against `SELECT vt.id`) — so each side's own rendering
174-
// never matches. Rendered on one server, they do.
175-
$same = ($a !== null && $a === $b)
176-
|| ($b !== null && self::major($source) !== self::major($target)
177-
&& self::canonical($target, $kind, $table, $sourceDef) === $b);
178-
return !$same;
179+
$verdict = self::compareRendered($source, $target, $kind, $table, $sourceDef, $targetDef);
180+
if ($verdict === null) {
181+
// Neither server could render both: compare as written. Better a
182+
// change shown than one hidden — and applying needs a writable
183+
// target, which would have rendered it.
184+
Logger::warning(sprintf(
185+
'%s %s: the definitions differ as written, and neither server could render both to tell whether they mean the same (a read-only connection, or no TEMP privilege)',
186+
$kind,
187+
trim(($diff->table ?? '') . ' ' . ($diff->name ?? ''))
188+
));
189+
}
190+
return $verdict ?? true;
179191
}
180192

181-
/** @var array<int, int> server major version, per connection */
182-
private static array $majors = [];
183-
184-
private static function major(Connection $connection): int {
185-
$key = spl_object_id($connection);
186-
return self::$majors[$key] ??= intdiv((int) $connection->selectOne('SHOW server_version_num')['server_version_num'], 10000);
193+
/**
194+
* Whether the two renderings differ, on the server PostgresRenderers
195+
* chooses; null when no server could render both.
196+
*/
197+
private static function compareRendered(Connection $source, Connection $target, string $kind, ?string $table, string $sourceDef, string $targetDef): ?bool {
198+
$differ = fn(?string $a, ?string $b) => $a !== null && $b !== null ? $a !== $b : null;
199+
$verdict = null;
200+
$renderer = PostgresRenderers::choose($source, $target);
201+
if ($renderer === false) {
202+
// Each side renders its own: same version, both able to.
203+
$verdict = $differ(self::canonical($source, $kind, $table, $sourceDef), self::canonical($target, $kind, $table, $targetDef));
204+
$renderer = PostgresRenderers::canRender($target) ? $target : $source;
205+
}
206+
if ($verdict === null && $renderer !== null) {
207+
$verdict = $differ(self::canonical($renderer, $kind, $table, $sourceDef), self::canonical($renderer, $kind, $table, $targetDef));
208+
}
209+
return $verdict;
187210
}
188211

189212
/**

‎src/DB/Support/PostgresRenderers.php‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
<?php namespace DBDiff\DB\Support;
2+
3+
use Illuminate\Database\Connection;
4+
5+
/**
6+
* Which server renders a pair of definitions to compare them — see
7+
* PostgresExpressionEquivalence.
8+
*
9+
* Rendering creates temporary objects in a rolled-back transaction, which a
10+
* read-only connection, or one without TEMP, refuses.
11+
*/
12+
final class PostgresRenderers {
13+
14+
/** @var array<int, bool> whether a connection can create temporary objects */
15+
private static array $renders = [];
16+
17+
/** @var array<int, int> server major version, per connection */
18+
private static array $majors = [];
19+
20+
/** Forget what was probed: connection ids are reused once a connection is gone. */
21+
public static function reset(): void {
22+
self::$renders = [];
23+
}
24+
25+
/**
26+
* false: each definition on its own server — the same major version, and
27+
* both can render. Otherwise one server renders both, since across
28+
* versions (PostgreSQL 16 dropped the table qualifiers 15 writes in a
29+
* view) or with only one side able to render, only one server's
30+
* renderings are comparable: the target if it can, else the source.
31+
* null: neither can.
32+
*
33+
* A read-only source used to leave equivalent definitions reported as
34+
* changes — a CHECK rewritten as `= ANY (ARRAY[...])` by a dump and
35+
* restore, a view with DISTINCT ON — because the target-side rendering
36+
* was tried only across major versions.
37+
*/
38+
public static function choose(Connection $source, Connection $target): Connection|false|null {
39+
if (self::major($source) === self::major($target) && self::canRender($source) && self::canRender($target)) {
40+
return false;
41+
}
42+
if (self::canRender($target)) {
43+
return $target;
44+
}
45+
return self::canRender($source) ? $source : null;
46+
}
47+
48+
public static function canRender(Connection $connection): bool {
49+
return self::$renders[spl_object_id($connection)] ??= self::probe($connection);
50+
}
51+
52+
public static function major(Connection $connection): int {
53+
$key = spl_object_id($connection);
54+
return self::$majors[$key] ??= intdiv((int) $connection->selectOne('SHOW server_version_num')['server_version_num'], 10000);
55+
}
56+
57+
private static function probe(Connection $connection): bool {
58+
$pdo = $connection->getPdo();
59+
$pdo->beginTransaction();
60+
try {
61+
$pdo->exec('CREATE TEMP TABLE pg_temp.dbdiff_render_probe (x int)');
62+
return true;
63+
} catch (\Throwable) {
64+
return false;
65+
} finally {
66+
$pdo->rollBack();
67+
}
68+
}
69+
}

‎src/Diff/AlterMatView.php‎

Lines changed: 8 additions & 0 deletions

0 commit comments

Comments
 (0)