From 110ad49dceaf43ab8dcda6e9883441978ff3e690 Mon Sep 17 00:00:00 2001 From: Norbert Orzechowicz Date: Mon, 1 Jun 2026 12:20:52 +0200 Subject: [PATCH] fix: postgresql schema comparator handling views --- composer.lock | 48 +++---- .../Schema/Diff/SchemaComparator.php | 20 ++- .../Database/ViewDiffDatabaseTest.php | 135 ++++++++++++++++++ .../Unit/Schema/Diff/SchemaComparatorTest.php | 135 ++++++++++++++++++ tools/box/composer.lock | 14 +- tools/infection/composer.lock | 12 +- 6 files changed, 325 insertions(+), 39 deletions(-) create mode 100644 src/lib/postgresql/tests/Flow/PostgreSql/Tests/Integration/QueryBuilder/Database/ViewDiffDatabaseTest.php diff --git a/composer.lock b/composer.lock index 08d7d60b03..0ac69bb13c 100644 --- a/composer.lock +++ b/composer.lock @@ -752,16 +752,16 @@ }, { "name": "google/apiclient-services", - "version": "v0.442.0", + "version": "v0.443.0", "source": { "type": "git", "url": "https://github.com/googleapis/google-api-php-client-services.git", - "reference": "3afe7b5c9d1cebf2a5be018820ce62eeb906b81f" + "reference": "102aba0523d5105adaffdcac6c0c74a527f2bbfc" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/googleapis/google-api-php-client-services/zipball/3afe7b5c9d1cebf2a5be018820ce62eeb906b81f", - "reference": "3afe7b5c9d1cebf2a5be018820ce62eeb906b81f", + "url": "https://api.github.com/repos/googleapis/google-api-php-client-services/zipball/102aba0523d5105adaffdcac6c0c74a527f2bbfc", + "reference": "102aba0523d5105adaffdcac6c0c74a527f2bbfc", "shasum": "" }, "require": { @@ -790,22 +790,22 @@ ], "support": { "issues": "https://github.com/googleapis/google-api-php-client-services/issues", - "source": "https://github.com/googleapis/google-api-php-client-services/tree/v0.442.0" + "source": "https://github.com/googleapis/google-api-php-client-services/tree/v0.443.0" }, - "time": "2026-05-25T01:34:24+00:00" + "time": "2026-05-29T01:56:23+00:00" }, { "name": "google/auth", - "version": "v1.50.1", + "version": "v1.50.2", "source": { "type": "git", "url": "https://github.com/googleapis/google-auth-library-php.git", - "reference": "870c17ee3a1d73338d39a9ffa77a700ba77f5a83" + "reference": "58788f86d47e59de692c0dae0bb0c006bd0b6018" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/googleapis/google-auth-library-php/zipball/870c17ee3a1d73338d39a9ffa77a700ba77f5a83", - "reference": "870c17ee3a1d73338d39a9ffa77a700ba77f5a83", + "url": "https://api.github.com/repos/googleapis/google-auth-library-php/zipball/58788f86d47e59de692c0dae0bb0c006bd0b6018", + "reference": "58788f86d47e59de692c0dae0bb0c006bd0b6018", "shasum": "" }, "require": { @@ -852,9 +852,9 @@ "support": { "docs": "https://cloud.google.com/php/docs/reference/auth/latest", "issues": "https://github.com/googleapis/google-auth-library-php/issues", - "source": "https://github.com/googleapis/google-auth-library-php/tree/v1.50.1" + "source": "https://github.com/googleapis/google-auth-library-php/tree/v1.50.2" }, - "time": "2026-03-18T20:03:29+00:00" + "time": "2026-05-29T19:22:31+00:00" }, { "name": "google/protobuf", @@ -1112,16 +1112,16 @@ }, { "name": "guzzlehttp/psr7", - "version": "2.10.3", + "version": "2.10.4", "source": { "type": "git", "url": "https://github.com/guzzle/psr7.git", - "reference": "7c1472269227dc6f18930bd903d7a88fe6c52130" + "reference": "d2a1a094e396da8957e797489fddaf860c340cfc" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/guzzle/psr7/zipball/7c1472269227dc6f18930bd903d7a88fe6c52130", - "reference": "7c1472269227dc6f18930bd903d7a88fe6c52130", + "url": "https://api.github.com/repos/guzzle/psr7/zipball/d2a1a094e396da8957e797489fddaf860c340cfc", + "reference": "d2a1a094e396da8957e797489fddaf860c340cfc", "shasum": "" }, "require": { @@ -1209,7 +1209,7 @@ ], "support": { "issues": "https://github.com/guzzle/psr7/issues", - "source": "https://github.com/guzzle/psr7/tree/2.10.3" + "source": "https://github.com/guzzle/psr7/tree/2.10.4" }, "funding": [ { @@ -1225,7 +1225,7 @@ "type": "tidelift" } ], - "time": "2026-05-27T11:48:20+00:00" + "time": "2026-05-29T12:59:07+00:00" }, { "name": "halaxa/json-machine", @@ -6577,16 +6577,16 @@ }, { "name": "twig/twig", - "version": "v3.27.0", + "version": "v3.27.1", "source": { "type": "git", "url": "https://github.com/twigphp/Twig.git", - "reference": "04ae1bfe9463c816cf72ca0abe7eae2c77a9a9ed" + "reference": "ae2071bffb38f04847fc0864d730c94b9cb8ab74" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/twigphp/Twig/zipball/04ae1bfe9463c816cf72ca0abe7eae2c77a9a9ed", - "reference": "04ae1bfe9463c816cf72ca0abe7eae2c77a9a9ed", + "url": "https://api.github.com/repos/twigphp/Twig/zipball/ae2071bffb38f04847fc0864d730c94b9cb8ab74", + "reference": "ae2071bffb38f04847fc0864d730c94b9cb8ab74", "shasum": "" }, "require": { @@ -6641,7 +6641,7 @@ ], "support": { "issues": "https://github.com/twigphp/Twig/issues", - "source": "https://github.com/twigphp/Twig/tree/v3.27.0" + "source": "https://github.com/twigphp/Twig/tree/v3.27.1" }, "funding": [ { @@ -6653,7 +6653,7 @@ "type": "tidelift" } ], - "time": "2026-05-27T13:05:51+00:00" + "time": "2026-05-30T17:09:26+00:00" } ], "aliases": [], diff --git a/src/lib/postgresql/src/Flow/PostgreSql/Schema/Diff/SchemaComparator.php b/src/lib/postgresql/src/Flow/PostgreSql/Schema/Diff/SchemaComparator.php index e0fe0f18e0..f5ffbfa5a0 100644 --- a/src/lib/postgresql/src/Flow/PostgreSql/Schema/Diff/SchemaComparator.php +++ b/src/lib/postgresql/src/Flow/PostgreSql/Schema/Diff/SchemaComparator.php @@ -18,6 +18,7 @@ use Flow\PostgreSql\Schema\Table; use Flow\PostgreSql\Schema\View; use Flow\PostgreSql\Schema\ViewDependencyOrder; +use Throwable; final readonly class SchemaComparator { @@ -36,6 +37,7 @@ public function __construct( private ExecutionOrderStrategy $materializedViewOrderStrategy = new MaterializedViewDependencyOrder( new Parser(), ), + private Parser $parser = new Parser(), ) {} public function compare(Schema $source, Schema $target): SchemaDiff @@ -65,7 +67,7 @@ public function compare(Schema $source, Schema $target): SchemaDiff $source->views, $target->views, static fn(View $v): string => $v->name, - static fn(View $a, View $b): ?ViewDiff => $a->definition === $b->definition + fn(View $a, View $b): ?ViewDiff => $this->definitionsEqual($a->definition, $b->definition) && $a->isUpdatable === $b->isUpdatable ? null : new ViewDiff($a, $b), @@ -139,6 +141,11 @@ public function compare(Schema $source, Schema $target): SchemaDiff ); } + private function definitionsEqual(string $a, string $b): bool + { + return $this->normalizeDefinition($a) === $this->normalizeDefinition($b); + } + /** * @param list $sourceDomains * @param list $targetDomains @@ -187,7 +194,7 @@ function (MaterializedView $a, MaterializedView $b): ?MaterializedViewDiff { $indexChanges = $this->indexComparator->compare($a->indexes, $b->indexes); if ( - $a->definition === $b->definition + $this->definitionsEqual($a->definition, $b->definition) && $indexChanges->added === [] && $indexChanges->removed === [] && ($indexChanges->renamed === null || $indexChanges->renamed === []) @@ -221,4 +228,13 @@ private function diffTables(array $sourceTables, array $targetTables): ChangeSet return new ChangeSet($renameResult->added, $renameResult->removed, $initial->modified, $renameResult->renamed); } + + private function normalizeDefinition(string $definition): string + { + try { + return $this->parser->parse($definition)->deparse(); + } catch (Throwable) { + return $definition; + } + } } diff --git a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Integration/QueryBuilder/Database/ViewDiffDatabaseTest.php b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Integration/QueryBuilder/Database/ViewDiffDatabaseTest.php new file mode 100644 index 0000000000..670e50c340 --- /dev/null +++ b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Integration/QueryBuilder/Database/ViewDiffDatabaseTest.php @@ -0,0 +1,135 @@ +pgsqlContext() + ->client() + ->execute( + create() + ->table(self::TABLE_SOURCE) + ->column(column('id', column_type_serial())) + ->column(column('name', column_type_varchar(100))) + ->toSql(), + ); + } + + protected function tearDown(): void + { + $this->pgsqlContext()->dropViewIfExists(self::VIEW_SIMPLE); + $this->pgsqlContext()->dropMaterializedViewIfExists(self::MATVIEW_SIMPLE); + $this->pgsqlContext()->dropTableIfExists(self::TABLE_SOURCE); + + parent::tearDown(); + } + + public function test_catalog_materialized_view_matches_single_line_definition(): void + { + $select = select(col('id'), col('name'))->from(table(self::TABLE_SOURCE)); + $this + ->pgsqlContext() + ->client() + ->execute(create()->materializedView(self::MATVIEW_SIMPLE)->as($select)->toSql()); + + $catalogSchema = (new PgCatalogProvider($this->pgsqlContext()->client(), ['public']))->get()->get('public'); + $catalogMatViews = array_values(array_filter( + $catalogSchema->materializedViews, + static fn(MaterializedView $mv): bool => $mv->name === self::MATVIEW_SIMPLE, + )); + static::assertCount(1, $catalogMatViews); + + $constraintComparator = new ConstraintComparator(); + $comparator = new SchemaComparator( + new TableComparator( + new IndexComparator(new GreedySimilarityRenameStrategy(new SimilarTextStrategy())), + $constraintComparator, + new GreedySimilarityRenameStrategy(new SimilarTextStrategy()), + ), + new IndexComparator(new GreedySimilarityRenameStrategy(new SimilarTextStrategy())), + $constraintComparator, + new TableStructureComparator(new GreedySimilarityRenameStrategy(new SimilarTextStrategy())), + ); + + $diff = $comparator->compare( + new Schema('public', materializedViews: [$catalogMatViews[0]]), + new Schema('public', materializedViews: [ + schema_materialized_view(self::MATVIEW_SIMPLE, $select->toSql()), + ]), + ); + + static::assertSame([], $diff->modifiedMaterializedViews); + } + + public function test_catalog_view_matches_single_line_definition(): void + { + $select = select(col('id'), col('name'))->from(table(self::TABLE_SOURCE)); + $this->pgsqlContext()->client()->execute(create()->view(self::VIEW_SIMPLE)->as($select)->toSql()); + + $catalogSchema = (new PgCatalogProvider($this->pgsqlContext()->client(), ['public']))->get()->get('public'); + $catalogViews = array_values(array_filter( + $catalogSchema->views, + static fn(View $v): bool => $v->name === self::VIEW_SIMPLE, + )); + static::assertCount(1, $catalogViews); + + $constraintComparator = new ConstraintComparator(); + $comparator = new SchemaComparator( + new TableComparator( + new IndexComparator(new GreedySimilarityRenameStrategy(new SimilarTextStrategy())), + $constraintComparator, + new GreedySimilarityRenameStrategy(new SimilarTextStrategy()), + ), + new IndexComparator(new GreedySimilarityRenameStrategy(new SimilarTextStrategy())), + $constraintComparator, + new TableStructureComparator(new GreedySimilarityRenameStrategy(new SimilarTextStrategy())), + ); + + $diff = $comparator->compare( + new Schema('public', views: [$catalogViews[0]]), + new Schema('public', views: [ + schema_view(self::VIEW_SIMPLE, $select->toSql(), isUpdatable: $catalogViews[0]->isUpdatable), + ]), + ); + + static::assertSame([], $diff->modifiedViews); + } +} diff --git a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/Diff/SchemaComparatorTest.php b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/Diff/SchemaComparatorTest.php index ef0c8dd780..ca636e1a0a 100644 --- a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/Diff/SchemaComparatorTest.php +++ b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/Schema/Diff/SchemaComparatorTest.php @@ -485,6 +485,30 @@ public function test_materialized_view_modified_definition(): void static::assertCount(1, $diff->modifiedMaterializedViews); } + public function test_materialized_view_formatting_only_difference_is_not_a_change(): void + { + $constraintComparator = new ConstraintComparator(); + $comparator = new SchemaComparator( + new TableComparator( + new IndexComparator(new GreedySimilarityRenameStrategy(new SimilarTextStrategy())), + $constraintComparator, + new GreedySimilarityRenameStrategy(new SimilarTextStrategy()), + ), + new IndexComparator(new GreedySimilarityRenameStrategy(new SimilarTextStrategy())), + $constraintComparator, + new TableStructureComparator(new GreedySimilarityRenameStrategy(new SimilarTextStrategy())), + ); + $source = new Schema('public', materializedViews: [schema_materialized_view( + 'mv', + " SELECT a,\n b\n FROM t;", + )]); + $target = new Schema('public', materializedViews: [schema_materialized_view('mv', 'SELECT a, b FROM t')]); + + $diff = $comparator->compare($source, $target); + + static::assertSame([], $diff->modifiedMaterializedViews); + } + public function test_materialized_view_modified_index(): void { $constraintComparator = new ConstraintComparator(); @@ -1112,6 +1136,117 @@ public function test_view_added(): void static::assertSame('v_users', $diff->addedViews[0]->name); } + public function test_view_formatting_only_difference_is_not_a_change(): void + { + $constraintComparator = new ConstraintComparator(); + $comparator = new SchemaComparator( + new TableComparator( + new IndexComparator(new GreedySimilarityRenameStrategy(new SimilarTextStrategy())), + $constraintComparator, + new GreedySimilarityRenameStrategy(new SimilarTextStrategy()), + ), + new IndexComparator(new GreedySimilarityRenameStrategy(new SimilarTextStrategy())), + $constraintComparator, + new TableStructureComparator(new GreedySimilarityRenameStrategy(new SimilarTextStrategy())), + ); + $source = new Schema('public', views: [schema_view('v', " SELECT a,\n b\n FROM t;")]); + $target = new Schema('public', views: [schema_view('v', 'SELECT a, b FROM t')]); + + $diff = $comparator->compare($source, $target); + + static::assertSame([], $diff->modifiedViews); + } + + public function test_view_real_change_is_detected(): void + { + $constraintComparator = new ConstraintComparator(); + $comparator = new SchemaComparator( + new TableComparator( + new IndexComparator(new GreedySimilarityRenameStrategy(new SimilarTextStrategy())), + $constraintComparator, + new GreedySimilarityRenameStrategy(new SimilarTextStrategy()), + ), + new IndexComparator(new GreedySimilarityRenameStrategy(new SimilarTextStrategy())), + $constraintComparator, + new TableStructureComparator(new GreedySimilarityRenameStrategy(new SimilarTextStrategy())), + ); + $source = new Schema('public', views: [schema_view('v_users', 'SELECT id FROM users')]); + $target = new Schema('public', views: [schema_view('v_users', 'SELECT id, name FROM users')]); + + $diff = $comparator->compare($source, $target); + + static::assertCount(1, $diff->modifiedViews); + } + + public function test_view_literal_change_is_detected(): void + { + $constraintComparator = new ConstraintComparator(); + $comparator = new SchemaComparator( + new TableComparator( + new IndexComparator(new GreedySimilarityRenameStrategy(new SimilarTextStrategy())), + $constraintComparator, + new GreedySimilarityRenameStrategy(new SimilarTextStrategy()), + ), + new IndexComparator(new GreedySimilarityRenameStrategy(new SimilarTextStrategy())), + $constraintComparator, + new TableStructureComparator(new GreedySimilarityRenameStrategy(new SimilarTextStrategy())), + ); + $source = new Schema('public', views: [schema_view('v_users', 'SELECT id FROM users WHERE x = 1')]); + $target = new Schema('public', views: [schema_view('v_users', 'SELECT id FROM users WHERE x = 2')]); + + $diff = $comparator->compare($source, $target); + + static::assertCount(1, $diff->modifiedViews); + } + + public function test_view_is_updatable_change_is_detected(): void + { + $constraintComparator = new ConstraintComparator(); + $comparator = new SchemaComparator( + new TableComparator( + new IndexComparator(new GreedySimilarityRenameStrategy(new SimilarTextStrategy())), + $constraintComparator, + new GreedySimilarityRenameStrategy(new SimilarTextStrategy()), + ), + new IndexComparator(new GreedySimilarityRenameStrategy(new SimilarTextStrategy())), + $constraintComparator, + new TableStructureComparator(new GreedySimilarityRenameStrategy(new SimilarTextStrategy())), + ); + $source = new Schema('public', views: [schema_view('v_users', "SELECT id\nFROM users", isUpdatable: false)]); + $target = new Schema('public', views: [schema_view('v_users', 'SELECT id FROM users', isUpdatable: true)]); + + $diff = $comparator->compare($source, $target); + + static::assertCount(1, $diff->modifiedViews); + } + + public function test_unparseable_view_definition_falls_back_to_raw_comparison(): void + { + $constraintComparator = new ConstraintComparator(); + $comparator = new SchemaComparator( + new TableComparator( + new IndexComparator(new GreedySimilarityRenameStrategy(new SimilarTextStrategy())), + $constraintComparator, + new GreedySimilarityRenameStrategy(new SimilarTextStrategy()), + ), + new IndexComparator(new GreedySimilarityRenameStrategy(new SimilarTextStrategy())), + $constraintComparator, + new TableStructureComparator(new GreedySimilarityRenameStrategy(new SimilarTextStrategy())), + ); + + $equalGarbage = $comparator->compare( + new Schema('public', views: [schema_view('v_x', 'this is not sql')]), + new Schema('public', views: [schema_view('v_x', 'this is not sql')]), + ); + static::assertSame([], $equalGarbage->modifiedViews); + + $differentGarbage = $comparator->compare( + new Schema('public', views: [schema_view('v_x', 'this is not sql')]), + new Schema('public', views: [schema_view('v_x', 'also not sql')]), + ); + static::assertCount(1, $differentGarbage->modifiedViews); + } + public function test_view_modified_definition(): void { $constraintComparator = new ConstraintComparator(); diff --git a/tools/box/composer.lock b/tools/box/composer.lock index a0727b6056..4ecd976182 100644 --- a/tools/box/composer.lock +++ b/tools/box/composer.lock @@ -532,16 +532,16 @@ }, { "name": "amphp/process", - "version": "v2.0.3", + "version": "v2.1.0", "source": { "type": "git", "url": "https://github.com/amphp/process.git", - "reference": "52e08c09dec7511d5fbc1fb00d3e4e79fc77d58d" + "reference": "583959df17d00304ad7b0b32285373f985935643" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/amphp/process/zipball/52e08c09dec7511d5fbc1fb00d3e4e79fc77d58d", - "reference": "52e08c09dec7511d5fbc1fb00d3e4e79fc77d58d", + "url": "https://api.github.com/repos/amphp/process/zipball/583959df17d00304ad7b0b32285373f985935643", + "reference": "583959df17d00304ad7b0b32285373f985935643", "shasum": "" }, "require": { @@ -555,7 +555,7 @@ "amphp/php-cs-fixer-config": "^2", "amphp/phpunit-util": "^3", "phpunit/phpunit": "^9", - "psalm/phar": "^5.4" + "psalm/phar": "6.16.1" }, "type": "library", "autoload": { @@ -588,7 +588,7 @@ "homepage": "https://amphp.org/process", "support": { "issues": "https://github.com/amphp/process/issues", - "source": "https://github.com/amphp/process/tree/v2.0.3" + "source": "https://github.com/amphp/process/tree/v2.1.0" }, "funding": [ { @@ -596,7 +596,7 @@ "type": "github" } ], - "time": "2024-04-19T03:13:44+00:00" + "time": "2026-05-31T15:11:55+00:00" }, { "name": "amphp/serialization", diff --git a/tools/infection/composer.lock b/tools/infection/composer.lock index b74e254bd0..0da9a71bba 100644 --- a/tools/infection/composer.lock +++ b/tools/infection/composer.lock @@ -1108,16 +1108,16 @@ }, { "name": "sanmai/di-container", - "version": "0.1.12", + "version": "0.1.17", "source": { "type": "git", "url": "https://github.com/sanmai/di-container.git", - "reference": "8b9ad72f6ac1f9e185e5bd060dc9479cb5191d8b" + "reference": "a901c4a8778c9212ef4d66607525281af2f787bd" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/sanmai/di-container/zipball/8b9ad72f6ac1f9e185e5bd060dc9479cb5191d8b", - "reference": "8b9ad72f6ac1f9e185e5bd060dc9479cb5191d8b", + "url": "https://api.github.com/repos/sanmai/di-container/zipball/a901c4a8778c9212ef4d66607525281af2f787bd", + "reference": "a901c4a8778c9212ef4d66607525281af2f787bd", "shasum": "" }, "require": { @@ -1175,7 +1175,7 @@ ], "support": { "issues": "https://github.com/sanmai/di-container/issues", - "source": "https://github.com/sanmai/di-container/tree/0.1.12" + "source": "https://github.com/sanmai/di-container/tree/0.1.17" }, "funding": [ { @@ -1183,7 +1183,7 @@ "type": "github" } ], - "time": "2026-01-27T08:25:46+00:00" + "time": "2026-06-01T08:52:14+00:00" }, { "name": "sanmai/duoclock",