Allow ColumnInterface as column type (#766)

* Allow `ColumnInterface` as column type * Update tests * Fix psalm issue * Rename variable in test `$type` => `$columnType` * Move psalm type declaration to method docs * Add line to CHANGELOG.md [skip ci] * Restart tests * Apply rector suggestions * Update UPGRADE.md by @vjik --------- Co-authored-by: Sergei Predvoditelev <[email protected]>
yiisoft · Apr 7, 2024 · 2eb63c0 · 2eb63c0
1 parent 749c3f3
commit 2eb63c0
Show file tree

Hide file tree

Showing 15 changed files with 78 additions and 37 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,8 +1,8 @@
 # Yii Database Change Log
 
-## 1.3.1 under development
+## 2.0.0 under development
 
-- no changes in this release.
+- Enh #766: Allow `ColumnInterface` as column type. (@Tigrov)
 
 ## 1.3.0 March 21, 2024
 

diff --git a/UPGRADE.md b/UPGRADE.md
@@ -1,14 +1,18 @@
 # Upgrading Instructions for Yii Database
 
-This file contains the upgrade notes for the Yii Database.
-These notes highlight changes that could break your application when you upgrade it from one version to another.
-Even though we try to ensure backwards compatibility (BC) as much as possible, sometimes
-it isn't possible or very complicated to avoid it and still create a good solution to
-a problem. While upgrade to Yii 3.0 might require substantial changes to both your application and extensions,
-the changes are bearable and require "refactoring", not "rewrite".
-All the "Yes, it is" cool stuff, and Yii soul is still in place.
-
-Changes summary:
-
-* `Yiisoft\Db\Connection::$charset` has been removed. All supported PDO classes allow you to specify the connection
-  charset in the DSN.
+The following upgrading instructions are cumulative. That is, if you want to upgrade from version A to version C and 
+there is version B between A and C, you need to following the instructions for both A and B.
+
+## Upgrade from 1.x to 2.x
+
+Add `ColumnInterface` support and change type of parameter `$type` from `string` to `ColumnInterface|string` 
+in `addColumn()` method of your classes that implement the following interfaces:
+
+- `Yiisoft\Db\Command\CommandInterface`;
+- `Yiisoft\Db\QueryBuilder\DDLQueryBuilderInterface`;
+
+… or inherit from the following classes:
+
+- `Yiisoft\Db\Command\AbstractCommand`;
+- `Yiisoft\Db\QueryBuilder\AbstractDDLQueryBuilder`;
+- `Yiisoft\Db\QueryBuilder\AbstractQueryBuilder`.
diff --git a/src/Command/AbstractCommand.php b/src/Command/AbstractCommand.php
@@ -130,7 +130,7 @@ public function addCheck(string $table, string $name, string $expression): stati
         return $this->setSql($sql)->requireTableSchemaRefresh($table);
     }
 
-    public function addColumn(string $table, string $column, string $type): static
+    public function addColumn(string $table, string $column, ColumnInterface|string $type): static
     {
         $sql = $this->getQueryBuilder()->addColumn($table, $column, $type);
         return $this->setSql($sql)->requireTableSchemaRefresh($table);

diff --git a/src/Command/CommandInterface.php b/src/Command/CommandInterface.php
@@ -42,13 +42,13 @@ public function addCheck(string $table, string $name, string $expression): stati
      *
      * @param string $table The name of the table to add new column to.
      * @param string $column The name of the new column.
-     * @param string $type The column type. {@see QueryBuilder::getColumnType()} will be called to convert the given
-     * column type to the database one.
+     * @param ColumnInterface|string $type The column type. {@see QueryBuilder::getColumnType()} will be called
+     * to convert the given column type to the database one.
      * For example, `string` will be converted to `varchar(255)`, and `string not null` becomes `varchar(255) not null`.
      *
      * Note: The method will quote the `table` and `column` parameters before using them in the generated SQL.
      */
-    public function addColumn(string $table, string $column, string $type): static;
+    public function addColumn(string $table, string $column, ColumnInterface|string $type): static;
 
     /**
      * Builds an SQL command for adding a comment to a column.
@@ -309,13 +309,16 @@ public function createIndex(
      *
      * @param string $table The name of the table to create.
      * @param array $columns The columns (name => definition) in the new table.
+     * The definition can be `string` or {@see ColumnInterface} instance.
      * @param string|null $options More SQL fragments to append to the generated SQL.
      *
      * @throws Exception
      * @throws InvalidConfigException
      * @throws NotSupportedException
      *
      * Note: The method will quote the `table` and `columns` parameter before using it in the generated SQL.
+     *
+     * @psalm-param array<string, ColumnInterface>|string[] $columns
      */
     public function createTable(string $table, array $columns, string $options = null): static;
 

diff --git a/src/Debug/CommandInterfaceProxy.php b/src/Debug/CommandInterfaceProxy.php
@@ -30,7 +30,7 @@ public function addCheck(string $table, string $name, string $expression): stati
     /**
      * @psalm-suppress  MixedArgument
      */
-    public function addColumn(string $table, string $column, string $type): static
+    public function addColumn(string $table, string $column, ColumnInterface|string $type): static
     {
         return new self($this->decorated->{__FUNCTION__}(...func_get_args()), $this->collector);
     }

diff --git a/src/Query/Query.php b/src/Query/Query.php
@@ -680,8 +680,6 @@ public function withQueries(array $withQueries): static
      *
      * Restores the value of select to make this query reusable.
      *
-     * @param ExpressionInterface|string $selectExpression
-     *
      * @throws Exception
      * @throws InvalidArgumentException
      * @throws InvalidConfigException

diff --git a/src/QueryBuilder/AbstractDDLQueryBuilder.php b/src/QueryBuilder/AbstractDDLQueryBuilder.php
@@ -39,7 +39,7 @@ public function addCheck(string $table, string $name, string $expression): strin
             . ' CHECK (' . $this->quoter->quoteSql($expression) . ')';
     }
 
-    public function addColumn(string $table, string $column, string $type): string
+    public function addColumn(string $table, string $column, ColumnInterface|string $type): string
     {
         return 'ALTER TABLE '
             . $this->quoter->quoteTableName($table)
@@ -169,14 +169,14 @@ public function createTable(string $table, array $columns, string $options = nul
     {
         $cols = [];
 
-        /** @psalm-var string[] $columns */
         foreach ($columns as $name => $type) {
             if (is_string($name)) {
                 $cols[] = "\t"
                     . $this->quoter->quoteColumnName($name)
                     . ' '
                     . $this->queryBuilder->getColumnType($type);
             } else {
+                /** @psalm-var string $type */
                 $cols[] = "\t" . $type;
             }
         }

diff --git a/src/QueryBuilder/AbstractQueryBuilder.php b/src/QueryBuilder/AbstractQueryBuilder.php
@@ -53,7 +53,7 @@ public function addCheck(string $table, string $name, string $expression): strin
         return $this->ddlBuilder->addCheck($table, $name, $expression);
     }
 
-    public function addColumn(string $table, string $column, string $type): string
+    public function addColumn(string $table, string $column, ColumnInterface|string $type): string
     {
         return $this->ddlBuilder->addColumn($table, $column, $type);
     }

diff --git a/src/QueryBuilder/DDLQueryBuilderInterface.php b/src/QueryBuilder/DDLQueryBuilderInterface.php
@@ -36,7 +36,7 @@ public function addCheck(string $table, string $name, string $expression): strin
      *
      * @param string $table The table to add the new column will to.
      * @param string $column The name of the new column.
-     * @param string $type The column type.
+     * @param ColumnInterface|string $type The column type.
      * {@see getColumnType()} Method will be invoked to convert an abstract column type (if any) into the physical one.
      * Anything that isn't recognized as an abstract type will be kept in the generated SQL.
      * For example, 'string' will be turned into 'varchar(255)', while 'string not null' will become
@@ -46,7 +46,7 @@ public function addCheck(string $table, string $name, string $expression): strin
      *
      * Note: The method will quote the `table` and `column` parameters before using them in the generated SQL.
      */
-    public function addColumn(string $table, string $column, string $type): string;
+    public function addColumn(string $table, string $column, ColumnInterface|string $type): string;
 
     /**
      * Builds an SQL command for adding comment to column.
@@ -233,11 +233,14 @@ public function createIndex(
      *
      * @param string $table The name of the table to create.
      * @param array $columns The columns (name => definition) in the new table.
+     * The definition can be `string` or {@see ColumnInterface} instance.
      * @param string|null $options More SQL fragments to append to the generated SQL.
      *
      * @return string The SQL statement for creating a new DB table.
      *
      * Note: The method will quote the `table` and `columns` parameter before using it in the generated SQL.
+     *
+     * @psalm-param array<string, ColumnInterface>|string[] $columns
      */
     public function createTable(string $table, array $columns, string $options = null): string;
 

diff --git a/tests/AbstractQueryBuilderTest.php b/tests/AbstractQueryBuilderTest.php
@@ -21,6 +21,7 @@
 use Yiisoft\Db\Query\Query;
 use Yiisoft\Db\Query\QueryInterface;
 use Yiisoft\Db\QueryBuilder\Condition\SimpleCondition;
+use Yiisoft\Db\Schema\Builder\ColumnInterface;
 use Yiisoft\Db\Schema\QuoterInterface;
 use Yiisoft\Db\Schema\SchemaInterface;
 use Yiisoft\Db\Tests\Support\Assert;
@@ -52,18 +53,19 @@ public function testAddCheck(): void
         );
     }
 
-    public function testAddColumn(): void
+    /** @dataProvider \Yiisoft\Db\Tests\Provider\QueryBuilderProvider::columnTypes */
+    public function testAddColumn(ColumnInterface|string $type): void
     {
         $db = $this->getConnection();
 
         $qb = $db->getQueryBuilder();
-        $sql = $qb->addColumn('table', 'column', SchemaInterface::TYPE_STRING);
+        $sql = $qb->addColumn('table', 'column', $type);
 
         $this->assertSame(
             DbHelper::replaceQuotes(
                 <<<SQL
                 ALTER TABLE [[table]] ADD [[column]]
-                SQL . ' ' . $qb->getColumnType(SchemaInterface::TYPE_STRING),
+                SQL . ' ' . $qb->getColumnType($type),
                 $db->getDriverName(),
             ),
             $sql,

diff --git a/tests/Common/CommonCommandTest.php b/tests/Common/CommonCommandTest.php
@@ -24,6 +24,7 @@
 use Yiisoft\Db\Schema\SchemaInterface;
 use Yiisoft\Db\Tests\AbstractCommandTest;
 use Yiisoft\Db\Tests\Support\Assert;
+use Yiisoft\Db\Tests\Support\Stub\Column;
 use Yiisoft\Db\Transaction\TransactionInterface;
 
 use function call_user_func_array;
@@ -544,16 +545,23 @@ public function testCreateTable(): void
 
         $command->createTable(
             '{{testCreateTable}}',
-            ['[[id]]' => SchemaInterface::TYPE_PK, '[[bar]]' => SchemaInterface::TYPE_INTEGER],
+            [
+                '[[id]]' => SchemaInterface::TYPE_PK,
+                '[[bar]]' => SchemaInterface::TYPE_INTEGER,
+                '[[name]]' => (new Column('string(100)'))->notNull(),
+            ],
         )->execute();
-        $command->insert('{{testCreateTable}}', ['[[bar]]' => 1])->execute();
+        $command->insert('{{testCreateTable}}', ['[[bar]]' => 1, '[[name]]' => 'Lilo'])->execute();
         $records = $command->setSql(
             <<<SQL
-            SELECT [[id]], [[bar]] FROM [[testCreateTable]];
+            SELECT [[id]], [[bar]], [[name]] FROM [[testCreateTable]];
             SQL
         )->queryAll();
 
-        $this->assertEquals([['id' => 1, 'bar' => 1]], $records);
+        $nameCol = $schema->getTableSchema('{{testCreateTable}}', true)->getColumn('name');
+
+        $this->assertFalse($nameCol->isAllowNull());
+        $this->assertEquals([['id' => 1, 'bar' => 1, 'name' => 'Lilo']], $records);
 
         $db->close();
     }

diff --git a/tests/Db/Command/CommandTest.php b/tests/Db/Command/CommandTest.php
@@ -5,6 +5,7 @@
 namespace Yiisoft\Db\Tests\Db\Command;
 
 use Yiisoft\Db\Exception\NotSupportedException;
+use Yiisoft\Db\Schema\Builder\ColumnInterface;
 use Yiisoft\Db\Schema\SchemaInterface;
 use Yiisoft\Db\Tests\AbstractCommandTest;
 use Yiisoft\Db\Tests\Support\Assert;
@@ -39,17 +40,20 @@ public function testAddCheck(): void
         );
     }
 
-    public function testAddColumn(): void
+    /** @dataProvider \Yiisoft\Db\Tests\Provider\CommandProvider::columnTypes */
+    public function testAddColumn(ColumnInterface|string $type): void
     {
         $db = $this->getConnection();
 
         $command = $db->createCommand();
-        $sql = $command->addColumn('table', 'column', SchemaInterface::TYPE_INTEGER)->getSql();
+        $sql = $command->addColumn('table', 'column', $type)->getSql();
+
+        $columnType = $db->getQueryBuilder()->getColumnType($type);
 
         $this->assertSame(
             DbHelper::replaceQuotes(
                 <<<SQL
-                ALTER TABLE [[table]] ADD [[column]] integer
+                ALTER TABLE [[table]] ADD [[column]] {$columnType}
                 SQL,
                 $db->getDriverName(),
             ),

diff --git a/tests/Db/QueryBuilder/QueryBuilderTest.php b/tests/Db/QueryBuilder/QueryBuilderTest.php
@@ -14,6 +14,7 @@
 use Yiisoft\Db\Query\QueryInterface;
 use Yiisoft\Db\Tests\AbstractQueryBuilderTest;
 use Yiisoft\Db\Tests\Support\DbHelper;
+use Yiisoft\Db\Tests\Support\Stub\Column;
 use Yiisoft\Db\Tests\Support\Stub\QueryBuilder;
 use Yiisoft\Db\Tests\Support\Stub\Schema;
 use Yiisoft\Db\Tests\Support\TestTrait;
@@ -122,7 +123,7 @@ public function testCreateTable(): void
                 [
                     'id' => 'pk',
                     'name' => 'string(255) NOT NULL',
-                    'email' => 'string(255) NOT NULL',
+                    'email' => (new Column('string(255)'))->notNull(),
                     'status' => 'integer NOT NULL',
                     'created_at' => 'datetime NOT NULL',
                     'UNIQUE test_email_unique (email)',

diff --git a/tests/Provider/CommandProvider.php b/tests/Provider/CommandProvider.php
@@ -12,6 +12,7 @@
 use Yiisoft\Db\Schema\SchemaInterface;
 use Yiisoft\Db\Tests\Support\DbHelper;
 use Yiisoft\Db\Tests\Support\Stringable;
+use Yiisoft\Db\Tests\Support\Stub\Column;
 use Yiisoft\Db\Tests\Support\TestTrait;
 
 class CommandProvider
@@ -921,4 +922,12 @@ public static function upsert(): array
             ],
         ];
     }
+
+    public static function columnTypes(): array
+    {
+        return [
+            [SchemaInterface::TYPE_INTEGER],
+            [new Column('string(100)')],
+        ];
+    }
 }
diff --git a/tests/Provider/QueryBuilderProvider.php b/tests/Provider/QueryBuilderProvider.php
@@ -12,6 +12,7 @@
 use Yiisoft\Db\QueryBuilder\QueryBuilderInterface;
 use Yiisoft\Db\Schema\SchemaInterface;
 use Yiisoft\Db\Tests\Support\DbHelper;
+use Yiisoft\Db\Tests\Support\Stub\Column;
 use Yiisoft\Db\Tests\Support\TestTrait;
 use Yiisoft\Db\Tests\Support\TraversableObject;
 
@@ -1277,4 +1278,12 @@ public static function cteAliases(): array
             'with extra space' => ['a(b,c,d) ', 'a(b,c,d) '],
         ];
     }
+
+    public static function columnTypes(): array
+    {
+        return [
+            [SchemaInterface::TYPE_STRING],
+            [new Column('string(100)')],
+        ];
+    }
 }