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.

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

Jump to bottom

Fix SQL Server "extended property" SQL generation #6353

Merged
merged 3 commits into from May 3, 2024
Merged

Fix SQL Server "extended property" SQL generation #6353

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0695541
Refactor SQLServerPlatform - prefer null coalescing over cast
mvorisek Apr 7, 2024
8df663a
Deduplicate/unify SQLServerPlatform::getCommentOnTableSQL()
mvorisek Apr 7, 2024
607f7dd
Fix 4283 issue - EXEC sp_addextendedproperty accepts string literals
mvorisek Apr 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
96 changes: 52 additions & 44 deletions src/Platforms/SQLServerPlatform.php
View file
Open in desktop
Expand Up @@ -38,9 +38,12 @@
use function preg_match;
use function preg_match_all;
use function sprintf;
use function str_ends_with;
use function str_replace;
use function str_starts_with;
use function strpos;
use function strtoupper;
use function substr;
use function substr_count;

use const PREG_OFFSET_CAPTURE;
Expand Down Expand Up @@ -399,6 +402,13 @@ public function getCreatePrimaryKeySQL(Index $index, $table)
return $sql . ' (' . $this->getIndexFieldDeclarationListSQL($index) . ')';
}

private function unquoteSingleIdentifier(string $possiblyQuotedName): string
{
return str_starts_with($possiblyQuotedName, '[') && str_ends_with($possiblyQuotedName, ']')
? substr($possiblyQuotedName, 1, -1)
: $possiblyQuotedName;
}

/**
* Returns the SQL statement for creating a column comment.
*
Expand All @@ -419,23 +429,20 @@ public function getCreatePrimaryKeySQL(Index $index, $table)
protected function getCreateColumnCommentSQL($tableName, $columnName, $comment)
{
if (strpos($tableName, '.') !== false) {
[$schemaSQL, $tableSQL] = explode('.', $tableName);
$schemaSQL = $this->quoteStringLiteral($schemaSQL);
$tableSQL = $this->quoteStringLiteral($tableSQL);
[$schemaName, $tableName] = explode('.', $tableName);
} else {
$schemaSQL = "'dbo'";
$tableSQL = $this->quoteStringLiteral($tableName);
$schemaName = 'dbo';
}

return $this->getAddExtendedPropertySQL(
'MS_Description',
$comment,
'SCHEMA',
$schemaSQL,
$this->quoteStringLiteral($this->unquoteSingleIdentifier($schemaName)),
'TABLE',
$tableSQL,
$this->quoteStringLiteral($this->unquoteSingleIdentifier($tableName)),
'COLUMN',
$columnName,
$this->quoteStringLiteral($this->unquoteSingleIdentifier($columnName)),
);
}

Expand Down Expand Up @@ -806,23 +813,20 @@ private function alterColumnRequiresDropDefaultConstraint(ColumnDiff $columnDiff
protected function getAlterColumnCommentSQL($tableName, $columnName, $comment)
{
if (strpos($tableName, '.') !== false) {
[$schemaSQL, $tableSQL] = explode('.', $tableName);
$schemaSQL = $this->quoteStringLiteral($schemaSQL);
$tableSQL = $this->quoteStringLiteral($tableSQL);
[$schemaName, $tableName] = explode('.', $tableName);
} else {
$schemaSQL = "'dbo'";
$tableSQL = $this->quoteStringLiteral($tableName);
$schemaName = 'dbo';
}

return $this->getUpdateExtendedPropertySQL(
'MS_Description',
$comment,
'SCHEMA',
$schemaSQL,
$this->quoteStringLiteral($this->unquoteSingleIdentifier($schemaName)),
'TABLE',
$tableSQL,
$this->quoteStringLiteral($this->unquoteSingleIdentifier($tableName)),
'COLUMN',
$columnName,
$this->quoteStringLiteral($this->unquoteSingleIdentifier($columnName)),
);
}

Expand All @@ -845,22 +849,19 @@ protected function getAlterColumnCommentSQL($tableName, $columnName, $comment)
protected function getDropColumnCommentSQL($tableName, $columnName)
{
if (strpos($tableName, '.') !== false) {
[$schemaSQL, $tableSQL] = explode('.', $tableName);
$schemaSQL = $this->quoteStringLiteral($schemaSQL);
$tableSQL = $this->quoteStringLiteral($tableSQL);
[$schemaName, $tableName] = explode('.', $tableName);
} else {
$schemaSQL = "'dbo'";
$tableSQL = $this->quoteStringLiteral($tableName);
$schemaName = 'dbo';
}

return $this->getDropExtendedPropertySQL(
'MS_Description',
'SCHEMA',
$schemaSQL,
$this->quoteStringLiteral($this->unquoteSingleIdentifier($schemaName)),
'TABLE',
$tableSQL,
$this->quoteStringLiteral($this->unquoteSingleIdentifier($tableName)),
'COLUMN',
$columnName,
$this->quoteStringLiteral($this->unquoteSingleIdentifier($columnName)),
);
}

Expand Down Expand Up @@ -907,10 +908,13 @@ public function getAddExtendedPropertySQL(
$level2Name = null
) {
return 'EXEC sp_addextendedproperty ' .
'N' . $this->quoteStringLiteral($name) . ', N' . $this->quoteStringLiteral((string) $value) . ', ' .
'N' . $this->quoteStringLiteral((string) $level0Type) . ', ' . $level0Name . ', ' .
'N' . $this->quoteStringLiteral((string) $level1Type) . ', ' . $level1Name . ', ' .
'N' . $this->quoteStringLiteral((string) $level2Type) . ', ' . $level2Name;
'N' . $this->quoteStringLiteral($name) . ', N' . $this->quoteStringLiteral($value ?? '') . ', ' .
'N' . $this->quoteStringLiteral($level0Type ?? '') . ', ' . $level0Name . ', ' .
'N' . $this->quoteStringLiteral($level1Type ?? '') . ', ' . $level1Name .
($level2Type !== null || $level2Name !== null
? ', N' . $this->quoteStringLiteral($level2Type ?? '') . ', ' . $level2Name
: ''
);
}

/**
Expand Down Expand Up @@ -941,9 +945,12 @@ public function getDropExtendedPropertySQL(
) {
return 'EXEC sp_dropextendedproperty ' .
'N' . $this->quoteStringLiteral($name) . ', ' .
'N' . $this->quoteStringLiteral((string) $level0Type) . ', ' . $level0Name . ', ' .
'N' . $this->quoteStringLiteral((string) $level1Type) . ', ' . $level1Name . ', ' .
'N' . $this->quoteStringLiteral((string) $level2Type) . ', ' . $level2Name;
'N' . $this->quoteStringLiteral($level0Type ?? '') . ', ' . $level0Name . ', ' .
'N' . $this->quoteStringLiteral($level1Type ?? '') . ', ' . $level1Name .
($level2Type !== null || $level2Name !== null
? ', N' . $this->quoteStringLiteral($level2Type ?? '') . ', ' . $level2Name
: ''
);
}

/**
Expand Down Expand Up @@ -975,10 +982,13 @@ public function getUpdateExtendedPropertySQL(
$level2Name = null
) {
return 'EXEC sp_updateextendedproperty ' .
'N' . $this->quoteStringLiteral($name) . ', N' . $this->quoteStringLiteral((string) $value) . ', ' .
'N' . $this->quoteStringLiteral((string) $level0Type) . ', ' . $level0Name . ', ' .
'N' . $this->quoteStringLiteral((string) $level1Type) . ', ' . $level1Name . ', ' .
'N' . $this->quoteStringLiteral((string) $level2Type) . ', ' . $level2Name;
'N' . $this->quoteStringLiteral($name) . ', N' . $this->quoteStringLiteral($value ?? '') . ', ' .
'N' . $this->quoteStringLiteral($level0Type ?? '') . ', ' . $level0Name . ', ' .
'N' . $this->quoteStringLiteral($level1Type ?? '') . ', ' . $level1Name .
($level2Type !== null || $level2Name !== null
? ', N' . $this->quoteStringLiteral($level2Type ?? '') . ', ' . $level2Name
: ''
);
}

/**
Expand Down Expand Up @@ -1765,15 +1775,13 @@ private function generateIdentifierName($identifier): string

protected function getCommentOnTableSQL(string $tableName, ?string $comment): string
{
return sprintf(
<<<'SQL'
EXEC sys.sp_addextendedproperty @name=N'MS_Description',
@value=N%s, @level0type=N'SCHEMA', @level0name=N'dbo',
@level1type=N'TABLE', @level1name=N%s
SQL
,
$this->quoteStringLiteral((string) $comment),
$this->quoteStringLiteral($tableName),
return $this->getAddExtendedPropertySQL(
'MS_Description',
$comment,
'SCHEMA',
$this->quoteStringLiteral('dbo'),
'TABLE',
$this->quoteStringLiteral($this->unquoteSingleIdentifier($tableName)),
);
}

Expand Down
23 changes: 18 additions & 5 deletions tests/Functional/Schema/SQLServerSchemaManagerTest.php
View file
Open in desktop
Expand Up @@ -97,10 +97,13 @@ public function testDefaultConstraints(): void
self::assertEquals(666, $columns['df_integer']->getDefault());
}

/** @psalm-suppress DeprecatedConstant */
public function testColumnComments(): void
/**
* @dataProvider columnCommentsProvider
* @psalm-suppress DeprecatedConstant
*/
public function testColumnComments(string $tableName): void
{
$table = new Table('sqlsrv_column_comment');
$table = new Table($tableName);
$table->addColumn('id', Types::INTEGER, ['autoincrement' => true]);
$table->addColumn('comment_null', Types::INTEGER, ['comment' => null]);
$table->addColumn('comment_false', Types::INTEGER, ['comment' => false]);
Expand Down Expand Up @@ -130,7 +133,7 @@ public function testColumnComments(): void

$this->schemaManager->createTable($table);

$columns = $this->schemaManager->listTableColumns('sqlsrv_column_comment');
$columns = $this->schemaManager->listTableColumns($tableName);
self::assertCount(13, $columns);
self::assertNull($columns['id']->getComment());
self::assertNull($columns['comment_null']->getComment());
Expand Down Expand Up @@ -209,7 +212,7 @@ public function testColumnComments(): void

$this->schemaManager->alterTable($diff);

$columns = $this->schemaManager->listTableColumns('sqlsrv_column_comment');
$columns = $this->schemaManager->listTableColumns($tableName);
self::assertCount(24, $columns);
self::assertEquals('primary', $columns['id']->getComment());
self::assertNull($columns['comment_null']->getComment());
Expand Down Expand Up @@ -237,6 +240,16 @@ public function testColumnComments(): void
self::assertEquals('Some comment', $columns['commented_req_change_column']->getComment());
}

/** @return mixed[][] */
public static function columnCommentsProvider(): iterable
{
return [
'Simple table name' => ['sqlsrv_column_comment'],
'Quoted table name' => ['[sqlsrv_column_comment quoted]'],
'Quoted table name with schema' => ['[dbo].[sqlsrv_column_comment " with_schema]'],
];
}

public function testPkOrdering(): void
{
// SQL Server stores index column information in a system table with two
Expand Down