Fixes yiisoft#13128: Fixed incorrect position of {pos} string in Colu…

…mnSchemaBuilder `__toString`
Deele · Dec 11, 2016 · ae83dd0 · ae83dd0
1 parent 75b98c6
commit ae83dd0
Show file tree

Hide file tree

Showing 6 changed files with 70 additions and 29 deletions.
diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md
@@ -30,6 +30,7 @@ Yii Framework 2 Change Log
 - Bug #13159: Fixed `destroy` method in `yii.captcha.js` which did not work as expected (arogachev)
 - Bug #7727: Fixed truncateHtml leaving extra tags (developeruz)
 - Bug #13118: Fixed `handleAction()` function in `yii.js` to handle attribute `data-pjax=0` as disabled PJAX (silverfire)
+- Bug #13128: Fixed incorrect position of {pos} string in ColumnSchemaBuilder `__toString` (df2)
 - Bug #13105: Fixed `validate()` method in `yii.activeForm.js` to prevent unexpected form submit when `forceValidate` set to `true` (silverfire)
 - Enh #475: Added Bash and Zsh completion support for the `./yii` command (cebe, silverfire)
 - Enh #6242: Access to validator in inline validation (arogachev)

diff --git a/framework/db/ColumnSchemaBuilder.php b/framework/db/ColumnSchemaBuilder.php
@@ -258,7 +258,8 @@ public function defaultExpression($default)
     }
 
     /**
-     * Specify additional SQL to be appended to schema string.
+     * Specify additional SQL to be appended to column definition.
+     * Position modifiers will be appended after column definition in databases that support them.
      * @param string $sql the SQL string to be appended.
      * @return $this
      * @since 2.0.9

diff --git a/framework/db/cubrid/ColumnSchemaBuilder.php b/framework/db/cubrid/ColumnSchemaBuilder.php
@@ -58,13 +58,13 @@ public function __toString()
     {
         switch ($this->getTypeCategory()) {
             case self::CATEGORY_PK:
-                $format = '{type}{check}{pos}{comment}{append}';
+                $format = '{type}{check}{comment}{append}{pos}';
                 break;
             case self::CATEGORY_NUMERIC:
-                $format = '{type}{length}{unsigned}{notnull}{unique}{default}{check}{comment}{pos}{append}';
+                $format = '{type}{length}{unsigned}{notnull}{unique}{default}{check}{comment}{append}{pos}';
                 break;
             default:
-                $format = '{type}{length}{notnull}{unique}{default}{check}{comment}{pos}{append}';
+                $format = '{type}{length}{notnull}{unique}{default}{check}{comment}{append}{pos}';
         }
         return $this->buildCompleteString($format);
     }

diff --git a/framework/db/mysql/ColumnSchemaBuilder.php b/framework/db/mysql/ColumnSchemaBuilder.php
@@ -58,13 +58,13 @@ public function __toString()
     {
         switch ($this->getTypeCategory()) {
             case self::CATEGORY_PK:
-                $format = '{type}{length}{check}{comment}{pos}{append}';
+                $format = '{type}{length}{check}{comment}{append}{pos}';
                 break;
             case self::CATEGORY_NUMERIC:
-                $format = '{type}{length}{unsigned}{notnull}{unique}{default}{check}{comment}{pos}{append}';
+                $format = '{type}{length}{unsigned}{notnull}{unique}{default}{check}{comment}{append}{pos}';
                 break;
             default:
-                $format = '{type}{length}{notnull}{unique}{default}{check}{comment}{pos}{append}';
+                $format = '{type}{length}{notnull}{unique}{default}{check}{comment}{append}{pos}';
         }
         return $this->buildCompleteString($format);
     }

diff --git a/framework/db/oci/ColumnSchemaBuilder.php b/framework/db/oci/ColumnSchemaBuilder.php
@@ -26,38 +26,20 @@ protected function buildUnsignedString()
         return $this->isUnsigned ? ' UNSIGNED' : '';
     }
 
-    /**
-     * @inheritdoc
-     */
-    protected function buildAfterString()
-    {
-        return $this->after !== null ?
-            ' AFTER ' . $this->db->quoteColumnName($this->after) :
-            '';
-    }
-
-    /**
-     * @inheritdoc
-     */
-    protected function buildFirstString()
-    {
-        return $this->isFirst ? ' FIRST' : '';
-    }
-
     /**
      * @inheritdoc
      */
     public function __toString()
     {
         switch ($this->getTypeCategory()) {
             case self::CATEGORY_PK:
-                $format = '{type}{length}{check}{pos}{append}';
+                $format = '{type}{length}{check}{append}';
                 break;
             case self::CATEGORY_NUMERIC:
-                $format = '{type}{length}{unsigned}{default}{notnull}{check}{pos}{append}';
+                $format = '{type}{length}{unsigned}{default}{notnull}{check}{append}';
                 break;
             default:
-                $format = '{type}{length}{default}{notnull}{check}{pos}{append}';
+                $format = '{type}{length}{default}{notnull}{check}{append}';
         }
         return $this->buildCompleteString($format);
     }

diff --git a/tests/framework/db/QueryBuilderTest.php b/tests/framework/db/QueryBuilderTest.php
@@ -923,6 +923,61 @@ public function columnTypes()
                     'cubrid' => "int NOT NULL AUTO_INCREMENT PRIMARY KEY COMMENT 'test comment'",
                 ],
             ],
+            [
+                Schema::TYPE_PK . " FIRST",
+                $this->primaryKey()->first(),
+                [
+                    'mysql' => "int(11) NOT NULL AUTO_INCREMENT PRIMARY KEY FIRST",
+                    'postgres' => 'serial NOT NULL PRIMARY KEY',
+                    'oci' => 'NUMBER(10) NOT NULL PRIMARY KEY',
+                    'sqlsrv' => 'int IDENTITY PRIMARY KEY',
+                    'cubrid' => "int NOT NULL AUTO_INCREMENT PRIMARY KEY FIRST",
+                ],
+            ],
+            [
+                Schema::TYPE_INTEGER . " FIRST",
+                $this->integer()->first(),
+                [
+                    'mysql' => "int(11) FIRST",
+                    'postgres' => 'integer',
+                    'oci' => "NUMBER(10)",
+                    'sqlsrv' => 'int',
+                    'cubrid' => "int FIRST",
+                ],
+            ],
+            [
+                Schema::TYPE_STRING . ' FIRST',
+                $this->string()->first(),
+                [
+                    'mysql' => 'varchar(255) FIRST',
+                    'postgres' => 'varchar(255)',
+                    'oci' => 'VARCHAR2(255)',
+                    'sqlsrv' => 'varchar(255)',
+                    'cubrid' => 'varchar(255) FIRST',
+                ],
+            ],
+            [
+                Schema::TYPE_INTEGER . " NOT NULL FIRST",
+                $this->integer()->append('NOT NULL')->first(),
+                [
+                    'mysql' => "int(11) NOT NULL FIRST",
+                    'postgres' => 'integer NOT NULL',
+                    'oci' => "NUMBER(10) NOT NULL",
+                    'sqlsrv' => 'int NOT NULL',
+                    'cubrid' => "int NOT NULL FIRST",
+                ],
+            ],
+            [
+                Schema::TYPE_STRING . ' NOT NULL FIRST',
+                $this->string()->append('NOT NULL')->first(),
+                [
+                    'mysql' => 'varchar(255) NOT NULL FIRST',
+                    'postgres' => 'varchar(255) NOT NULL',
+                    'oci' => 'VARCHAR2(255) NOT NULL',
+                    'sqlsrv' => 'varchar(255) NOT NULL',
+                    'cubrid' => 'varchar(255) NOT NULL FIRST',
+                ],
+            ],
         ];
 
         foreach ($items as $i => $item) {
@@ -965,7 +1020,9 @@ public function testCreateTableColumnTypes()
             if (!(strncmp($column, Schema::TYPE_PK, 2) === 0 ||
                   strncmp($column, Schema::TYPE_UPK, 3) === 0 ||
                   strncmp($column, Schema::TYPE_BIGPK, 5) === 0 ||
-                  strncmp($column, Schema::TYPE_UBIGPK, 6) === 0)) {
+                  strncmp($column, Schema::TYPE_UBIGPK, 6) === 0 ||
+                  strncmp(substr($column, -5), 'FIRST', 5) === 0
+                )) {
                 $columns['col' . ++$i] = str_replace('CHECK (value', 'CHECK ([[col' . $i . ']]', $column);
             }
         }