Skip to content
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.

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

Added support functional indexes for MySQL and Postgres #6414

Open
wants to merge 2 commits into
base: 4.3.x
Choose a base branch
from
Open
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
15 changes: 13 additions & 2 deletions src/Driver/AbstractMySQLDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use Doctrine\DBAL\Platforms\MariaDB1052Platform;
use Doctrine\DBAL\Platforms\MariaDB1060Platform;
use Doctrine\DBAL\Platforms\MariaDBPlatform;
use Doctrine\DBAL\Platforms\MySQL8013Platform;
use Doctrine\DBAL\Platforms\MySQL80Platform;
use Doctrine\DBAL\Platforms\MySQL84Platform;
use Doctrine\DBAL\Platforms\MySQLPlatform;
Expand Down Expand Up @@ -47,7 +48,7 @@ public function getDatabasePlatform(ServerVersionProvider $versionProvider): Abs

Deprecation::trigger(
'doctrine/dbal',
'https://github.com/doctrine/dbal/pull/6343',
'https://github.com/doctrine/dbal/pull/6414',
'Support for MariaDB < 10.5.2 is deprecated and will be removed in DBAL 5',
);

Expand All @@ -58,13 +59,23 @@ public function getDatabasePlatform(ServerVersionProvider $versionProvider): Abs
return new MySQL84Platform();
}

if (version_compare($version, '8.0.13', '>=')) {
return new MySQL8013Platform();
}

if (version_compare($version, '8.0.0', '>=')) {
Deprecation::trigger(
'doctrine/dbal',
'https://github.com/doctrine/dbal/pull/6414',
'Support for MySQL <= 8.0.13 is deprecated and will be removed in DBAL 5',
);

return new MySQL80Platform();
derrabus marked this conversation as resolved.
Show resolved Hide resolved
}

Deprecation::trigger(
'doctrine/dbal',
'https://github.com/doctrine/dbal/pull/6343',
'https://github.com/doctrine/dbal/pull/6414',
'Support for MySQL < 8 is deprecated and will be removed in DBAL 5',
);

Expand Down
5 changes: 5 additions & 0 deletions src/Platforms/AbstractMySQLPlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ abstract class AbstractMySQLPlatform extends AbstractPlatform
final public const LENGTH_LIMIT_BLOB = 65535;
final public const LENGTH_LIMIT_MEDIUMBLOB = 16777215;

public function getColumnOrExpressionNameForIndexFetching(): string
{
return 'COLUMN_NAME';
}

protected function doModifyLimitQuery(string $query, ?int $limit, int $offset): string
{
if ($limit !== null) {
Expand Down
38 changes: 38 additions & 0 deletions src/Platforms/AbstractPlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
use function assert;
use function count;
use function explode;
use function get_debug_type;
use function implode;
use function in_array;
use function is_array;
Expand Down Expand Up @@ -794,6 +795,16 @@ private function buildCreateTableSQL(Table $table, bool $createForeignKeys): arr
$options['primary'] = [];

foreach ($table->getIndexes() as $index) {
if ($index->isFunctional() && ! $this->supportsFunctionalIndex()) {
throw new InvalidArgumentException(sprintf(
'Index "%s" on table "%s" contains a functional part, ' .
'but platform "%s" does not support functional indexes.',
$index->getName(),
$table->getName(),
get_debug_type($this),
));
}

if (! $index->isPrimary()) {
$options['indexes'][$index->getQuotedName($this)] = $index;

Expand Down Expand Up @@ -1081,6 +1092,16 @@ public function getCreateIndexSQL(Index $index, string $table): string
));
}

if ($index->isFunctional() && ! $this->supportsFunctionalIndex()) {
throw new InvalidArgumentException(sprintf(
'Index "%s" on table "%s" contains a functional part, ' .
'but platform "%s" does not support functional indexes.',
$name,
$table,
get_debug_type($this),
));
}

if ($index->isPrimary()) {
return $this->getCreatePrimaryKeySQL($index, $table);
}
Expand Down Expand Up @@ -1533,6 +1554,15 @@ public function getIndexDeclarationSQL(Index $index): string
throw new InvalidArgumentException('Incomplete definition. "columns" required.');
}

if ($index->isFunctional() && ! $this->supportsFunctionalIndex()) {
throw new InvalidArgumentException(sprintf(
'Index "%s" contains a functional part, ' .
'but platform "%s" does not support functional indexes.',
$index->getName(),
get_debug_type($this),
));
}

return $this->getCreateIndexSQLFlags($index) . 'INDEX ' . $index->getQuotedName($this)
. ' (' . implode(', ', $index->getQuotedColumns($this)) . ')' . $this->getPartialIndexSQL($index);
}
Expand Down Expand Up @@ -1973,6 +2003,14 @@ public function supportsColumnCollation(): bool
return false;
}

/**
* A flag that indicates whether the platform supports functional indexes.
*/
public function supportsFunctionalIndex(): bool
{
return true;
}
Comment on lines +2006 to +2012
Copy link
Member

Choose a reason for hiding this comment

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

Repeating myself: I don't think we need this supports() function. We've removed a lot of those and I'm not eager to add any new ones.

Copy link
Author

Choose a reason for hiding this comment

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

Why we have supportsPartialIndexes but can't have supportsFunctionalIndex ?
What's the difference ?

Copy link
Member

Choose a reason for hiding this comment

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

The difference is that the method in question was introduced in 2014 and we have 2024 now. Quoting myself:

We've removed a lot of those and I'm not eager to add any new ones.

Copy link
Author

Choose a reason for hiding this comment

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

I can't figure out what the alternative is? If you told me this approach is wrong, what is the right approach?
There are databases that do not supports functional indexes, so we must have a way to check and know that.

Copy link
Author

Choose a reason for hiding this comment

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

I can change the code, but I can't read your thoughts on it.

Copy link
Member

Choose a reason for hiding this comment

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

I can't figure out what the alternative is?

Create a new method in the platform classes that renders DDL for a functional index. Let it throw by default. Override that method inside the platform classes that support functional indexes.

There are databases that do not supports functional indexes, so we must have a way to check and know that.

No, we don't.


/**
* Gets the format string, as accepted by the date() function, that describes
* the format of a stored datetime value of this platform.
Expand Down
5 changes: 5 additions & 0 deletions src/Platforms/MariaDBPlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -162,4 +162,9 @@ protected function createReservedKeywordsList(): KeywordList
{
return new MariaDBKeywords();
}

public function supportsFunctionalIndex(): bool
{
return false;
}
}
26 changes: 26 additions & 0 deletions src/Platforms/MySQL8013Platform.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

declare(strict_types=1);

namespace Doctrine\DBAL\Platforms;

/**
* Provides features of the MySQL since 8.0.13 database platform.
*
* Note: Should not be used with versions prior to 8.0.13.
*/
class MySQL8013Platform extends MySQLPlatform
{
public function getColumnOrExpressionNameForIndexFetching(): string
{
return <<<'SQL'
COALESCE(
COLUMN_NAME,
(CASE WHEN SUBSTR(EXPRESSION, 1, 1) != '('
THEN CONCAT('(', REPLACE(EXPRESSION, '\'', ''''), ')')
ELSE REPLACE(EXPRESSION, '\'', '''')
END)
)
SQL;
}
}
5 changes: 5 additions & 0 deletions src/Platforms/MySQL80Platform.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,9 @@ public function createSelectSQLBuilder(): SelectSQLBuilder
{
return AbstractPlatform::createSelectSQLBuilder();
}

public function supportsFunctionalIndex(): bool
{
return false;
}
}
2 changes: 1 addition & 1 deletion src/Platforms/MySQL84Platform.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
/**
* Provides the behavior, features and SQL dialect of the MySQL 8.4 database platform.
*/
class MySQL84Platform extends MySQL80Platform
class MySQL84Platform extends MySQL8013Platform
{
protected function createReservedKeywordsList(): KeywordList
{
Expand Down
5 changes: 5 additions & 0 deletions src/Platforms/PostgreSQLPlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -781,4 +781,9 @@ public function createSchemaManager(Connection $connection): PostgreSQLSchemaMan
{
return new PostgreSQLSchemaManager($connection, $this);
}

public function supportsFunctionalIndex(): bool
{
return true;
}
}
11 changes: 11 additions & 0 deletions src/Platforms/SQLitePlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
use function array_values;
use function count;
use function explode;
use function get_debug_type;
use function implode;
use function sprintf;
use function str_replace;
Expand Down Expand Up @@ -561,6 +562,16 @@ public function getCreateIndexSQL(Index $index, string $table): string
));
}

if ($index->isFunctional() && ! $this->supportsFunctionalIndex()) {
Copy link
Member

Choose a reason for hiding this comment

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

We're inside a platform class for SQLite. We should know whether SQLite supports functional indexes.

throw new InvalidArgumentException(sprintf(
'Index "%s" on table "%s" contains a functional part, ' .
'but platform "%s" does not support functional indexes.',
$name,
$table,
get_debug_type($this),
));
}

if ($index->isPrimary()) {
return $this->getCreatePrimaryKeySQL($index, $table);
}
Expand Down
30 changes: 27 additions & 3 deletions src/Schema/Index.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
use function array_search;
use function array_shift;
use function count;
use function str_ends_with;
use function str_starts_with;
use function strtolower;

class Index extends AbstractAsset
Expand All @@ -27,6 +29,8 @@ class Index extends AbstractAsset

protected bool $_isPrimary = false;

protected bool $_isFunctional = false;

/**
* Platform specific flags for indexes.
*
Expand Down Expand Up @@ -58,6 +62,12 @@ public function __construct(

foreach ($columns as $column) {
$this->_addColumn($column);

if ($this->_isFunctional === true) {
continue;
}

$this->_isFunctional = self::isColumnNameAnExpression($column);
}

foreach ($flags as $flag) {
Expand Down Expand Up @@ -101,10 +111,14 @@ public function getQuotedColumns(AbstractPlatform $platform): array
foreach ($this->_columns as $column) {
$length = array_shift($subParts);

$quotedColumn = $column->getQuotedName($platform);
if ($this->isFunctional()) {
$quotedColumn = $column->getName();
Copy link
Member

Choose a reason for hiding this comment

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

A functional column does not have a name. We should find a better representation for such columns.

Copy link
Author

Choose a reason for hiding this comment

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

And which representation is better :) Even in databases, functional indexes are stored in a hackish way.

} else {
$quotedColumn = $column->getQuotedName($platform);

if ($length !== null) {
$quotedColumn .= '(' . $length . ')';
if ($length !== null) {
$quotedColumn .= '(' . $length . ')';
}
}

$columns[] = $quotedColumn;
Expand Down Expand Up @@ -137,6 +151,11 @@ public function isPrimary(): bool
return $this->_isPrimary;
}

public function isFunctional(): bool
{
return $this->_isFunctional;
}

public function hasColumnAtPosition(string $name, int $pos = 0): bool
{
$name = $this->trimQuotes(strtolower($name));
Expand Down Expand Up @@ -283,6 +302,11 @@ public function getOptions(): array
return $this->options;
}

public static function isColumnNameAnExpression(string $columnName): bool
{
return str_starts_with($columnName, '(') && str_ends_with($columnName, ')');
}

/**
* Return whether the two indexes have the same partial index
*/
Expand Down
19 changes: 17 additions & 2 deletions src/Schema/MySQLSchemaManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -390,10 +390,12 @@ protected function selectIndexColumns(string $databaseName, ?string $tableName =
$sql .= ' TABLE_NAME,';
}

$sql .= <<<'SQL'
$columnName = $this->getColumnNameForIndexFetch();

$sql .= <<<SQL
NON_UNIQUE AS Non_Unique,
INDEX_NAME AS Key_name,
COLUMN_NAME AS Column_Name,
{$columnName},
SUB_PART AS Sub_Part,
INDEX_TYPE AS Index_Type
FROM information_schema.STATISTICS
Expand Down Expand Up @@ -539,4 +541,17 @@ private function getDefaultTableOptions(): DefaultTableOptions

return $this->defaultTableOptions;
}

/**
* EXPRESSION
*
* MySQL 8.0.13 and higher supports functional key parts (see Functional Key Parts), which affects both
* the COLUMN_NAME and EXPRESSION columns:
* For a nonfunctional key part, COLUMN_NAME indicates the column indexed by the key part and EXPRESSION is NULL.
* For a functional key part, COLUMN_NAME column is NULL and EXPRESSION indicates the expression for the key part.
*/
private function getColumnNameForIndexFetch(): string
{
return $this->platform->getColumnOrExpressionNameForIndexFetching() . ' as Column_Name';
}
}
Loading
Loading