Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 12 additions & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,18 @@ awareness about deprecated code.

# Upgrade to 5.0

## BC BREAK: Changes in `Index` behavior

The following `Index` usage scenarios are no longer supported:

1. Instantiation of an index with empty columns
2. Instantiation of a primary key index with column lengths specified
3. Using qualified or otherwise invalid column names in index columns
4. Using other values than positive integers as index column lengths
5. Using nullable columns in a primary key index

The `Index` class has been declared as final.

## BC BREAK: Removed `AbstractSchemaManager` methods

The following `AbstractSchemaManager` methods have been removed:
Expand Down
42 changes: 42 additions & 0 deletions src/Schema/Exception/InvalidIndexDefinition.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<?php

declare(strict_types=1);

namespace Doctrine\DBAL\Schema\Exception;

use Doctrine\DBAL\Schema\Name\UnqualifiedName;
use Doctrine\DBAL\Schema\SchemaException;
use LogicException;

use function gettype;
use function is_object;
use function sprintf;

final class InvalidIndexDefinition extends LogicException implements SchemaException
{
public static function columnNamesNotSet(): self
{
return new self('Index column names are not set.');
}

public static function primaryKeyIndexHasColumnLengths(): self
{
return new self('Primary key index cannot have column lengths.');
}

public static function primaryKeyIndexOnANullableColumn(UnqualifiedName $columnName): self
{
return new self(sprintf(
'Primary key index cannot use nullable column %s.',
$columnName->toString(),
));
}

public static function invalidColumnLength(mixed $length): self
{
return new self(sprintf(
'Indexed column length must be an integer, %s given.',
is_object($length) ? $length::class : gettype($length),
));
}
}
6 changes: 0 additions & 6 deletions src/Schema/Exception/InvalidState.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,6 @@
use Doctrine\DBAL\Schema\SchemaException;
use LogicException;

use function sprintf;

final class InvalidState extends LogicException implements SchemaException
{
public static function indexHasInvalidColumns(string $indexName): self
{
return new self(sprintf('Index "%s" has invalid columns.', $indexName));
}
}
91 changes: 21 additions & 70 deletions src/Schema/Index.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,55 +5,48 @@
namespace Doctrine\DBAL\Schema;

use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Schema\Exception\InvalidState;
use Doctrine\DBAL\Schema\Exception\InvalidIndexDefinition;
use Doctrine\DBAL\Schema\Exception\InvalidName;
use Doctrine\DBAL\Schema\Index\IndexedColumn;
use Doctrine\DBAL\Schema\Name\Parser;
use Doctrine\DBAL\Schema\Name\Parser\UnqualifiedNameParser;
use Doctrine\DBAL\Schema\Name\Parsers;
use Doctrine\DBAL\Schema\Name\UnqualifiedName;
use Doctrine\Deprecations\Deprecation;
use Throwable;

use function array_filter;
use function array_keys;
use function array_map;
use function array_search;
use function array_shift;
use function count;
use function gettype;
use function is_int;
use function is_object;
use function strtolower;

/**
* @final
* @extends AbstractNamedObject<UnqualifiedName>
*/
/** @extends AbstractNamedObject<UnqualifiedName> */
final class Index extends AbstractNamedObject
{
/**
* Asset identifier instances of the column names the index is associated with.
*
* @var array<string, Identifier>
* @var non-empty-array<string, Identifier>
*/
protected array $_columns = [];
private readonly array $_columns;
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this is a good occasion to finally drop these underscores?

Copy link
Member Author

Choose a reason for hiding this comment

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

Initially I did that but then decided to revert. The reasons are:

  1. We already have $_columns and $colums, so the former would have to be renamed to say $columnNames, but then the getter that returns it would still remain named getColumns(), so the code would get a bit inconsistent.
  2. This way, the $isPrimary constructor parameter would get promoted to a property but $isUnique wouldn't, so again, the could would become a bit more inconsistent.

Eventually, all these properties will go away:

  1. $_columns will get removed in favor of $columns.
  2. $_isPrimary will get removed in favor of PrimaryKeyConstraint.
  3. $_isUnique and $_flags will get replaced by some other properties because currently an index can be UNIQUE, FULLTEXT and SPATIAL at the same time, but it doesn't make sense.


protected bool $_isUnique = false;
private readonly bool $_isUnique;

protected bool $_isPrimary = false;
private readonly bool $_isPrimary;

/**
* Platform specific flags for indexes.
*
* @var array<string, true>
*/
protected array $_flags = [];
private array $_flags = [];

/**
* Column the index is associated with.
*
* An empty list indicates that an attempt to parse indexed columns failed.
*
* @var list<IndexedColumn>
* @var non-empty-list<IndexedColumn>
*/
private readonly array $columns;

Expand All @@ -73,20 +66,19 @@ public function __construct(
parent::__construct($name ?? '');

if (count($columns) < 1) {
Deprecation::trigger(
'doctrine/dbal',
'https://github.com/doctrine/dbal/pull/6787',
'Instantiation of an index without column names is deprecated.',
);
throw InvalidIndexDefinition::columnNamesNotSet();
}

$this->_isUnique = $isUnique || $isPrimary;
$this->_isPrimary = $isPrimary;

$identifiers = [];
foreach ($columns as $column) {
$this->_addColumn($column);
$identifiers[$column] = new Identifier($column);
}

$this->_columns = $identifiers;

foreach ($flags as $flag) {
$this->addFlag($flag);
}
Expand All @@ -106,26 +98,16 @@ protected function getNameParser(): UnqualifiedNameParser
*/
public function getIndexedColumns(): array
{
if (count($this->columns) < 1) {
throw InvalidState::indexHasInvalidColumns($this->getName());
}

return $this->columns;
}

protected function _addColumn(string $column): void
{
$this->_columns[$column] = new Identifier($column);
}

/**
* Returns the names of the referencing table columns the constraint is associated with.
*
* @return non-empty-list<string>
*/
public function getColumns(): array
{
/** @phpstan-ignore return.type */
return array_keys($this->_columns);
}

Expand Down Expand Up @@ -336,7 +318,7 @@ public function getOptions(): array
* @param non-empty-array<int, string> $columnNames
* @param array<int> $lengths
*
* @return list<IndexedColumn>
* @return non-empty-list<IndexedColumn>
*/
private function parseColumns(bool $isPrimary, array $columnNames, array $lengths): array
{
Expand All @@ -347,50 +329,19 @@ private function parseColumns(bool $isPrimary, array $columnNames, array $length
foreach ($columnNames as $columnName) {
try {
$parsedName = $parser->parse($columnName);
} catch (Throwable $e) {
Deprecation::trigger(
'doctrine/dbal',
'https://github.com/doctrine/dbal/pull/6787',
'Unable to parse column name: %s.',
$e->getMessage(),
);

return [];
} catch (Parser\Exception $e) {
throw InvalidName::fromParserException($columnName, $e);
}

$length = array_shift($lengths);

if ($length !== null) {
if ($isPrimary) {
Deprecation::trigger(
'doctrine/dbal',
'https://github.com/doctrine/dbal/pull/6787',
'Declaring column length for primary key indexes is deprecated.',
);

return [];
}

if (! is_int($length)) {
Deprecation::trigger(
'doctrine/dbal',
'https://github.com/doctrine/dbal/pull/6787',
'Indexed column length should be an integer, %s given.',
is_object($length) ? $length::class : gettype($length),
);

$length = (int) $length;
throw InvalidIndexDefinition::primaryKeyIndexHasColumnLengths();
}

if ($length < 1) {
Deprecation::trigger(
'doctrine/dbal',
'https://github.com/doctrine/dbal/pull/6787',
'Indexed column length should be a positive integer, %d given.',
$length,
);

return [];
if (! is_int($length) || $length < 1) {
throw InvalidIndexDefinition::invalidColumnLength($length);
}
}

Expand Down
9 changes: 2 additions & 7 deletions src/Schema/Table.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use Doctrine\DBAL\Schema\Exception\IndexDoesNotExist;
use Doctrine\DBAL\Schema\Exception\IndexNameInvalid;
use Doctrine\DBAL\Schema\Exception\InvalidForeignKeyConstraintDefinition;
use Doctrine\DBAL\Schema\Exception\InvalidIndexDefinition;
use Doctrine\DBAL\Schema\Exception\InvalidName;
use Doctrine\DBAL\Schema\Exception\InvalidTableName;
use Doctrine\DBAL\Schema\Exception\PrimaryKeyAlreadyExists;
Expand Down Expand Up @@ -144,14 +145,8 @@ public function setPrimaryKey(array $columnNames, ?string $indexName = null): se
$column = $this->getColumn($columnName);

if (! $column->getNotnull()) {
Deprecation::trigger(
'doctrine/dbal',
'https://github.com/doctrine/dbal/pull/6787',
'Using nullable columns in a primary key index is deprecated.',
);
throw InvalidIndexDefinition::primaryKeyIndexOnANullableColumn($column->getObjectName());
}

$column->setNotnull(true);
}

return $this;
Expand Down
3 changes: 0 additions & 3 deletions tests/DriverManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
use Doctrine\DBAL\DriverManager;
use Doctrine\DBAL\Exception;
use Doctrine\DBAL\Tools\DsnParser;
use Doctrine\Deprecations\PHPUnit\VerifyDeprecations;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\Attributes\RequiresPhpExtension;
use PHPUnit\Framework\TestCase;
Expand All @@ -22,8 +21,6 @@
/** @phpstan-import-type Params from DriverManager */
class DriverManagerTest extends TestCase
{
use VerifyDeprecations;

public function testCheckParams(): void
{
$this->expectException(Exception::class);
Expand Down
2 changes: 1 addition & 1 deletion tests/Functional/TypeConversionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class TypeConversionTest extends FunctionalTestCase
protected function setUp(): void
{
$table = new Table('type_conversion');
$table->addColumn('id', Types::INTEGER, ['notnull' => false]);
$table->addColumn('id', Types::INTEGER);
$table->addColumn('test_string', Types::STRING, [
'length' => 16,
'notnull' => false,
Expand Down
3 changes: 0 additions & 3 deletions tests/Platforms/MySQLPlatformTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,9 @@
use Doctrine\DBAL\Schema\Table;
use Doctrine\DBAL\TransactionIsolationLevel;
use Doctrine\DBAL\Types\Types;
use Doctrine\Deprecations\PHPUnit\VerifyDeprecations;

class MySQLPlatformTest extends AbstractMySQLPlatformTestCase
{
use VerifyDeprecations;

public function createPlatform(): AbstractPlatform
{
return new MySQLPlatform();
Expand Down
3 changes: 0 additions & 3 deletions tests/Schema/AbstractNamedObjectTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,10 @@
use Doctrine\DBAL\Schema\AbstractNamedObject;
use Doctrine\DBAL\Schema\Exception\InvalidName;
use Doctrine\DBAL\Schema\Name;
use Doctrine\Deprecations\PHPUnit\VerifyDeprecations;
use PHPUnit\Framework\TestCase;

class AbstractNamedObjectTest extends TestCase
{
use VerifyDeprecations;

public function testEmptyName(): void
{
$this->expectException(InvalidName::class);
Expand Down
3 changes: 0 additions & 3 deletions tests/Schema/ColumnTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,11 @@
use Doctrine\DBAL\Schema\Name\Identifier;
use Doctrine\DBAL\Types\Type;
use Doctrine\DBAL\Types\Types;
use Doctrine\Deprecations\PHPUnit\VerifyDeprecations;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\TestCase;

class ColumnTest extends TestCase
{
use VerifyDeprecations;

public function testGet(): void
{
$column = $this->createColumn();
Expand Down
3 changes: 0 additions & 3 deletions tests/Schema/IdentifierTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,10 @@
use Doctrine\DBAL\Exception;
use Doctrine\DBAL\Schema\Exception\InvalidName;
use Doctrine\DBAL\Schema\Identifier;
use Doctrine\Deprecations\PHPUnit\VerifyDeprecations;
use PHPUnit\Framework\TestCase;

class IdentifierTest extends TestCase
{
use VerifyDeprecations;

public function testEmptyName(): void
{
$this->expectException(InvalidName::class);
Expand Down
Loading
Loading