From 2f6f4953e47438d5568c152b658517e18b1ddcfa Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sat, 2 Nov 2024 16:28:00 +0700 Subject: [PATCH 1/3] Refactor `ColumnFactory` --- src/Column/ColumnFactory.php | 28 ++---- src/Schema.php | 110 ++++++++++------------ tests/CommandTest.php | 3 +- tests/Provider/SchemaProvider.php | 36 +++---- tests/Provider/Type/BinaryProvider.php | 4 +- tests/Provider/Type/CharProvider.php | 4 +- tests/Provider/Type/VarBinaryProvider.php | 6 +- tests/Provider/Type/VarCharProvider.php | 6 +- tests/QueryBuilderTest.php | 9 +- tests/SchemaTest.php | 12 --- tests/Type/DecimalTest.php | 4 +- tests/Type/NumericTest.php | 4 +- 12 files changed, 97 insertions(+), 129 deletions(-) diff --git a/src/Column/ColumnFactory.php b/src/Column/ColumnFactory.php index 4e3b5a658..5b29029d5 100644 --- a/src/Column/ColumnFactory.php +++ b/src/Column/ColumnFactory.php @@ -9,7 +9,6 @@ use Yiisoft\Db\Expression\Expression; use Yiisoft\Db\Schema\Column\AbstractColumnFactory; use Yiisoft\Db\Schema\Column\ColumnSchemaInterface; -use Yiisoft\Db\Schema\Column\StringColumnSchema; final class ColumnFactory extends AbstractColumnFactory { @@ -17,8 +16,9 @@ final class ColumnFactory extends AbstractColumnFactory * Mapping from physical column types (keys) to abstract column types (values). * * @var string[] + * @psalm-var array */ - private const TYPE_MAP = [ + protected const TYPE_MAP = [ /** Exact numbers */ 'bit' => ColumnType::BOOLEAN, 'tinyint' => ColumnType::TINYINT, @@ -69,37 +69,21 @@ final class ColumnFactory extends AbstractColumnFactory 'table' => ColumnType::STRING, ]; - protected function getType(string $dbType, array $info = []): string - { - return self::TYPE_MAP[$dbType] ?? ColumnType::STRING; - } - public function fromPseudoType(string $pseudoType, array $info = []): ColumnSchemaInterface { - if ($pseudoType === PseudoType::UUID_PK_SEQ) { - unset($info['type']); - $info['primaryKey'] = true; - $info['autoIncrement'] = true; + if ($pseudoType === PseudoType::UUID_PK_SEQ && !isset($info['defaultValue'])) { $info['defaultValue'] = new Expression('newsequentialid()'); - - return new StringColumnSchema(ColumnType::UUID, ...$info); } return parent::fromPseudoType($pseudoType, $info); } - public function fromType(string $type, array $info = []): ColumnSchemaInterface + protected function getColumnClass(string $type, array $info = []): string { if ($type === ColumnType::BINARY) { - unset($info['type']); - return new BinaryColumnSchema($type, ...$info); + return BinaryColumnSchema::class; } - return parent::fromType($type, $info); - } - - protected function isDbType(string $dbType): bool - { - return isset(self::TYPE_MAP[$dbType]); + return parent::getColumnClass($type, $info); } } diff --git a/src/Schema.php b/src/Schema.php index fb6899bca..e74d5551f 100644 --- a/src/Schema.php +++ b/src/Schema.php @@ -21,27 +21,30 @@ use Yiisoft\Db\Schema\TableSchemaInterface; use function array_change_key_case; +use function array_fill_keys; use function array_map; use function is_array; use function md5; use function preg_match; use function serialize; use function str_replace; -use function strcasecmp; /** * Implements the MSSQL Server specific schema, supporting MSSQL Server 2017 and above. * * @psalm-type ColumnArray = array{ * column_name: string, + * column_default: string|null, * is_nullable: string, * data_type: string, - * column_default: string|null, + * size: int|string|null, + * numeric_scale: int|string|null, * is_identity: string, * is_computed: string, - * comment: null|string, - * size?: int, - * scale?: int, + * comment: string|null, + * primaryKey: bool, + * schema: string|null, + * table: string * } * @psalm-type ConstraintArray = array< * array-key, @@ -366,18 +369,19 @@ protected function loadTableDefaultValues(string $tableName): array */ private function loadColumnSchema(array $info): ColumnSchemaInterface { - $columnFactory = $this->getColumnFactory(); - - $dbType = $info['data_type']; - /** @psalm-var ColumnArray $info */ - $column = $columnFactory->fromDefinition($dbType); - /** @psalm-suppress DeprecatedMethod */ - $column->name($info['column_name']); - $column->notNull($info['is_nullable'] !== 'YES'); - $column->dbType($dbType); - $column->autoIncrement($info['is_identity'] === '1'); - $column->computed($info['is_computed'] === '1'); - $column->comment($info['comment'] ?? ''); + $column = $this->getColumnFactory()->fromDbType($info['data_type'], [ + 'autoIncrement' => $info['is_identity'] === '1', + 'comment' => $info['comment'], + 'computed' => $info['is_computed'] === '1', + 'name' => $info['column_name'], + 'notNull' => $info['is_nullable'] !== 'YES', + 'primaryKey' => $info['primaryKey'], + 'scale' => $info['numeric_scale'] !== null ? (int) $info['numeric_scale'] : null, + 'schema' => $info['schema'], + 'size' => $info['size'] !== null ? (int) $info['size'] : null, + 'table' => $info['table'], + ]); + $column->defaultValue($this->normalizeDefaultValue($info['column_default'], $column)); return $column; @@ -420,61 +424,46 @@ private function normalizeDefaultValue(string|null $defaultValue, ColumnSchemaIn */ protected function findColumns(TableSchemaInterface $table): bool { - $columnsTableName = 'INFORMATION_SCHEMA.COLUMNS'; + $schemaName = $table->getSchemaName(); + $tableName = $table->getName(); - $whereParams = [':table_name' => $table->getName()]; + $columnsTableName = '[INFORMATION_SCHEMA].[COLUMNS]'; $whereSql = '[t1].[table_name] = :table_name'; + $whereParams = [':table_name' => $tableName]; if ($table->getCatalogName() !== null) { - $columnsTableName = "{$table->getCatalogName()}.$columnsTableName"; + $columnsTableName = "[{$table->getCatalogName()}].$columnsTableName"; $whereSql .= ' AND [t1].[table_catalog] = :catalog'; $whereParams[':catalog'] = $table->getCatalogName(); } - if ($table->getSchemaName() !== null) { - $whereSql .= " AND [t1].[table_schema] = '{$table->getSchemaName()}'"; + if ($schemaName !== null) { + $whereSql .= " AND [t1].[table_schema] = :schema_name"; + $whereParams[':schema_name'] = $schemaName; } - $columnsTableName = $this->db->getQuoter()->quoteTableName($columnsTableName); - $sql = <<db->createCommand($sql, $whereParams)->queryAll(); if (empty($columns)) { @@ -484,14 +473,17 @@ protected function findColumns(TableSchemaInterface $table): bool return false; } + $primaryKeys = array_fill_keys($table->getPrimaryKey(), true); + foreach ($columns as $info) { + $info = array_change_key_case($info); + + /** @psalm-var ColumnArray $info */ + $info['primaryKey'] = isset($primaryKeys[$info['column_name']]); + $info['schema'] = $schemaName; + $info['table'] = $tableName; + $column = $this->loadColumnSchema($info); - foreach ($table->getPrimaryKey() as $primaryKey) { - if (strcasecmp($info['column_name'], $primaryKey) === 0) { - $column->primaryKey(true); - break; - } - } if ($column->isPrimaryKey() && $column->isAutoIncrement()) { $table->sequenceName(''); diff --git a/tests/CommandTest.php b/tests/CommandTest.php index e3e50bfe2..301b76c6c 100644 --- a/tests/CommandTest.php +++ b/tests/CommandTest.php @@ -343,7 +343,8 @@ public function testAlterColumnWithDefaultNull() $this->assertTrue($fieldCol->isNotNull()); $this->assertNull($fieldCol->getDefaultValue()); - $this->assertSame('nvarchar(40)', $fieldCol->getDbType()); + $this->assertSame('nvarchar', $fieldCol->getDbType()); + $this->assertSame(40, $fieldCol->getSize()); $command->dropTable('column_with_constraint'); } diff --git a/tests/Provider/SchemaProvider.php b/tests/Provider/SchemaProvider.php index d53db017e..040ddccd3 100644 --- a/tests/Provider/SchemaProvider.php +++ b/tests/Provider/SchemaProvider.php @@ -22,8 +22,8 @@ public static function columns(): array 'notNull' => true, 'autoIncrement' => false, 'enumValues' => null, - 'size' => null, - 'scale' => null, + 'size' => 10, + 'scale' => 0, 'defaultValue' => null, ], 'int_col2' => [ @@ -34,8 +34,8 @@ public static function columns(): array 'notNull' => false, 'autoIncrement' => false, 'enumValues' => null, - 'size' => null, - 'scale' => null, + 'size' => 10, + 'scale' => 0, 'defaultValue' => 1, ], 'tinyint_col' => [ @@ -46,8 +46,8 @@ public static function columns(): array 'notNull' => false, 'autoIncrement' => false, 'enumValues' => null, - 'size' => null, - 'scale' => null, + 'size' => 3, + 'scale' => 0, 'defaultValue' => 1, ], 'smallint_col' => [ @@ -58,13 +58,13 @@ public static function columns(): array 'notNull' => false, 'autoIncrement' => false, 'enumValues' => null, - 'size' => null, - 'scale' => null, + 'size' => 5, + 'scale' => 0, 'defaultValue' => 1, ], 'char_col' => [ 'type' => 'char', - 'dbType' => 'char(100)', + 'dbType' => 'char', 'phpType' => 'string', 'primaryKey' => false, 'notNull' => true, @@ -76,7 +76,7 @@ public static function columns(): array ], 'char_col2' => [ 'type' => 'string', - 'dbType' => 'varchar(100)', + 'dbType' => 'varchar', 'phpType' => 'string', 'primaryKey' => false, 'notNull' => false, @@ -94,13 +94,13 @@ public static function columns(): array 'notNull' => false, 'autoIncrement' => false, 'enumValues' => null, - 'size' => null, + 'size' => 2147483647, 'scale' => null, 'defaultValue' => null, ], 'float_col' => [ 'type' => 'decimal', - 'dbType' => 'decimal(4,3)', + 'dbType' => 'decimal', 'phpType' => 'float', 'primaryKey' => false, 'notNull' => true, @@ -118,7 +118,7 @@ public static function columns(): array 'notNull' => false, 'autoIncrement' => false, 'enumValues' => null, - 'size' => null, + 'size' => 53, 'scale' => null, 'defaultValue' => 1.23, ], @@ -136,7 +136,7 @@ public static function columns(): array ], 'numeric_col' => [ 'type' => 'decimal', - 'dbType' => 'decimal(5,2)', + 'dbType' => 'decimal', 'phpType' => 'float', 'primaryKey' => false, 'notNull' => false, @@ -154,7 +154,7 @@ public static function columns(): array 'notNull' => true, 'autoIncrement' => false, 'enumValues' => null, - 'size' => null, + 'size' => 3, 'scale' => null, 'defaultValue' => '2002-01-01 00:00:00', ], @@ -195,13 +195,13 @@ public static function columns(): array 'notNull' => true, 'autoIncrement' => true, 'enumValues' => null, - 'size' => null, - 'scale' => null, + 'size' => 10, + 'scale' => 0, 'defaultValue' => null, ], 'type' => [ 'type' => 'string', - 'dbType' => 'varchar(255)', + 'dbType' => 'varchar', 'phpType' => 'string', 'primaryKey' => false, 'notNull' => true, diff --git a/tests/Provider/Type/BinaryProvider.php b/tests/Provider/Type/BinaryProvider.php index 2e14827b0..a0968ddd1 100644 --- a/tests/Provider/Type/BinaryProvider.php +++ b/tests/Provider/Type/BinaryProvider.php @@ -9,8 +9,8 @@ final class BinaryProvider public static function columns(): array { return [ - ['Mybinary1', 'binary(10)', 'mixed', 10, 'CONVERT([binary](10),\'binary\')'], - ['Mybinary2', 'binary(1)', 'mixed', 1, 'CONVERT([binary](1),\'b\')'], + ['Mybinary1', 'binary', 'mixed', 10, 'CONVERT([binary](10),\'binary\')'], + ['Mybinary2', 'binary', 'mixed', 1, 'CONVERT([binary](1),\'b\')'], ]; } } diff --git a/tests/Provider/Type/CharProvider.php b/tests/Provider/Type/CharProvider.php index b0be14aa7..ebd1aed28 100644 --- a/tests/Provider/Type/CharProvider.php +++ b/tests/Provider/Type/CharProvider.php @@ -9,8 +9,8 @@ final class CharProvider public static function columns(): array { return [ - ['Mychar1', 'char(10)', 'string', 10, 'char'], - ['Mychar2', 'char(1)', 'string', 1, 'c'], + ['Mychar1', 'char', 'string', 10, 'char'], + ['Mychar2', 'char', 'string', 1, 'c'], ]; } } diff --git a/tests/Provider/Type/VarBinaryProvider.php b/tests/Provider/Type/VarBinaryProvider.php index d1ea57620..1b1509262 100644 --- a/tests/Provider/Type/VarBinaryProvider.php +++ b/tests/Provider/Type/VarBinaryProvider.php @@ -9,9 +9,9 @@ final class VarBinaryProvider public static function columns(): array { return [ - ['Myvarbinary1', 'varbinary(10)', 'mixed', 10, 'CONVERT([varbinary](10),\'varbinary\')'], - ['Myvarbinary2', 'varbinary(100)', 'mixed', 100, 'CONVERT([varbinary](100),\'v\')'], - ['Myvarbinary3', 'varbinary(20)', 'mixed', 20, 'hashbytes(\'MD5\',\'test string\')'], + ['Myvarbinary1', 'varbinary', 'mixed', 10, 'CONVERT([varbinary](10),\'varbinary\')'], + ['Myvarbinary2', 'varbinary', 'mixed', 100, 'CONVERT([varbinary](100),\'v\')'], + ['Myvarbinary3', 'varbinary', 'mixed', 20, 'hashbytes(\'MD5\',\'test string\')'], ]; } } diff --git a/tests/Provider/Type/VarCharProvider.php b/tests/Provider/Type/VarCharProvider.php index f57d0e11e..692380639 100644 --- a/tests/Provider/Type/VarCharProvider.php +++ b/tests/Provider/Type/VarCharProvider.php @@ -9,9 +9,9 @@ final class VarCharProvider public static function columns(): array { return [ - ['Myvarchar1', 'varchar(10)', 'string', 10, 'varchar'], - ['Myvarchar2', 'varchar(100)', 'string', 100, 'v'], - ['Myvarchar3', 'varchar(20)', 'string', 20, 'TRY_CAST(datepart(year,getdate()) AS [varchar](20))'], + ['Myvarchar1', 'varchar', 'string', 10, 'varchar'], + ['Myvarchar2', 'varchar', 'string', 100, 'v'], + ['Myvarchar3', 'varchar', 'string', 20, 'TRY_CAST(datepart(year,getdate()) AS [varchar](20))'], ]; } } diff --git a/tests/QueryBuilderTest.php b/tests/QueryBuilderTest.php index a2cc69e9f..f079658a7 100644 --- a/tests/QueryBuilderTest.php +++ b/tests/QueryBuilderTest.php @@ -847,7 +847,8 @@ public function testAlterColumnOnDb(): void $db->createCommand($sql)->execute(); $schema = $db->getTableSchema('[foo1]', true); - $this->assertSame('varchar(255)', $schema?->getColumn('bar')->getDbType()); + $this->assertSame('varchar', $schema?->getColumn('bar')->getDbType()); + $this->assertSame(255, $schema?->getColumn('bar')->getSize()); $this->assertFalse($schema?->getColumn('bar')->isNotNull()); $sql = $db->getQueryBuilder()->alterColumn( @@ -858,7 +859,8 @@ public function testAlterColumnOnDb(): void $db->createCommand($sql)->execute(); $schema = $db->getTableSchema('[foo1]', true); - $this->assertSame('nvarchar(128)', $schema?->getColumn('bar')->getDbType()); + $this->assertSame('nvarchar', $schema?->getColumn('bar')->getDbType()); + $this->assertSame(128, $schema?->getColumn('bar')->getSize()); $this->assertTrue($schema?->getColumn('bar')->isNotNull()); $sql = $db->getQueryBuilder()->alterColumn( @@ -888,7 +890,8 @@ public function testAlterColumnWithCheckConstraintOnDb(): void ); $db->createCommand($sql)->execute(); $schema = $db->getTableSchema('[foo1]', true); - $this->assertEquals('nvarchar(128)', $schema?->getColumn('bar')->getDbType()); + $this->assertSame('nvarchar', $schema?->getColumn('bar')->getDbType()); + $this->assertSame(128, $schema?->getColumn('bar')->getSize()); $this->assertFalse($schema?->getColumn('bar')->isNotNull()); $sql = "INSERT INTO [foo1]([bar]) values('abcdef')"; diff --git a/tests/SchemaTest.php b/tests/SchemaTest.php index b6945f98a..e9046c9ab 100644 --- a/tests/SchemaTest.php +++ b/tests/SchemaTest.php @@ -73,18 +73,6 @@ public function testGetSchemaNames(): void $this->assertSame(['dbo', 'guest'], $schemas); } - /** - * @dataProvider \Yiisoft\Db\Mssql\Tests\Provider\SchemaProvider::columnsTypeChar - */ - public function testGetStringFieldsSize( - string $columnName, - string $columnType, - int|null $columnSize, - string $columnDbType - ): void { - parent::testGetStringFieldsSize($columnName, $columnType, $columnSize, $columnDbType); - } - public function testGetViewNames(): void { $db = $this->getConnection(true); diff --git a/tests/Type/DecimalTest.php b/tests/Type/DecimalTest.php index d5ade4350..2f6a8815f 100644 --- a/tests/Type/DecimalTest.php +++ b/tests/Type/DecimalTest.php @@ -38,7 +38,7 @@ public function testCreateTableWithDefaultValue(): void $tableSchema = $db->getTableSchema('decimal_default'); - $this->assertSame('decimal(38,0)', $tableSchema?->getColumn('Mydecimal')->getDbType()); + $this->assertSame('decimal', $tableSchema?->getColumn('Mydecimal')->getDbType()); $this->assertSame('float', $tableSchema?->getColumn('Mydecimal')->getPhpType()); $this->assertSame(38, $tableSchema?->getColumn('Mydecimal')->getSize()); $this->assertSame(9.9999999999999998e+037, $tableSchema?->getColumn('Mydecimal')->getDefaultValue()); @@ -86,7 +86,7 @@ public function testDefaultValue(): void $db = $this->getConnection(true); $tableSchema = $db->getTableSchema('decimal_default'); - $this->assertSame('decimal(38,0)', $tableSchema?->getColumn('Mydecimal')->getDbType()); + $this->assertSame('decimal', $tableSchema?->getColumn('Mydecimal')->getDbType()); $this->assertSame('float', $tableSchema?->getColumn('Mydecimal')->getPhpType()); $this->assertSame(38, $tableSchema?->getColumn('Mydecimal')->getSize()); $this->assertSame(9.9999999999999998e+037, $tableSchema?->getColumn('Mydecimal')->getDefaultValue()); diff --git a/tests/Type/NumericTest.php b/tests/Type/NumericTest.php index 073a1f888..068ea5ccf 100644 --- a/tests/Type/NumericTest.php +++ b/tests/Type/NumericTest.php @@ -38,7 +38,7 @@ public function testCreateTableWithDefaultValue(): void $tableSchema = $db->getTableSchema('numeric_default'); - $this->assertSame('numeric(38,0)', $tableSchema?->getColumn('Mynumeric')->getDbType()); + $this->assertSame('numeric', $tableSchema?->getColumn('Mynumeric')->getDbType()); $this->assertSame('float', $tableSchema?->getColumn('Mynumeric')->getPhpType()); $this->assertSame(38, $tableSchema?->getColumn('Mynumeric')->getSize()); $this->assertSame(9.9999999999999998e+037, $tableSchema?->getColumn('Mynumeric')->getDefaultValue()); @@ -86,7 +86,7 @@ public function testDefaultValue(): void $db = $this->getConnection(true); $tableSchema = $db->getTableSchema('numeric_default'); - $this->assertSame('numeric(38,0)', $tableSchema?->getColumn('Mynumeric')->getDbType()); + $this->assertSame('numeric', $tableSchema?->getColumn('Mynumeric')->getDbType()); $this->assertSame('float', $tableSchema?->getColumn('Mynumeric')->getPhpType()); $this->assertSame(38, $tableSchema?->getColumn('Mynumeric')->getSize()); $this->assertSame(9.9999999999999998e+037, $tableSchema?->getColumn('Mynumeric')->getDefaultValue()); From 5372e43f322fec058564240067b5766b4e7421a3 Mon Sep 17 00:00:00 2001 From: StyleCI Bot Date: Sat, 2 Nov 2024 09:28:19 +0000 Subject: [PATCH 2/3] Apply fixes from StyleCI --- src/Schema.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Schema.php b/src/Schema.php index e74d5551f..6fb9ab847 100644 --- a/src/Schema.php +++ b/src/Schema.php @@ -438,7 +438,7 @@ protected function findColumns(TableSchemaInterface $table): bool } if ($schemaName !== null) { - $whereSql .= " AND [t1].[table_schema] = :schema_name"; + $whereSql .= ' AND [t1].[table_schema] = :schema_name'; $whereParams[':schema_name'] = $schemaName; } From 376313e326b0292301f247542a4ea744d626dc01 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Fri, 8 Nov 2024 08:39:02 +0700 Subject: [PATCH 3/3] Update CHANGELOG.md [skip ci] --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a62bc3933..b17e0bf6f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,13 +10,14 @@ - Enh #312: Refactor `bit` type (@Tigrov) - Enh #315: Refactor PHP type of `ColumnSchemaInterface` instances (@Tigrov) - Enh #317: Raise minimum PHP version to `^8.1` with minor refactoring (@Tigrov) -- New #316: Implement `ColumnFactory` class (@Tigrov) +- New #316, #327: Implement `ColumnFactory` class (@Tigrov) - Enh #319: Separate column type constants (@Tigrov) - New #320: Realize `ColumnBuilder` class (@Tigrov) - Enh #321: Update according changes in `ColumnSchemaInterface` (@Tigrov) - New #322: Add `ColumnDefinitionBuilder` class (@Tigrov) - Enh #323: Refactor `Dsn` class (@Tigrov) - Enh #324: Use constructor to create columns and initialize properties (@Tigrov) +- Enh #327: Refactor `Schema::findColumns()` method (@Tigrov) ## 1.2.0 March 21, 2024