From f312562a43a75e530a3e93e883732f018bc85a5d Mon Sep 17 00:00:00 2001 From: August Trapper Bigelow Date: Wed, 28 Apr 2021 16:18:22 -0600 Subject: [PATCH 1/5] Force SQL Server adapter to always drop/readd the default constraint ALTER TABLE COLUMN calls error out unless the constraint is properly removed prior. This removes the logic of determining if the default changes, as the database doesn't care if it does or not. --- src/Phinx/Db/Adapter/SqlServerAdapter.php | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/Phinx/Db/Adapter/SqlServerAdapter.php b/src/Phinx/Db/Adapter/SqlServerAdapter.php index 760568fff..2ce391b21 100644 --- a/src/Phinx/Db/Adapter/SqlServerAdapter.php +++ b/src/Phinx/Db/Adapter/SqlServerAdapter.php @@ -609,9 +609,6 @@ protected function getChangeDefault($tableName, Column $newColumn) protected function getChangeColumnInstructions($tableName, $columnName, Column $newColumn) { $columns = $this->getColumns($tableName); - $changeDefault = - $newColumn->getDefault() !== $columns[$columnName]->getDefault() || - $newColumn->getType() !== $columns[$columnName]->getType(); $instructions = new AlterInstructions(); @@ -621,9 +618,7 @@ protected function getChangeColumnInstructions($tableName, $columnName, Column $ ); } - if ($changeDefault) { - $instructions->merge($this->getDropDefaultConstraint($tableName, $newColumn->getName())); - } + $instructions->merge($this->getDropDefaultConstraint($tableName, $newColumn->getName())); $instructions->addPostStep(sprintf( 'ALTER TABLE %s ALTER COLUMN %s %s', @@ -636,9 +631,7 @@ protected function getChangeColumnInstructions($tableName, $columnName, Column $ $instructions->addPostStep($this->getColumnCommentSqlDefinition($newColumn, $tableName)); } - if ($changeDefault) { - $instructions->merge($this->getChangeDefault($tableName, $newColumn)); - } + $instructions->merge($this->getChangeDefault($tableName, $newColumn)); return $instructions; } From b0bb62401a01ea0384cb905de5d804c909d00cfc Mon Sep 17 00:00:00 2001 From: August Trapper Bigelow Date: Wed, 28 Apr 2021 16:52:01 -0600 Subject: [PATCH 2/5] Remove use of NTEXT SQL type and use NVARCHAR(MAX) instead TEXT types have been deprecated for years and VARCHAR(MAX) has been the recommended type to use. This changes the translation to use nvarchar(max) instead. Also ensures that there isn't a limit set when retrieving text columns, to keep this abstracted away. --- src/Phinx/Db/Adapter/SqlServerAdapter.php | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/Phinx/Db/Adapter/SqlServerAdapter.php b/src/Phinx/Db/Adapter/SqlServerAdapter.php index 2ce391b21..972a51dcd 100644 --- a/src/Phinx/Db/Adapter/SqlServerAdapter.php +++ b/src/Phinx/Db/Adapter/SqlServerAdapter.php @@ -458,7 +458,7 @@ public function getColumns($tableName) $rows = $this->fetchAll($sql); foreach ($rows as $columnInfo) { try { - $type = $this->getPhinxType($columnInfo['type']); + $type = $this->getPhinxType($columnInfo['type'], $columnInfo['char_length']); } catch (UnsupportedColumnTypeException $e) { $type = Literal::from($columnInfo['type']); } @@ -471,7 +471,8 @@ public function getColumns($tableName) ->setIdentity($columnInfo['identity'] === '1') ->setComment($this->getColumnComment($columnInfo['table_name'], $columnInfo['name'])); - if (!empty($columnInfo['char_length'])) { + if ($type != self::PHINX_TYPE_TEXT && !empty($columnInfo['char_length'])) { + // Do not set a limit for text, as that is handled elsewhere. $column->setLimit($columnInfo['char_length']); } @@ -1062,7 +1063,7 @@ public function getSqlType($type, $limit = null) case static::PHINX_TYPE_CHAR: return ['name' => 'nchar', 'limit' => 255]; case static::PHINX_TYPE_TEXT: - return ['name' => 'ntext']; + return ['name' => 'nvarchar', 'limit' => 'max']; case static::PHINX_TYPE_INTEGER: return ['name' => 'int']; case static::PHINX_TYPE_TINY_INTEGER: @@ -1102,17 +1103,23 @@ public function getSqlType($type, $limit = null) * @internal param string $sqlType SQL type * * @param string $sqlType SQL Type definition + * @param int|null $char_length Column length value * * @throws \Phinx\Db\Adapter\UnsupportedColumnTypeException * * @return string Phinx type */ - public function getPhinxType($sqlType) + public function getPhinxType($sqlType, $char_length = null) { switch ($sqlType) { case 'nvarchar': case 'varchar': - return static::PHINX_TYPE_STRING; + if ($char_length == -1) { + // max char length + return static::PHINX_TYPE_TEXT; + } else { + return static::PHINX_TYPE_STRING; + } case 'char': case 'nchar': return static::PHINX_TYPE_CHAR; From 9b9de7acd72b99b14fbdbba050fc0150d40a92c5 Mon Sep 17 00:00:00 2001 From: August Trapper Bigelow Date: Thu, 29 Apr 2021 14:21:33 -0600 Subject: [PATCH 3/5] Remove unneeded else No need for an else with an early return. --- src/Phinx/Db/Adapter/SqlServerAdapter.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Phinx/Db/Adapter/SqlServerAdapter.php b/src/Phinx/Db/Adapter/SqlServerAdapter.php index 972a51dcd..57d93475b 100644 --- a/src/Phinx/Db/Adapter/SqlServerAdapter.php +++ b/src/Phinx/Db/Adapter/SqlServerAdapter.php @@ -1117,9 +1117,9 @@ public function getPhinxType($sqlType, $char_length = null) if ($char_length == -1) { // max char length return static::PHINX_TYPE_TEXT; - } else { - return static::PHINX_TYPE_STRING; } + + return static::PHINX_TYPE_STRING; case 'char': case 'nchar': return static::PHINX_TYPE_CHAR; From 1d80a36ffa260a5dd93e09e4ac0e0dc69d07ff7d Mon Sep 17 00:00:00 2001 From: Trapper Bigelow Date: Fri, 3 Sep 2021 14:55:25 -0600 Subject: [PATCH 4/5] Update does not equals check to does not equal with type. --- src/Phinx/Db/Adapter/SqlServerAdapter.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Phinx/Db/Adapter/SqlServerAdapter.php b/src/Phinx/Db/Adapter/SqlServerAdapter.php index 57d93475b..8adaab0fe 100644 --- a/src/Phinx/Db/Adapter/SqlServerAdapter.php +++ b/src/Phinx/Db/Adapter/SqlServerAdapter.php @@ -471,7 +471,7 @@ public function getColumns($tableName) ->setIdentity($columnInfo['identity'] === '1') ->setComment($this->getColumnComment($columnInfo['table_name'], $columnInfo['name'])); - if ($type != self::PHINX_TYPE_TEXT && !empty($columnInfo['char_length'])) { + if ($type !== self::PHINX_TYPE_TEXT && !empty($columnInfo['char_length'])) { // Do not set a limit for text, as that is handled elsewhere. $column->setLimit($columnInfo['char_length']); } From f8e3a65f9cb000d4e6b14d2bc444574733b37c77 Mon Sep 17 00:00:00 2001 From: Trapper Bigelow Date: Fri, 3 Sep 2021 14:56:17 -0600 Subject: [PATCH 5/5] camelCase char_length argument. --- src/Phinx/Db/Adapter/SqlServerAdapter.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Phinx/Db/Adapter/SqlServerAdapter.php b/src/Phinx/Db/Adapter/SqlServerAdapter.php index 8adaab0fe..42b10d9d3 100644 --- a/src/Phinx/Db/Adapter/SqlServerAdapter.php +++ b/src/Phinx/Db/Adapter/SqlServerAdapter.php @@ -1103,18 +1103,18 @@ public function getSqlType($type, $limit = null) * @internal param string $sqlType SQL type * * @param string $sqlType SQL Type definition - * @param int|null $char_length Column length value + * @param int|null $charLength Column length value * * @throws \Phinx\Db\Adapter\UnsupportedColumnTypeException * * @return string Phinx type */ - public function getPhinxType($sqlType, $char_length = null) + public function getPhinxType($sqlType, $charLength = null) { switch ($sqlType) { case 'nvarchar': case 'varchar': - if ($char_length == -1) { + if ($charLength == -1) { // max char length return static::PHINX_TYPE_TEXT; }