From bb14c293117157774dff910a147d3d3c9dc69460 Mon Sep 17 00:00:00 2001 From: Norbert Orzechowicz Date: Tue, 22 Sep 2026 19:24:16 +0200 Subject: [PATCH] fix(flow-php/postgresql): sql_query_tables() returns only relation names - new RelationCollector: skips FOR UPDATE OF names and CTE references (PostgreSQL scope rules) - DROP / COMMENT ON / SECURITY LABEL / ALTER EXTENSION targets reported - fixes false view dependency cycles and false view drops in schema diffs - upgrading.md: plain characters instead of typographic ellipses --- documentation/components/libs/postgresql.md | 8 +- documentation/upgrading.md | 59 ++-- .../AST/Visitors/RelationCollector.php | 263 ++++++++++++++++++ .../src/Flow/PostgreSql/Extractors/Tables.php | 4 +- .../Tests/Mother/StopTraversalVisitor.php | 26 ++ .../AST/Visitors/RelationCollectorTest.php | 156 +++++++++++ .../PostgreSql/Tests/Unit/ParsedQueryTest.php | 39 ++- .../Diff/ViewDependencyResolverTest.php | 32 +++ .../MaterializedViewDependencyOrderTest.php | 28 ++ .../Unit/Schema/ViewDependencyOrderTest.php | 22 ++ 10 files changed, 593 insertions(+), 44 deletions(-) create mode 100644 src/lib/postgresql/src/Flow/PostgreSql/AST/Visitors/RelationCollector.php create mode 100644 src/lib/postgresql/tests/Flow/PostgreSql/Tests/Mother/StopTraversalVisitor.php create mode 100644 src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/AST/Visitors/RelationCollectorTest.php diff --git a/documentation/components/libs/postgresql.md b/documentation/components/libs/postgresql.md index f7514022ae..86494a61df 100644 --- a/documentation/components/libs/postgresql.md +++ b/documentation/components/libs/postgresql.md @@ -87,9 +87,10 @@ foreach (sql_query_functions($query)->all() as $func) { } ``` -Extractors report every node of their kind anywhere in the statement — DDL targets (`CREATE TABLE x AS …`, -`CREATE VIEW v …`, `SELECT … INTO t`), `FOR UPDATE OF t`, `excluded.*`, window `ORDER BY` — so filter the result -when you need only some of them. +Extractors report every node of their kind anywhere in the statement - DDL targets (`CREATE TABLE x AS ...`, +`CREATE VIEW v ...`, `SELECT ... INTO t`), `excluded.*`, window `ORDER BY` - so filter the result when you need only +some of them. `sql_query_tables()` returns relations only: it skips CTE references and `FOR UPDATE OF` names, and +includes the targets of `DROP`, `COMMENT ON`, `SECURITY LABEL` and `ALTER EXTENSION`. ### Parsing Utilities @@ -574,6 +575,7 @@ Visitors declare which node types they handle via `nodeClasses()` (one or many). - `ColumnRefCollector` - collects all `ColumnRef` nodes - `FuncCallCollector` - collects all `FuncCall` nodes - `RangeVarCollector` - collects all `RangeVar` nodes +- `RelationCollector` - collects a `RangeVar` per relation - skips CTE references and `FOR UPDATE OF` names, builds one for `DROP` / `COMMENT ON` targets ### Custom Modifiers diff --git a/documentation/upgrading.md b/documentation/upgrading.md index ef434a3064..3c36a06639 100644 --- a/documentation/upgrading.md +++ b/documentation/upgrading.md @@ -280,24 +280,23 @@ final class MyExtractor implements Extractor ### 27) `flow-php/postgresql` - `Traverser` visits every node -| Before | After | -|-----------------------------------------------------------------------------------------------|-----------------------------------------------------------| -| `sql_query_tables('CREATE TABLE x AS SELECT * FROM t')` - `[]` | `[t, x]` | -| `sql_query_tables('SELECT * FROM t JOIN u ON true FOR UPDATE OF t')` - `[t, u]` | `[t, u, t]` - every reference, filter duplicates yourself | -| `sql_query_tables('CREATE VIEW v AS SELECT a FROM src')` - `[]` | `[v, src]` | -| `sql_query_tables('SELECT a INTO new_t FROM src')` - `[src]` | `[new_t, src]` | -| `sql_query_columns('… ON CONFLICT (name) DO UPDATE SET name = excluded.name')` - `[]` | `[excluded.name]` | -| `OrderBy` of `SELECT a, row_number() OVER (ORDER BY b) FROM t ORDER BY a` - 1 clause | 2 clauses - window `ORDER BY` included | -| `sql_query_tables('SELECT (SELECT x FROM a) FROM b')` - `[b, a]` | `[a, b]` - descriptor (PostgreSQL walker) order | -| `sql_query_depth()`: `EXPLAIN SELECT 1` 0, `CREATE VIEW v AS SELECT 1` 0, window subquery 1 | 1, 1, 2 | -| `sql_to_keyset_query()` cursor on `… WHERE $1 IN (SELECT …)` - `$1`, clashing with the user's | `$2` | -| `TypeCastStripper` left casts under a `SubLink` test, a window and `COLLATE` | stripped | +| Before | After | +|---------------------------------------------------------------------------------------------------|-------------------------------------------------| +| `sql_query_tables('CREATE TABLE x AS SELECT * FROM t')` - `[]` | `[t, x]` | +| `sql_query_tables('CREATE VIEW v AS SELECT a FROM src')` - `[]` | `[v, src]` | +| `sql_query_tables('SELECT a INTO new_t FROM src')` - `[src]` | `[new_t, src]` | +| `sql_query_columns('... ON CONFLICT (name) DO UPDATE SET name = excluded.name')` - `[]` | `[excluded.name]` | +| `OrderBy` of `SELECT a, row_number() OVER (ORDER BY b) FROM t ORDER BY a` - 1 clause | 2 clauses - window `ORDER BY` included | +| `sql_query_tables('SELECT (SELECT x FROM a) FROM b')` - `[b, a]` | `[a, b]` - descriptor (PostgreSQL walker) order | +| `sql_query_depth()`: `EXPLAIN SELECT 1` 0, `CREATE VIEW v AS SELECT 1` 0, window subquery 1 | 1, 1, 2 | +| `sql_to_keyset_query()` cursor on `... WHERE $1 IN (SELECT ...)` - `$1`, clashing with the user's | `$2` | +| `TypeCastStripper` left casts under a `SubLink` test, a window and `COLLATE` | stripped | ### 28) `flow-php/postgresql` - traversal contract: messages, depth, replacement, `REMOVE_NODE` | Before | After | |-------------------------------------------------------------------|------------------------------------------------------------------------------------------| -| `ModificationContext::ancestors()` / `parent()` - `Node` wrappers | the real messages (`SelectStmt`, `RangeSubselect`, …), no `Node` wrappers | +| `ModificationContext::ancestors()` / `parent()` - `Node` wrappers | the real messages (`SelectStmt`, `RangeSubselect`, ...), no `Node` wrappers | | depth of a CTE body - 3 | 4 - `WithClause`, `WindowDef`, `IntoClause`, `OnConflictClause` are levels too | | a replacement returned below the top-level statement - ignored | written into its slot | | a replacement of the wrong class - ignored | `ParserException` | @@ -310,15 +309,15 @@ final class MyExtractor implements Extractor | `ExplainModifier::nodeClasses()` - `[SelectStmt::class]` | `[ParseResult::class]` | | INSERT/UPDATE/DELETE/MERGE/CTAS/EXECUTE/DECLARE - returned unwrapped | wrapped in `EXPLAIN` | | `sql_to_explain('CREATE TABLE x (a int)')`, `sql_to_explain('EXPLAIN SELECT 1')` - returned unwrapped | `InvalidStatementException` | -| `$client->explain('INSERT …')` - the `INSERT` ran and committed | EXPLAIN; with ANALYZE inside a transaction (savepoint when one is open) that is always rolled back | -| `traverse(new PaginationModifier(…), new ExplainModifier(…))` - both applied | the pagination is dropped (or the traversal throws) - call `traverse()` again with `ExplainModifier` alone, last | +| `$client->explain('INSERT ...')` - the `INSERT` ran and committed | EXPLAIN; with ANALYZE inside a transaction (savepoint when one is open) that is always rolled back | +| `traverse(new PaginationModifier(...), new ExplainModifier(...))` - both applied | the pagination is dropped (or the traversal throws) - call `traverse()` again with `ExplainModifier` alone, last | ### 30) `flow-php/postgresql` - keyset pagination wraps `UNION` / `INTERSECT` / `EXCEPT` -| Before | After | -|-----------------------------------------------------------------------------------------------------------------------------|--------------------------------------------------------------------| -| `sql_to_keyset_query('SELECT id FROM t UNION SELECT id FROM u ORDER BY id', …)` - cursor dropped, page 2 fails with `08P01` | `SELECT * FROM (…) _keyset_subq WHERE id > $1 ORDER BY id LIMIT …` | -| qualified keyset column (`t.id`) on a set operation - `42P01` at run time | `PaginationException` | +| Before | After | +|-------------------------------------------------------------------------------------------------------------------------------|------------------------------------------------------------------------| +| `sql_to_keyset_query('SELECT id FROM t UNION SELECT id FROM u ORDER BY id', ...)` - cursor dropped, page 2 fails with `08P01` | `SELECT * FROM (...) _keyset_subq WHERE id > $1 ORDER BY id LIMIT ...` | +| qualified keyset column (`t.id`) on a set operation - `42P01` at run time | `PaginationException` | ### 31) `flow-php/postgresql` - schema keeps the declared expression text, compares normalised keys @@ -335,13 +334,13 @@ final class MyExtractor implements Extractor ### 32) `flow-php/postgresql` - schema DDL emits index `WHERE` and trigger `WHEN` -| Before | After | -|----------------------------------------------------------------|-------------------------------------------------------------------------------| -| `CREATE UNIQUE INDEX t_email_live ON s.t (email)` | `CREATE UNIQUE INDEX t_email_live ON s.t (email) WHERE deleted_at IS NULL` | -| `CREATE TRIGGER t_trg … FOR EACH ROW EXECUTE FUNCTION f()` | `CREATE TRIGGER t_trg … FOR EACH ROW WHEN (new.i > 0) EXECUTE FUNCTION s.f()` | -| an unqualified trigger function resolved through `search_path` | resolved to the table's schema | -| introspected `Trigger::$functionName` - `name` | `schema.name`; new `Trigger::withFunctionSchema()` | -| a declared `'s.f'` always drifted against the catalog | no drift | +| Before | After | +|----------------------------------------------------------------|---------------------------------------------------------------------------------| +| `CREATE UNIQUE INDEX t_email_live ON s.t (email)` | `CREATE UNIQUE INDEX t_email_live ON s.t (email) WHERE deleted_at IS NULL` | +| `CREATE TRIGGER t_trg ... FOR EACH ROW EXECUTE FUNCTION f()` | `CREATE TRIGGER t_trg ... FOR EACH ROW WHEN (new.i > 0) EXECUTE FUNCTION s.f()` | +| an unqualified trigger function resolved through `search_path` | resolved to the table's schema | +| introspected `Trigger::$functionName` - `name` | `schema.name`; new `Trigger::withFunctionSchema()` | +| a declared `'s.f'` always drifted against the catalog | no drift | ### 33) `flow-php/postgresql` - a failed `SAVEPOINT` leaves the outer transaction open @@ -349,6 +348,14 @@ final class MyExtractor implements Extractor |---------------------------------------------------------------------------------------------|--------------------------------------------------------------------------| | nesting level reset to 0, the caller's `rollBack()` throws, the connection stays in `25P02` | nesting level kept; the caller's `rollBack()` ends the outer transaction | +### 34) `flow-php/postgresql` - `sql_query_tables()` skips CTE references, reports `DROP` / `COMMENT ON` targets + +| Before | After | +|--------------------------------------------------------------------------------------|------------| +| `sql_query_tables('WITH c AS (SELECT * FROM users) SELECT * FROM c')` - `[c, users]` | `[users]` | +| `sql_query_tables('DROP TABLE a, s.b')` - `[]` | `[a, s.b]` | +| `sql_query_tables("COMMENT ON COLUMN s.t.c IS 'x'")` - `[]` | `[s.t]` | + --- ## Upgrading from 0.43.x to 0.44.x diff --git a/src/lib/postgresql/src/Flow/PostgreSql/AST/Visitors/RelationCollector.php b/src/lib/postgresql/src/Flow/PostgreSql/AST/Visitors/RelationCollector.php new file mode 100644 index 0000000000..44c0cea61a --- /dev/null +++ b/src/lib/postgresql/src/Flow/PostgreSql/AST/Visitors/RelationCollector.php @@ -0,0 +1,263 @@ + + */ + private array $rangeVars = []; + + /** + * @var list, recursive: bool, entered: int, inCte: bool}> + */ + private array $scopes = []; + + /** + * @var list + */ + private array $targets = []; + + public static function nodeClasses(): array + { + return [ + SelectStmt::class, + InsertStmt::class, + UpdateStmt::class, + DeleteStmt::class, + MergeStmt::class, + DropStmt::class, + CommentStmt::class, + SecLabelStmt::class, + AlterExtensionContentsStmt::class, + CommonTableExpr::class, + IntoClause::class, + LockingClause::class, + RangeVar::class, + ]; + } + + public function enter(object $node): ?int + { + if ($node instanceof LockingClause) { + return NodeVisitor::DONT_TRAVERSE_CHILDREN; + } + + if ($node instanceof IntoClause) { + $rel = $node->getRel(); + + if ($rel !== null) { + $this->targets[] = $rel; + } + + return null; + } + + if ($node instanceof CommonTableExpr) { + $scope = count($this->scopes) - 1; + $this->scopes[$scope]['entered']++; + $this->scopes[$scope]['inCte'] = true; + + return null; + } + + if ($node instanceof RangeVar) { + if (in_array($node, $this->targets, true) || $node->getSchemaname() !== '') { + $this->rangeVars[] = $node; + + return null; + } + + foreach ($this->scopes as $scope) { + if (in_array( + $node->getRelname(), + $scope['inCte'] && !$scope['recursive'] + ? array_slice($scope['names'], 0, $scope['entered'] - 1) + : $scope['names'], + true, + )) { + return null; + } + } + + $this->rangeVars[] = $node; + + return null; + } + + $objectType = null; + $objects = []; + + if ($node instanceof DropStmt) { + $objectType = $node->getRemoveType(); + $objects = iterator_to_array($node->getObjects(), false); + } + + if ( + $node instanceof CommentStmt + || $node instanceof SecLabelStmt + || $node instanceof AlterExtensionContentsStmt + ) { + $objectType = $node->getObjtype(); + $objects = [$node->getObject()]; + } + + $member = in_array($objectType, self::RELATION_MEMBER_KINDS, true); + + if ($member || in_array($objectType, self::RELATION_KINDS, true)) { + foreach ($objects as $object) { + $names = []; + + foreach ($object?->getList()?->getItems() ?? [] as $item) { + $name = $item->getString()?->getSval(); + + if ($name !== null) { + $names[] = $name; + } + } + + if ($member) { + array_pop($names); + } + + $relname = array_pop($names); + + if ($relname === null) { + continue; + } + + $this->rangeVars[] = new RangeVar([ + 'relname' => $relname, + 'schemaname' => array_pop($names) ?? '', + 'catalogname' => array_pop($names) ?? '', + ]); + } + + return null; + } + + if ( + $node instanceof InsertStmt + || $node instanceof UpdateStmt + || $node instanceof DeleteStmt + || $node instanceof MergeStmt + ) { + $relation = $node->getRelation(); + + if ($relation !== null) { + $this->targets[] = $relation; + } + } + + if ( + $node instanceof SelectStmt + || $node instanceof InsertStmt + || $node instanceof UpdateStmt + || $node instanceof DeleteStmt + || $node instanceof MergeStmt + ) { + $with = $node->getWithClause(); + + if ($with !== null) { + $names = []; + + foreach ($with->getCtes() as $cte) { + $names[] = $cte->getCommonTableExpr()?->getCtename() ?? ''; + } + + $this->scopes[] = [ + 'names' => $names, + 'recursive' => $with->getRecursive(), + 'entered' => 0, + 'inCte' => false, + ]; + } + } + + return null; + } + + /** + * @return list + */ + public function getRangeVars(): array + { + return $this->rangeVars; + } + + public function leave(object $node): ?int + { + if ($node instanceof CommonTableExpr) { + $this->scopes[count($this->scopes) - 1]['inCte'] = false; + + return null; + } + + if ( + ( + $node instanceof SelectStmt + || $node instanceof InsertStmt + || $node instanceof UpdateStmt + || $node instanceof DeleteStmt + || $node instanceof MergeStmt + ) + && $node->getWithClause() !== null + ) { + array_pop($this->scopes); + } + + return null; + } + + public function reset(): void + { + $this->rangeVars = []; + $this->scopes = []; + $this->targets = []; + } +} diff --git a/src/lib/postgresql/src/Flow/PostgreSql/Extractors/Tables.php b/src/lib/postgresql/src/Flow/PostgreSql/Extractors/Tables.php index 1918bd38f9..aabc5ddfe3 100644 --- a/src/lib/postgresql/src/Flow/PostgreSql/Extractors/Tables.php +++ b/src/lib/postgresql/src/Flow/PostgreSql/Extractors/Tables.php @@ -5,7 +5,7 @@ namespace Flow\PostgreSql\Extractors; use Flow\PostgreSql\AST\Nodes\Table; -use Flow\PostgreSql\AST\Visitors\RangeVarCollector; +use Flow\PostgreSql\AST\Visitors\RelationCollector; use Flow\PostgreSql\ParsedQuery; use function array_filter; @@ -23,7 +23,7 @@ public function __construct( */ public function all(): array { - $collector = new RangeVarCollector(); + $collector = new RelationCollector(); $this->query->traverse($collector); return array_values(array_filter( diff --git a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Mother/StopTraversalVisitor.php b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Mother/StopTraversalVisitor.php new file mode 100644 index 0000000000..2fb00d8d96 --- /dev/null +++ b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Mother/StopTraversalVisitor.php @@ -0,0 +1,26 @@ +}> + */ + public static function provide_relation_names(): Generator + { + yield 'for update of alias' => ['SELECT * FROM enriched.production p FOR UPDATE OF p', ['enriched.production']]; + yield 'for update of relation name' => ['SELECT * FROM t JOIN u ON true FOR UPDATE OF t', ['t', 'u']]; + yield 'for share of subquery alias' => ['SELECT * FROM (SELECT * FROM t) sub FOR SHARE OF sub', ['t']]; + yield 'cte reference' => ['WITH c AS (SELECT * FROM users) SELECT * FROM c', ['users']]; + yield 'non-recursive self reference' => ['WITH t AS (SELECT * FROM t) SELECT * FROM t', ['t']]; + yield 'earlier sibling' => ['WITH a AS (SELECT * FROM src), b AS (SELECT * FROM a) SELECT * FROM b', ['src']]; + yield 'later sibling is a relation' => ['WITH b AS (SELECT * FROM a), a AS (SELECT 1) SELECT * FROM b', ['a']]; + yield 'recursive later sibling' => [ + 'WITH RECURSIVE b AS (SELECT * FROM a), a AS (SELECT 1) SELECT * FROM b', + [], + ]; + yield 'recursive self reference' => [ + 'WITH RECURSIVE r AS (SELECT 1 AS n UNION ALL SELECT n + 1 FROM r WHERE n < 5) SELECT * FROM r', + [], + ]; + yield 'with on left union branch' => [ + '(WITH c AS (SELECT 1) SELECT * FROM c) UNION ALL SELECT * FROM c', + ['c'], + ]; + yield 'with over union' => ['WITH c AS (SELECT 1) SELECT * FROM c UNION ALL SELECT * FROM c', []]; + yield 'outer cte in sublink' => ['WITH c AS (SELECT 1) SELECT (SELECT * FROM c)', []]; + yield 'subquery cte stays inside' => ['SELECT * FROM (WITH c AS (SELECT 1) SELECT * FROM c) x, c', ['c']]; + yield 'schema-qualified same name' => ['WITH c AS (SELECT 1) SELECT * FROM public.c', ['public.c']]; + yield 'insert target named as cte' => ['WITH c AS (SELECT 1 AS id) INSERT INTO c SELECT id FROM c', ['c']]; + yield 'update from cte' => ['WITH s AS (SELECT 1 AS id) UPDATE t SET id = s.id FROM s', ['t']]; + yield 'update target named as cte' => ['WITH c AS (SELECT 1 AS id) UPDATE c SET id = 2', ['c']]; + yield 'delete using cte' => ['WITH s AS (SELECT 1 AS id) DELETE FROM t USING s WHERE t.id = s.id', ['t']]; + yield 'delete target named as cte' => ['WITH c AS (SELECT 1 AS id) DELETE FROM c', ['c']]; + yield 'merge source cte' => [ + 'WITH s AS (SELECT 1 AS id) MERGE INTO t USING s ON t.id = s.id WHEN MATCHED THEN DELETE', + ['t'], + ]; + yield 'merge target named as cte' => [ + 'WITH t AS (SELECT 1 AS id) MERGE INTO t USING src ON t.id = src.id WHEN MATCHED THEN DELETE', + ['t', 'src'], + ]; + yield 'select into named as cte' => ['WITH c AS (SELECT 1 AS id) SELECT id INTO c FROM c', ['c']]; + yield 'ctas' => ['CREATE TABLE x AS SELECT * FROM t', ['t', 'x']]; + yield 'create view' => ['CREATE VIEW v AS SELECT a FROM src', ['v', 'src']]; + yield 'recursive view' => [ + 'CREATE RECURSIVE VIEW v (n) AS VALUES (1) UNION ALL SELECT n + 1 FROM v WHERE n < 5', + ['v'], + ]; + yield 'data-modifying cte' => [ + 'WITH a AS (SELECT 1 AS id), b AS (INSERT INTO a SELECT id FROM a RETURNING id) SELECT * FROM b', + ['a'], + ]; + yield 'second statement' => ['WITH c AS (SELECT 1) SELECT * FROM c; SELECT * FROM c', ['c']]; + yield 'quoted name is distinct' => ['WITH "C" AS (SELECT 1) SELECT * FROM c', ['c']]; + yield 'drop tables' => ['DROP TABLE a, s.b', ['a', 's.b']]; + yield 'drop view' => ['DROP VIEW IF EXISTS s.v CASCADE', ['s.v']]; + yield 'drop materialized view' => ['DROP MATERIALIZED VIEW mv', ['mv']]; + yield 'drop foreign table' => ['DROP FOREIGN TABLE ft', ['ft']]; + yield 'drop sequence' => ['DROP SEQUENCE seq', ['seq']]; + yield 'drop index' => ['DROP INDEX CONCURRENTLY s.idx', ['s.idx']]; + yield 'drop trigger' => ['DROP TRIGGER trg ON s.t', ['s.t']]; + yield 'drop rule' => ['DROP RULE r ON t', ['t']]; + yield 'drop policy' => ['DROP POLICY p ON t', ['t']]; + yield 'drop type' => ['DROP TYPE ty', []]; + yield 'drop schema' => ['DROP SCHEMA s', []]; + yield 'comment on table' => ["COMMENT ON TABLE s.t IS 'x'", ['s.t']]; + yield 'comment on column' => ["COMMENT ON COLUMN s.t.c IS 'x'", ['s.t']]; + yield 'comment on constraint' => ["COMMENT ON CONSTRAINT con ON s.t IS 'x'", ['s.t']]; + yield 'comment on domain constraint' => ["COMMENT ON CONSTRAINT dc ON DOMAIN d IS 'x'", []]; + yield 'comment on function' => ["COMMENT ON FUNCTION f() IS 'x'", []]; + yield 'unqualified column comment' => ["COMMENT ON COLUMN c IS 'x'", []]; + yield 'security label on column' => ["SECURITY LABEL FOR p ON COLUMN t.c IS 'x'", ['t']]; + yield 'alter extension add table' => ['ALTER EXTENSION e ADD TABLE s.t', ['s.t']]; + yield 'catalog-qualified drop' => ['DROP TABLE db.s.t', ['s.t']]; + yield 'drop then select' => ['DROP TABLE b; SELECT * FROM a', ['b', 'a']]; + } + + protected function setUp(): void + { + if (!extension_loaded('pg_query')) { + self::markTestSkipped( + 'pg_query extension is not loaded. For local development use `nix-shell --arg with-pg-query-ext true` to enable it in the shell.', + ); + } + } + + /** + * @param list $expected + */ + #[DataProvider('provide_relation_names')] + public function test_collects_relation_names(string $sql, array $expected): void + { + $collector = new RelationCollector(); + sql_parse($sql)->traverse($collector); + + static::assertSame($expected, array_map( + static fn(RangeVar $rangeVar): string => ( + ($rangeVar->getSchemaname() === '' ? '' : $rangeVar->getSchemaname() . '.') . $rangeVar->getRelname() + ), + $collector->getRangeVars(), + )); + } + + public function test_name_list_keeps_catalog_name(): void + { + $collector = new RelationCollector(); + sql_parse('DROP TABLE db.s.t')->traverse($collector); + + static::assertSame('db', $collector->getRangeVars()[0]->getCatalogname()); + } + + public function test_reset_forgets_cte_scopes_left_open_by_a_stopped_traversal(): void + { + $collector = new RelationCollector(); + sql_parse('WITH c AS (SELECT 1) SELECT * FROM c')->traverse($collector, new StopTraversalVisitor()); + $collector->reset(); + sql_parse('SELECT * FROM c')->traverse($collector); + + static::assertSame( + ['c'], + array_map(static fn(RangeVar $rangeVar): string => $rangeVar->getRelname(), $collector->getRangeVars()), + ); + } + + public function test_reset_forgets_collected_relations(): void + { + $collector = new RelationCollector(); + sql_parse('SELECT * FROM t')->traverse($collector); + $collector->reset(); + sql_parse('SELECT * FROM u')->traverse($collector); + + static::assertSame( + ['u'], + array_map(static fn(RangeVar $rangeVar): string => $rangeVar->getRelname(), $collector->getRangeVars()), + ); + } +} diff --git a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/ParsedQueryTest.php b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/ParsedQueryTest.php index 69235a3ef8..f51c3a2d51 100644 --- a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/ParsedQueryTest.php +++ b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/ParsedQueryTest.php @@ -325,6 +325,14 @@ public function test_tables_include_ctas_target(): void ); } + public function test_tables_include_dropped_relations(): void + { + $tables = sql_query_tables(sql_parse('DROP TABLE a, s.b'))->all(); + + static::assertSame(['a', 'b'], array_map(static fn(Table $t) => $t->name(), $tables)); + static::assertSame([null, 's'], array_map(static fn(Table $t) => $t->schema(), $tables)); + } + public function test_tables_include_view_name_and_select_into_target(): void { static::assertSame( @@ -343,30 +351,35 @@ public function test_tables_include_view_name_and_select_into_target(): void ); } - public function test_tables_repeat_locked_relation(): void + public function test_tables_skip_locked_relation_names(): void { static::assertSame( - ['t', 'u', 't'], + ['t', 'u'], array_map( static fn(Table $t) => $t->name(), sql_query_tables(sql_parse('SELECT * FROM t JOIN u ON true FOR UPDATE OF t'))->all(), ), ); + static::assertSame( + ['production'], + array_map( + static fn(Table $t) => $t->name(), + sql_query_tables(sql_parse('SELECT * FROM enriched.production p FOR UPDATE OF p'))->all(), + ), + ); } - public function test_tables_from_cte(): void + public function test_tables_skip_cte_references(): void { - $result = sql_parse( - 'WITH active_users AS (SELECT * FROM users WHERE active = true) SELECT * FROM active_users', + static::assertSame( + ['users'], + array_map( + static fn(Table $t) => $t->name(), + sql_query_tables(sql_parse( + 'WITH active_users AS (SELECT * FROM users WHERE active = true) SELECT * FROM active_users', + ))->all(), + ), ); - - $tables = sql_query_tables($result)->all(); - - static::assertCount(2, $tables); - - $tableNames = array_map(static fn(Table $t) => $t->name(), $tables); - static::assertContains('users', $tableNames); - static::assertContains('active_users', $tableNames); } public function test_tables_from_delete(): void diff --git a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/Diff/ViewDependencyResolverTest.php b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/Diff/ViewDependencyResolverTest.php index 27eca05c83..e054474eb1 100644 --- a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/Diff/ViewDependencyResolverTest.php +++ b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/Diff/ViewDependencyResolverTest.php @@ -38,6 +38,38 @@ public function test_does_not_resolve_view_depending_on_unmodified_table(): void static::assertTrue($result->isEmpty()); } + public function test_does_not_resolve_view_locking_same_named_table_of_other_schema(): void + { + $catalog = new Catalog([ + schema('enriched', tables: [schema_table('production', [schema_column_integer('id', false)])]), + schema( + 'public', + tables: [schema_table('production', [schema_column_integer('id', false)])], + views: [schema_view('locked', 'SELECT id FROM enriched.production FOR UPDATE OF production')], + ), + ]); + + static::assertTrue(ast_view_dependency_resolver()->resolve($catalog, ['public.production'])->isEmpty()); + + $result = ast_view_dependency_resolver()->resolve($catalog, ['enriched.production']); + + static::assertCount(1, $result->toDrop); + static::assertSame('public.locked', $result->toDrop[0]->qualifiedName()); + } + + public function test_does_not_resolve_view_whose_cte_shadows_modified_table(): void + { + $catalog = new Catalog([ + schema( + 'public', + tables: [schema_table('users', [schema_column_integer('id', false)])], + views: [schema_view('with_cte', 'WITH users AS (SELECT 1 AS id) SELECT id FROM users')], + ), + ]); + + static::assertTrue(ast_view_dependency_resolver()->resolve($catalog, ['public.users'])->isEmpty()); + } + public function test_resolves_cascading_view_dependencies(): void { $catalog = new Catalog([ diff --git a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/MaterializedViewDependencyOrderTest.php b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/MaterializedViewDependencyOrderTest.php index 79dc7a2462..f435f30f6e 100644 --- a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/MaterializedViewDependencyOrderTest.php +++ b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/MaterializedViewDependencyOrderTest.php @@ -25,11 +25,39 @@ public function test_circular_dependency_throws_exception(): void (new MaterializedViewDependencyOrder(new Parser()))->order([$viewA, $viewB]); } + public function test_cte_name_is_not_a_dependency(): void + { + $a = new MaterializedView('a', 'WITH b AS (SELECT 1 AS id) SELECT id FROM b'); + $b = new MaterializedView('b', 'WITH a AS (SELECT 1 AS id) SELECT id FROM a'); + + static::assertSame( + ['a', 'b'], + array_map( + static fn(MaterializedView $v) => $v->name, + (new MaterializedViewDependencyOrder(new Parser()))->order([$a, $b]), + ), + ); + } + public function test_empty_list(): void { static::assertSame([], (new MaterializedViewDependencyOrder(new Parser()))->order([])); } + public function test_for_update_of_alias_is_not_a_dependency(): void + { + $a = new MaterializedView('a', 'SELECT id FROM t b FOR UPDATE OF b'); + $b = new MaterializedView('b', 'SELECT id FROM t a FOR UPDATE OF a'); + + static::assertSame( + ['a', 'b'], + array_map( + static fn(MaterializedView $v) => $v->name, + (new MaterializedViewDependencyOrder(new Parser()))->order([$a, $b]), + ), + ); + } + public function test_repeated_reference_is_not_a_cycle(): void { $base = new MaterializedView('v1', 'SELECT 1 AS a'); diff --git a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/ViewDependencyOrderTest.php b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/ViewDependencyOrderTest.php index a1f675601b..0c3db1aeab 100644 --- a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/ViewDependencyOrderTest.php +++ b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/ViewDependencyOrderTest.php @@ -25,11 +25,33 @@ public function test_circular_dependency_throws_exception(): void (new ViewDependencyOrder(new Parser()))->order([$viewA, $viewB]); } + public function test_cte_name_is_not_a_dependency(): void + { + $a = new View('a', 'WITH b AS (SELECT 1 AS id) SELECT id FROM b'); + $b = new View('b', 'WITH a AS (SELECT 1 AS id) SELECT id FROM a'); + + static::assertSame( + ['a', 'b'], + array_map(static fn(View $v) => $v->name, (new ViewDependencyOrder(new Parser()))->order([$a, $b])), + ); + } + public function test_empty_list(): void { static::assertSame([], (new ViewDependencyOrder(new Parser()))->order([])); } + public function test_for_update_of_alias_is_not_a_dependency(): void + { + $a = new View('a', 'SELECT id FROM t b FOR UPDATE OF b'); + $b = new View('b', 'SELECT id FROM t a FOR UPDATE OF a'); + + static::assertSame( + ['a', 'b'], + array_map(static fn(View $v) => $v->name, (new ViewDependencyOrder(new Parser()))->order([$a, $b])), + ); + } + public function test_linear_chain(): void { $base = new View('base_view', 'SELECT id, name FROM users');