Skip to content

Make SchemaTool use the new DBAL API #12113

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: 3.5.x
Choose a base branch
from
Open

Make SchemaTool use the new DBAL API #12113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 2 additions & 38 deletions phpstan-baseline.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3150,7 +3150,7 @@ parameters:
Use \{@see getPrimaryKeyConstraint\(\)\} instead\.$#
'''
identifier: method.deprecated
count: 1
count: 2
path: src/Tools/SchemaTool.php

-
Expand Down Expand Up @@ -3255,12 +3255,6 @@ parameters:
count: 1
path: src/Tools/SchemaTool.php

-
message: '#^Method Doctrine\\ORM\\Tools\\SchemaTool\:\:getIndexColumns\(\) has parameter \$class with generic class Doctrine\\ORM\\Mapping\\ClassMetadata but does not specify its types\: T$#'
identifier: missingType.generics
count: 1
path: src/Tools/SchemaTool.php

-
message: '#^Method Doctrine\\ORM\\Tools\\SchemaTool\:\:getSchemaFromMetadata\(\) has parameter \$classes with generic class Doctrine\\ORM\\Mapping\\ClassMetadata but does not specify its types\: T$#'
identifier: missingType.generics
Expand Down Expand Up @@ -3291,38 +3285,14 @@ parameters:
count: 1
path: src/Tools/SchemaTool.php

-
message: '#^Parameter \#1 \$columnNames of method Doctrine\\DBAL\\Schema\\Table\:\:addIndex\(\) expects non\-empty\-list\<string\>, list\<string\> given\.$#'
identifier: argument.type
count: 1
path: src/Tools/SchemaTool.php

-
message: '#^Parameter \#1 \$columnNames of method Doctrine\\DBAL\\Schema\\Table\:\:addUniqueIndex\(\) expects non\-empty\-list\<string\>, array\<string\> given\.$#'
identifier: argument.type
count: 1
path: src/Tools/SchemaTool.php

-
message: '#^Parameter \#1 \$columnNames of method Doctrine\\DBAL\\Schema\\Table\:\:setPrimaryKey\(\) expects non\-empty\-list\<string\>, array\<string\> given\.$#'
identifier: argument.type
count: 1
path: src/Tools/SchemaTool.php

-
message: '#^Parameter \#1 \$value of static method Doctrine\\DBAL\\Schema\\Name\\Identifier\:\:unquoted\(\) expects non\-empty\-string, string given\.$#'
identifier: argument.type
count: 1
path: src/Tools/SchemaTool.php

-
message: '#^Parameter \#2 \$columnNames of class Doctrine\\DBAL\\Schema\\PrimaryKeyConstraint constructor expects non\-empty\-list\<Doctrine\\DBAL\\Schema\\Name\\UnqualifiedName\>, list\<Doctrine\\DBAL\\Schema\\Name\\UnqualifiedName\> given\.$#'
identifier: argument.type
count: 1
path: src/Tools/SchemaTool.php

-
message: '#^Parameter \#2 \$columns of class Doctrine\\DBAL\\Schema\\Index constructor expects non\-empty\-list\<string\>, list\<string\> given\.$#'
message: '#^Parameter \#1 \$firstColumnName of method Doctrine\\DBAL\\Schema\\PrimaryKeyConstraintEditor\:\:setUnquotedColumnNames\(\) expects non\-empty\-string, string given\.$#'
Copy link
Member

@greg0ire greg0ire Aug 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that we have identifiers at our disposal, I think we should not add new issues to the baseline, and instead resort to e.g. /** @phpstan-ignore argument.type (reason why we ignore this) */

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'll try to do that when I rework the PR.

identifier: argument.type
count: 1
path: src/Tools/SchemaTool.php
Expand All @@ -3339,12 +3309,6 @@ parameters:
count: 1
path: src/Tools/SchemaTool.php

-
message: '#^Property Doctrine\\ORM\\Tools\\SchemaTool\:\:\$schemaManager with generic class Doctrine\\DBAL\\Schema\\AbstractSchemaManager does not specify its types\: T$#'
identifier: missingType.generics
count: 1
path: src/Tools/SchemaTool.php

-
message: '#^Access to an undefined property Doctrine\\ORM\\Mapping\\ManyToManyInverseSideMapping\|Doctrine\\ORM\\Mapping\\ManyToManyOwningSideMapping\|Doctrine\\ORM\\Mapping\\ManyToOneAssociationMapping\|Doctrine\\ORM\\Mapping\\OneToManyAssociationMapping\|Doctrine\\ORM\\Mapping\\OneToOneInverseSideMapping\|Doctrine\\ORM\\Mapping\\OneToOneOwningSideMapping\:\:\$inversedBy\.$#'
identifier: property.notFound
Expand Down
3 changes: 3 additions & 0 deletions phpstan-dbal3.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ parameters:
identifier: argument.unresolvableType
path: src/Mapping/Driver/DatabaseDriver.php

- '~undefined static method Doctrine\\DBAL\\Schema\\Index\:\:editor\(\)~'
- '~^Method Doctrine\\ORM\\Tools\\SchemaTool\:\:getIndexedColumns\(\) should return non-empty-list<string>~'

# To be removed in 4.0
-
message: '#Negated boolean expression is always false\.#'
Expand Down
113 changes: 85 additions & 28 deletions src/Tools/SchemaTool.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
use Doctrine\DBAL\Schema\ForeignKeyConstraintEditor;
use Doctrine\DBAL\Schema\Index;
use Doctrine\DBAL\Schema\Index\IndexedColumn;
use Doctrine\DBAL\Schema\Name\Identifier;
use Doctrine\DBAL\Schema\IndexEditor;
use Doctrine\DBAL\Schema\Name\UnqualifiedName;
use Doctrine\DBAL\Schema\PrimaryKeyConstraint;
use Doctrine\DBAL\Schema\Schema;
Expand All @@ -38,6 +38,7 @@
use function array_flip;
use function array_intersect_key;
use function array_map;
use function array_values;
use function assert;
use function class_exists;
use function count;
Expand All @@ -60,6 +61,7 @@ class SchemaTool

private readonly AbstractPlatform $platform;
private readonly QuoteStrategy $quoteStrategy;
/** @var AbstractSchemaManager<AbstractPlatform> */
private readonly AbstractSchemaManager $schemaManager;

/**
Expand Down Expand Up @@ -128,9 +130,10 @@ private function processingNotRequired(
/**
* Resolves fields in index mapping to column names
*
* @param mixed[] $indexData index or unique constraint data
* @param ClassMetadata<object> $class
* @param mixed[] $indexData index or unique constraint data
*
* @return list<string> Column names from combined fields and columns mappings
* @return non-empty-list<non-empty-string> Column names from combined fields and columns mappings
*/
private function getIndexColumns(ClassMetadata $class, array $indexData): array
{
Expand Down Expand Up @@ -167,6 +170,13 @@ private function getIndexColumns(ClassMetadata $class, array $indexData): array
}
}

if ($columns === []) {
throw MappingException::invalidIndexConfiguration(
(string) $class,
$indexData['name'] ?? 'unnamed',
);
}

return $columns;
}

Expand Down Expand Up @@ -314,17 +324,13 @@ public function getSchemaFromMetadata(array $classes): Schema
}
}

if (! $table->hasIndex('primary')) {
self::addPrimaryKeyConstraint($table, $pkColumns);
}
$primaryKey = $this->getPrimaryKeyConstraint($table) ?? $this->addPrimaryKeyConstraint($table, $pkColumns);

// there can be unique indexes automatically created for join column
// if join column is also primary key we should keep only primary key on this column
// so, remove indexes overruled by primary key
$primaryKey = $table->getIndex('primary');

foreach ($table->getIndexes() as $idxKey => $existingIndex) {
if ($existingIndex !== $primaryKey && $primaryKey->spansColumns(self::getIndexedColumns($existingIndex))) {
if ($idxKey !== 'primary' && $this->doesIndexOverlapWithPrimaryKey($existingIndex, $primaryKey)) {
$table->dropIndex($idxKey);
}
}
Expand All @@ -346,7 +352,7 @@ public function getSchemaFromMetadata(array $classes): Schema

if (isset($class->table['uniqueConstraints'])) {
foreach ($class->table['uniqueConstraints'] as $indexName => $indexData) {
$uniqIndex = new Index('tmp__' . $indexName, $this->getIndexColumns($class, $indexData), true, false, [], $indexData['options'] ?? []);
$uniqIndex = $this->createIndexForComparison('tmp__' . $indexName, $this->getIndexColumns($class, $indexData), $indexData['options'] ?? []);

foreach ($table->getIndexes() as $tableIndexName => $tableIndex) {
if ($tableIndex->isFulfilledBy($uniqIndex)) {
Expand Down Expand Up @@ -872,11 +878,7 @@ public function getDropSchemaSQL(array $classes): array
}

foreach ($schema->getTables() as $table) {
if (method_exists($table, 'getPrimaryKeyConstraint')) {
$primaryKey = $table->getPrimaryKeyConstraint();
} else {
$primaryKey = $table->getPrimaryKey();
}
$primaryKey = $this->getPrimaryKeyConstraint($table);

if ($primaryKey === null) {
continue;
Expand Down Expand Up @@ -969,29 +971,84 @@ private function createSchemaForComparison(Schema $toSchema): Schema
}
}

/** @param string[] $primaryKeyColumns */
private function addPrimaryKeyConstraint(Table $table, array $primaryKeyColumns): void
private function getPrimaryKeyConstraint(Table $table): PrimaryKeyConstraint|Index|null
{
if (class_exists(PrimaryKeyConstraint::class)) {
$primaryKeyColumnNames = [];
// DBAL < 4.3
if (! method_exists($table, 'getPrimaryKeyConstraint')) {
return $table->getPrimaryKey();
}

foreach ($primaryKeyColumns as $primaryKeyColumn) {
$primaryKeyColumnNames[] = new UnqualifiedName(Identifier::unquoted($primaryKeyColumn));
}
return $table->getPrimaryKeyConstraint();
}

$table->addPrimaryKeyConstraint(new PrimaryKeyConstraint(null, $primaryKeyColumnNames, true));
} else {
/** @param string[] $primaryKeyColumns */
Copy link
Member

@greg0ire greg0ire Aug 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use the same advanced types you used somewhere else in this PR? Maybe it will address the baselined issue.

Suggested change
/** @param string[] $primaryKeyColumns */
/** @param non-empty-list<non-empty-string> $primaryKeyColumns */

private function addPrimaryKeyConstraint(Table $table, array $primaryKeyColumns): PrimaryKeyConstraint|Index
{
// DBAL < 4.3
if (! class_exists(PrimaryKeyConstraint::class)) {
$table->setPrimaryKey($primaryKeyColumns);

return $table->getPrimaryKey();
}

$primaryKeyConstraint = PrimaryKeyConstraint::editor()
->setUnquotedColumnNames(...array_values($primaryKeyColumns))
->setIsClustered(true)
->create();

$table->addPrimaryKeyConstraint($primaryKeyConstraint);

return $table->getPrimaryKeyConstraint();
}

/** @return string[] */
/** @return non-empty-list<string> */
private static function getIndexedColumns(Index $index): array
{
if (method_exists(Index::class, 'getIndexedColumns')) {
return array_map(static fn (IndexedColumn $indexedColumn) => $indexedColumn->getColumnName()->toString(), $index->getIndexedColumns());
// DBAL < 4.3
if (! method_exists(Index::class, 'getIndexedColumns')) {
return $index->getColumns();
}

return array_map(static fn (IndexedColumn $indexedColumn) => $indexedColumn->getColumnName()->getIdentifier()->getValue(), $index->getIndexedColumns());
}

private function doesIndexOverlapWithPrimaryKey(Index $index, PrimaryKeyConstraint|Index $primaryKey): bool
{
// DBAL < 4.3
if ($primaryKey instanceof Index) {
return $index !== $primaryKey && $primaryKey->spansColumns(self::getIndexedColumns($index));
}

$indexedColumns = $index->getIndexedColumns();
foreach ($primaryKey->getColumnNames() as $i => $column) {
if (
! isset($indexedColumns[$i])
|| strtolower($column->getIdentifier()->getValue())
!== strtolower($indexedColumns[$i]->getColumnName()->getIdentifier()->getValue())
) {
return false;
}
}

return true;
}

/**
* @param non-empty-string $indexName
* @param non-empty-list<non-empty-string> $indexColumns
* @param array<string, mixed> $indexOptions
*/
private function createIndexForComparison(string $indexName, array $indexColumns, mixed $indexOptions): Index
{
// DBAL < 4.3
if (! class_exists(IndexEditor::class)) {
return new Index($indexName, $indexColumns, true, false, [], $indexOptions);
}

return $index->getColumns();
return Index::editor()
->setUnquotedName($indexName)
Copy link
Member

@morozov morozov Aug 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would double-check how this code behaves if the index name contains quotes (e.g. "123_index"). Previously, Index would parse it resulting in quoted name 123_index. In the new version, it will result in an unquoted name "123_index" (i.e. quotes will be included in the name, not part of the syntax).

If that's the case, you'll need to parse the string with UnqualifiedNameParser and use setName() instead of setUnquotedName().

->setUnquotedColumnNames(...$indexColumns)
->setType(Index\IndexType::UNIQUE)
->create();
}
}
Loading