From f317bd0b2745cf45d85681d6bbcb44829da104d5 Mon Sep 17 00:00:00 2001 From: Jonas Pardon Date: Wed, 7 Oct 2026 16:01:47 +0200 Subject: [PATCH 01/11] Respect an explicit null modifier when changing a column The ->change() shims used isset() to decide whether the migration set a modifier, which is false for null, so ->default(null) kept the old default instead of removing it. Co-Authored-By: Claude Opus 5.5 --- src/Database/Schema/BlueprintState.php | 2 +- .../Grammars/Concerns/MySqlBasedGrammar.php | 2 +- src/Database/Schema/Grammars/PostgresGrammar.php | 2 +- .../Schema/Grammars/SqlServerGrammar.php | 2 +- .../Schema/Grammars/MySqlSchemaGrammarTest.php | 15 +++++++++++++++ .../Grammars/PostgresSchemaGrammarTest.php | 15 +++++++++++++++ .../Schema/Grammars/SQLiteSchemaGrammarTest.php | 12 ++++++++++++ .../Grammars/SqlServerSchemaGrammarTest.php | 16 ++++++++++++++++ 8 files changed, 62 insertions(+), 4 deletions(-) diff --git a/src/Database/Schema/BlueprintState.php b/src/Database/Schema/BlueprintState.php index 09b999c92..f480ead91 100644 --- a/src/Database/Schema/BlueprintState.php +++ b/src/Database/Schema/BlueprintState.php @@ -66,7 +66,7 @@ protected function preserveExistingColumnAttributes(Fluent $column): void } foreach ($this->preservedAttributes as $attribute) { - if (!isset($column[$attribute]) && isset($existing[$attribute])) { + if (!array_key_exists($attribute, $column->getAttributes()) && isset($existing[$attribute])) { $column[$attribute] = $existing[$attribute]; } } diff --git a/src/Database/Schema/Grammars/Concerns/MySqlBasedGrammar.php b/src/Database/Schema/Grammars/Concerns/MySqlBasedGrammar.php index 6f749de77..c0f2992cd 100644 --- a/src/Database/Schema/Grammars/Concerns/MySqlBasedGrammar.php +++ b/src/Database/Schema/Grammars/Concerns/MySqlBasedGrammar.php @@ -46,7 +46,7 @@ public function compileChange(Blueprint $blueprint, Fluent $command) foreach ($this->modifiers as $modifier) { if (method_exists($this, $method = "modify{$modifier}")) { $mod = strtolower($modifier); - $col = isset($oldColumn->{$mod}) && !isset($column->{$mod}) ? $oldColumn : $column; + $col = isset($oldColumn->{$mod}) && !array_key_exists($mod, $column->getAttributes()) ? $oldColumn : $column; $sql .= $this->{$method}($blueprint, $col); } } diff --git a/src/Database/Schema/Grammars/PostgresGrammar.php b/src/Database/Schema/Grammars/PostgresGrammar.php index faf156f09..b15e85bdb 100755 --- a/src/Database/Schema/Grammars/PostgresGrammar.php +++ b/src/Database/Schema/Grammars/PostgresGrammar.php @@ -45,7 +45,7 @@ public function compileChange(Blueprint $blueprint, Fluent $command) if (method_exists($this, $method = "modify{$modifier}")) { $mod = strtolower($modifier); - $col = isset($oldColumn->{$mod}) && ! isset($column->{$mod}) ? $oldColumn : $column; + $col = isset($oldColumn->{$mod}) && ! array_key_exists($mod, $column->getAttributes()) ? $oldColumn : $column; $constraints = (array) $this->{$method}($blueprint, $col); foreach ($constraints as $constraint) { diff --git a/src/Database/Schema/Grammars/SqlServerGrammar.php b/src/Database/Schema/Grammars/SqlServerGrammar.php index 46ea16049..526e6f397 100755 --- a/src/Database/Schema/Grammars/SqlServerGrammar.php +++ b/src/Database/Schema/Grammars/SqlServerGrammar.php @@ -46,7 +46,7 @@ public function compileChange(Blueprint $blueprint, Fluent $command) foreach ($this->modifiers as $modifier) { if (method_exists($this, $method = "modify{$modifier}")) { $mod = strtolower($modifier); - $col = isset($oldColumn->{$mod}) && !isset($column->{$mod}) ? $oldColumn : $column; + $col = isset($oldColumn->{$mod}) && !array_key_exists($mod, $column->getAttributes()) ? $oldColumn : $column; $sql .= $this->{$method}($blueprint, $col); } } diff --git a/tests/Database/Schema/Grammars/MySqlSchemaGrammarTest.php b/tests/Database/Schema/Grammars/MySqlSchemaGrammarTest.php index 7ee243c66..cd7322413 100644 --- a/tests/Database/Schema/Grammars/MySqlSchemaGrammarTest.php +++ b/tests/Database/Schema/Grammars/MySqlSchemaGrammarTest.php @@ -70,4 +70,19 @@ public function testQuoteInDefaultValue() $statements = $this->runBlueprint($initialBlueprint); $this->assertSame("alter table `users` add `name` varchar(255) not null default 'O''Brian'", $statements[0]); } + + public function testChangeCanRemoveDefaultWithNull() + { + $initialBlueprint = $this->getBlueprint('users'); + $initialBlueprint->string('name')->default('admin'); + + $statements = $this->runBlueprint($initialBlueprint); + $this->assertSame("alter table `users` add `name` varchar(255) not null default 'admin'", $statements[0]); + + $changedBlueprint = $this->getBlueprint('users'); + $changedBlueprint->string('name')->nullable()->default(null)->change(); + + $statements = $this->runBlueprint($changedBlueprint); + $this->assertSame('alter table `users` modify `name` varchar(255) null', $statements[0]); + } } diff --git a/tests/Database/Schema/Grammars/PostgresSchemaGrammarTest.php b/tests/Database/Schema/Grammars/PostgresSchemaGrammarTest.php index e42278119..4d86330db 100644 --- a/tests/Database/Schema/Grammars/PostgresSchemaGrammarTest.php +++ b/tests/Database/Schema/Grammars/PostgresSchemaGrammarTest.php @@ -79,4 +79,19 @@ public function testQuoteInDefaultValue() $parts = explode(', ', $statements[0]); $this->assertSame("alter table \"users\" add column \"name\" varchar(255) not null default 'O''Brian'", $parts[0]); } + + public function testChangeCanRemoveDefaultWithNull() + { + $initialBlueprint = $this->getBlueprint('users'); + $initialBlueprint->string('name')->default('admin'); + + $statements = $this->runBlueprint($initialBlueprint); + $this->assertSame("alter table \"users\" add column \"name\" varchar(255) not null default 'admin'", $statements[0]); + + $changedBlueprint = $this->getBlueprint('users'); + $changedBlueprint->string('name')->nullable()->default(null)->change(); + + $statements = $this->runBlueprint($changedBlueprint); + $this->assertSame('alter table "users" alter column "name" type varchar(255), alter column "name" drop not null, alter column "name" drop default, alter column "name" drop identity if exists', $statements[0]); + } } diff --git a/tests/Database/Schema/Grammars/SQLiteSchemaGrammarTest.php b/tests/Database/Schema/Grammars/SQLiteSchemaGrammarTest.php index 9fa7be9f8..87df2f6a1 100644 --- a/tests/Database/Schema/Grammars/SQLiteSchemaGrammarTest.php +++ b/tests/Database/Schema/Grammars/SQLiteSchemaGrammarTest.php @@ -152,4 +152,16 @@ public function testChangeRebuildsTableExactlyOnce(): void $this->assertSame(1, $rebuilds, 'A single ->change() must rebuild the table exactly once.'); } + + public function testChangeCanRemoveDefaultWithNull(): void + { + [$pdo, , $builder] = $this->bootSqlite("CREATE TABLE users (id integer primary key, name varchar not null default 'bob')"); + + // An explicit null default must remove the existing default, not keep it. + $builder->table('users', fn (Blueprint $table) => $table->string('name')->nullable()->default(null)->change()); + + $info = $this->columnInfo($pdo, 'users', 'name'); + $this->assertSame(0, (int) $info['notnull']); + $this->assertNull($info['dflt_value']); + } } diff --git a/tests/Database/Schema/Grammars/SqlServerSchemaGrammarTest.php b/tests/Database/Schema/Grammars/SqlServerSchemaGrammarTest.php index b7ef82eac..8c6845244 100644 --- a/tests/Database/Schema/Grammars/SqlServerSchemaGrammarTest.php +++ b/tests/Database/Schema/Grammars/SqlServerSchemaGrammarTest.php @@ -71,4 +71,20 @@ public function testQuoteInDefaultValue() $statements = $this->runBlueprint($initialBlueprint); $this->assertSame("alter table \"users\" add \"name\" nvarchar(255) not null default 'O''Brian'", $statements[0]); } + + public function testChangeCanRemoveDefaultWithNull() + { + $initialBlueprint = $this->getBlueprint('users'); + $initialBlueprint->string('name')->default('admin'); + + $statements = $this->runBlueprint($initialBlueprint); + $this->assertSame('alter table "users" add "name" nvarchar(255) not null default \'admin\'', $statements[0]); + + $changedBlueprint = $this->getBlueprint('users'); + $changedBlueprint->string('name')->nullable()->default(null)->change(); + + $statements = $this->runBlueprint($changedBlueprint); + $this->assertCount(2, $statements); + $this->assertSame('alter table "users" alter column "name" nvarchar(255) null', $statements[1]); + } } From bacbbd1d590f843d767d83295bcc8def54a47663 Mon Sep 17 00:00:00 2001 From: Jonas Pardon Date: Thu, 8 Oct 2026 11:54:02 +0200 Subject: [PATCH 02/11] Keep column attributes correctly when changing a column on MySQL and MariaDB The change shim read the existing column with the modifier names, so the collation was never kept, and passed reported defaults through as strings: MariaDB's NULL became the string 'NULL' and CURRENT_TIMESTAMP was quoted. It also compiled every changed column for each change command. Keep the unset attributes on a copy of the column, normalise the reported default per server, and let Laravel compile the one column of each command. Co-Authored-By: Claude Opus 5.5 --- phpstan-baseline.neon | 20 ----- .../Grammars/Concerns/MySqlBasedGrammar.php | 87 +++++++++++++------ .../Grammars/MariaDbSchemaGrammarTest.php | 62 +++++++++++++ .../Grammars/MySqlSchemaGrammarTest.php | 57 ++++++++++++ tests/GrammarTestCase.php | 15 ++++ 5 files changed, 193 insertions(+), 48 deletions(-) create mode 100644 tests/Database/Schema/Grammars/MariaDbSchemaGrammarTest.php diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 15e130445..7b668ebcd 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -754,26 +754,6 @@ parameters: identifier: method.childParameterType path: src/Database/Relations/MorphToMany.php - - - message: '#^Access to an undefined property Illuminate\\Database\\Schema\\ColumnDefinition\:\:\$name\.$#' - identifier: property.notFound - path: src/Database/Schema/Grammars/MariaDbGrammar.php - - - - message: '#^Access to an undefined property Illuminate\\Database\\Schema\\ColumnDefinition\:\:\$renameTo\.$#' - identifier: property.notFound - path: src/Database/Schema/Grammars/MariaDbGrammar.php - - - - message: '#^Access to an undefined property Illuminate\\Database\\Schema\\ColumnDefinition\:\:\$name\.$#' - identifier: property.notFound - path: src/Database/Schema/Grammars/MySqlGrammar.php - - - - message: '#^Access to an undefined property Illuminate\\Database\\Schema\\ColumnDefinition\:\:\$renameTo\.$#' - identifier: property.notFound - path: src/Database/Schema/Grammars/MySqlGrammar.php - - message: '#^Access to an undefined property Illuminate\\Database\\Schema\\ColumnDefinition\:\:\$name\.$#' identifier: property.notFound diff --git a/src/Database/Schema/Grammars/Concerns/MySqlBasedGrammar.php b/src/Database/Schema/Grammars/Concerns/MySqlBasedGrammar.php index c0f2992cd..a422ecf6c 100644 --- a/src/Database/Schema/Grammars/Concerns/MySqlBasedGrammar.php +++ b/src/Database/Schema/Grammars/Concerns/MySqlBasedGrammar.php @@ -2,8 +2,10 @@ namespace Winter\Storm\Database\Schema\Grammars\Concerns; +use Illuminate\Database\MySqlConnection; +use Illuminate\Database\Query\Expression; use Illuminate\Database\Schema\Blueprint; -use Illuminate\Database\Schema\ColumnDefinition; +use Illuminate\Database\Schema\Grammars\MariaDbGrammar; use Illuminate\Support\Fluent; trait MySqlBasedGrammar @@ -18,42 +20,71 @@ trait MySqlBasedGrammar * @param \Illuminate\Database\Schema\Blueprint $blueprint * @param \Illuminate\Support\Fluent $command * @return array|string - * - * @throws \RuntimeException */ public function compileChange(Blueprint $blueprint, Fluent $command) { - $columns = []; - $schema = $this->connection->getSchemaBuilder(); - $table = $blueprint->getTable(); - - $oldColumns = collect($schema->getColumns($table)); - - foreach ($blueprint->getChangedColumns() as $column) { - $sql = sprintf( - '%s %s%s %s', - is_null($column->renameTo) ? 'modify' : 'change', - $this->wrap($column->name), - is_null($column->renameTo) ? '' : ' '.$this->wrap($column->renameTo), - $this->getType($column) - ); + /** @var \Illuminate\Database\Schema\ColumnDefinition $column */ + $column = clone $command->get('column'); - $oldColumn = $oldColumns->where('name', $column->name)->first(); - if (!$oldColumn instanceof ColumnDefinition) { - $oldColumn = new ColumnDefinition($oldColumn); - } + $existing = collect($this->connection->getSchemaBuilder()->getColumns($blueprint->getTable())) + ->firstWhere('name', $column->get('name')); - foreach ($this->modifiers as $modifier) { - if (method_exists($this, $method = "modify{$modifier}")) { - $mod = strtolower($modifier); - $col = isset($oldColumn->{$mod}) && !array_key_exists($mod, $column->getAttributes()) ? $oldColumn : $column; - $sql .= $this->{$method}($blueprint, $col); + if ($existing) { + foreach ($this->getKeptColumnAttributes($existing, $column) as $attribute => $value) { + if (!array_key_exists($attribute, $column->getAttributes())) { + $column[$attribute] = $value; } } - $columns[] = $sql; } - return 'alter table '.$this->wrapTable($blueprint).' '.implode(', ', $columns); + return parent::compileChange($blueprint, new Fluent(['column' => $column] + $command->getAttributes())); + } + + /** + * Get the attributes of an existing column, as reported by the schema builder, that a change keeps + * when the new definition doesn't set them. + * + * @param array|\ArrayAccess $existing + * @param \Illuminate\Support\Fluent $column + * @return array + */ + protected function getKeptColumnAttributes($existing, Fluent $column): array + { + $attributes = [ + 'nullable' => (bool) $existing['nullable'], + 'default' => $this->getKeptColumnDefault($existing['default']), + 'comment' => $existing['comment'], + 'unsigned' => str_contains(strtolower((string) $existing['type']), 'unsigned') ?: null, + ]; + + // Like Laravel 9, only keep the collation while the column stays a text column + if (in_array($column->get('type'), ['char', 'string', 'tinyText', 'text', 'mediumText', 'longText', 'enum', 'set'])) { + $attributes['collation'] = $existing['collation']; + } + + return array_filter($attributes, fn ($value) => !is_null($value)); + } + + /** + * Turn the default of an existing column, as reported by the schema builder, back into a definition. + * + * MariaDB reports every default as an SQL expression ('text', NULL, current_timestamp()). MySQL reports + * a literal default as its raw value, and an expression such as CURRENT_TIMESTAMP as is. + * + * @param mixed $default + * @return mixed + */ + protected function getKeptColumnDefault($default) + { + if (!is_string($default)) { + return $default; + } + + if ($this instanceof MariaDbGrammar || ($this->connection instanceof MySqlConnection && $this->connection->isMaria())) { + return $default === 'NULL' ? null : new Expression($default); + } + + return str_starts_with(strtolower($default), 'current_timestamp') ? new Expression($default) : $default; } /** diff --git a/tests/Database/Schema/Grammars/MariaDbSchemaGrammarTest.php b/tests/Database/Schema/Grammars/MariaDbSchemaGrammarTest.php new file mode 100644 index 000000000..2ccf123dc --- /dev/null +++ b/tests/Database/Schema/Grammars/MariaDbSchemaGrammarTest.php @@ -0,0 +1,62 @@ +grammarClass = MariaDbGrammar::class; + $this->builderClass = MariaDbBuilder::class; + + parent::setUp(); + } + + public function testChangeKeepsANullableColumnWithoutADefault() + { + // MariaDB reports a nullable column without a default as the expression NULL + $this->existingColumns([$this->mariaDbColumn('name', 'varchar(255)', nullable: true, default: 'NULL')]); + + $blueprint = $this->getBlueprint('users'); + $blueprint->string('name', 100)->change(); + + $this->assertSame('alter table `users` modify `name` varchar(100) null', $this->runBlueprint($blueprint)[0]); + } + + public function testChangeKeepsAQuotedDefault() + { + $this->existingColumns([$this->mariaDbColumn('name', 'varchar(255)', default: "'O''Brien'")]); + + $blueprint = $this->getBlueprint('users'); + $blueprint->string('name', 100)->change(); + + $this->assertSame("alter table `users` modify `name` varchar(100) not null default 'O''Brien'", $this->runBlueprint($blueprint)[0]); + } + + public function testChangeKeepsACurrentTimestampDefault() + { + $this->existingColumns([$this->mariaDbColumn('created_at', 'timestamp', default: 'current_timestamp()')]); + + $blueprint = $this->getBlueprint('users'); + $blueprint->timestamp('created_at')->nullable()->change(); + + $this->assertSame('alter table `users` modify `created_at` timestamp null default current_timestamp()', $this->runBlueprint($blueprint)[0]); + } + + /** + * A column as MariaDB's getColumns() reports it: every default as an SQL expression. + */ + protected function mariaDbColumn(string $name, string $type, bool $nullable = false, ?string $default = null): array + { + return compact('name', 'type', 'nullable', 'default') + [ + 'type_name' => strtok($type, '('), + 'collation' => null, + 'auto_increment' => false, + 'comment' => null, + 'generation' => null, + ]; + } +} diff --git a/tests/Database/Schema/Grammars/MySqlSchemaGrammarTest.php b/tests/Database/Schema/Grammars/MySqlSchemaGrammarTest.php index cd7322413..f054bf819 100644 --- a/tests/Database/Schema/Grammars/MySqlSchemaGrammarTest.php +++ b/tests/Database/Schema/Grammars/MySqlSchemaGrammarTest.php @@ -85,4 +85,61 @@ public function testChangeCanRemoveDefaultWithNull() $statements = $this->runBlueprint($changedBlueprint); $this->assertSame('alter table `users` modify `name` varchar(255) null', $statements[0]); } + + public function testChangeKeepsTheCollation() + { + $this->existingColumns([$this->mysqlColumn('name', 'varchar(255)', collation: 'utf8mb4_bin')]); + + $blueprint = $this->getBlueprint('users'); + $blueprint->string('name', 100)->change(); + + $this->assertSame("alter table `users` modify `name` varchar(100) collate 'utf8mb4_bin' not null", $this->runBlueprint($blueprint)[0]); + } + + public function testChangeToANonTextTypeDropsTheCollation() + { + $this->existingColumns([$this->mysqlColumn('code', 'varchar(255)', collation: 'utf8mb4_bin')]); + + $blueprint = $this->getBlueprint('users'); + $blueprint->integer('code')->change(); + + $this->assertSame('alter table `users` modify `code` int not null', $this->runBlueprint($blueprint)[0]); + } + + public function testChangeKeepsACurrentTimestampDefault() + { + $this->existingColumns([$this->mysqlColumn('created_at', 'timestamp', default: 'CURRENT_TIMESTAMP')]); + + $blueprint = $this->getBlueprint('users'); + $blueprint->timestamp('created_at')->nullable()->change(); + + $this->assertSame('alter table `users` modify `created_at` timestamp null default CURRENT_TIMESTAMP', $this->runBlueprint($blueprint)[0]); + } + + public function testChangingTwoColumnsCompilesOneStatementPerColumn() + { + $this->existingColumns([$this->mysqlColumn('first', 'varchar(255)'), $this->mysqlColumn('last', 'varchar(255)')]); + + $blueprint = $this->getBlueprint('users'); + $blueprint->string('first', 100)->change(); + $blueprint->string('last', 100)->change(); + + $this->assertSame([ + 'alter table `users` modify `first` varchar(100) not null', + 'alter table `users` modify `last` varchar(100) not null', + ], $this->runBlueprint($blueprint)); + } + + /** + * A column as MySQL's getColumns() reports it: literal defaults unquoted, expressions as is. + */ + protected function mysqlColumn(string $name, string $type, bool $nullable = false, ?string $default = null, ?string $collation = null): array + { + return compact('name', 'type', 'nullable', 'default', 'collation') + [ + 'type_name' => strtok($type, '('), + 'auto_increment' => false, + 'comment' => null, + 'generation' => null, + ]; + } } diff --git a/tests/GrammarTestCase.php b/tests/GrammarTestCase.php index f3e91119c..8150b6b20 100644 --- a/tests/GrammarTestCase.php +++ b/tests/GrammarTestCase.php @@ -64,6 +64,21 @@ protected function getSchemaBuilder($blueprint) ->getMock(); } + /** + * Report the given rows, in the format of Builder::getColumns(), as the table's existing columns. + * Call this before getBlueprint(). + */ + protected function existingColumns(array $columns): void + { + $builder = m::mock(Builder::class) + ->shouldReceive('getColumns')->andReturn($columns) + ->shouldReceive('getForeignKeys')->andReturn([]) + ->shouldReceive('getIndexes')->andReturn([]) + ->getMock(); + + $this->connection->shouldReceive('getSchemaBuilder')->andReturn($builder); + } + protected function runBlueprint(Blueprint $blueprint) { return $blueprint->toSql(); From f8e72930598939ec31a28cbd98faa80f91471dd9 Mon Sep 17 00:00:00 2001 From: Jonas Pardon Date: Thu, 8 Oct 2026 15:54:23 +0200 Subject: [PATCH 03/11] Read the column attributes once when keeping them Co-Authored-By: Claude Opus 5.5 --- src/Database/Schema/Grammars/Concerns/MySqlBasedGrammar.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Database/Schema/Grammars/Concerns/MySqlBasedGrammar.php b/src/Database/Schema/Grammars/Concerns/MySqlBasedGrammar.php index a422ecf6c..41ba6a3d8 100644 --- a/src/Database/Schema/Grammars/Concerns/MySqlBasedGrammar.php +++ b/src/Database/Schema/Grammars/Concerns/MySqlBasedGrammar.php @@ -30,8 +30,10 @@ public function compileChange(Blueprint $blueprint, Fluent $command) ->firstWhere('name', $column->get('name')); if ($existing) { + $attributes = $column->getAttributes(); + foreach ($this->getKeptColumnAttributes($existing, $column) as $attribute => $value) { - if (!array_key_exists($attribute, $column->getAttributes())) { + if (!array_key_exists($attribute, $attributes)) { $column[$attribute] = $value; } } From 23271c10c93409e487073f67e319f48468dc8819 Mon Sep 17 00:00:00 2001 From: Jonas Pardon Date: Thu, 8 Oct 2026 15:58:04 +0200 Subject: [PATCH 04/11] Read the column attributes once per changed column Co-Authored-By: Claude Opus 5.5 --- src/Database/Schema/BlueprintState.php | 4 +++- src/Database/Schema/Grammars/PostgresGrammar.php | 4 +++- src/Database/Schema/Grammars/SqlServerGrammar.php | 4 +++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/Database/Schema/BlueprintState.php b/src/Database/Schema/BlueprintState.php index f480ead91..275d3d4dd 100644 --- a/src/Database/Schema/BlueprintState.php +++ b/src/Database/Schema/BlueprintState.php @@ -65,8 +65,10 @@ protected function preserveExistingColumnAttributes(Fluent $column): void continue; } + $attributes = $column->getAttributes(); + foreach ($this->preservedAttributes as $attribute) { - if (!array_key_exists($attribute, $column->getAttributes()) && isset($existing[$attribute])) { + if (!array_key_exists($attribute, $attributes) && isset($existing[$attribute])) { $column[$attribute] = $existing[$attribute]; } } diff --git a/src/Database/Schema/Grammars/PostgresGrammar.php b/src/Database/Schema/Grammars/PostgresGrammar.php index b15e85bdb..fc1175986 100755 --- a/src/Database/Schema/Grammars/PostgresGrammar.php +++ b/src/Database/Schema/Grammars/PostgresGrammar.php @@ -38,6 +38,8 @@ public function compileChange(Blueprint $blueprint, Fluent $command) $oldColumn = new ColumnDefinition($oldColumn); } + $attributes = $column->getAttributes(); + foreach ($this->modifiers as $modifier) { if ($modifier === 'Collate') { continue; @@ -45,7 +47,7 @@ public function compileChange(Blueprint $blueprint, Fluent $command) if (method_exists($this, $method = "modify{$modifier}")) { $mod = strtolower($modifier); - $col = isset($oldColumn->{$mod}) && ! array_key_exists($mod, $column->getAttributes()) ? $oldColumn : $column; + $col = isset($oldColumn->{$mod}) && ! array_key_exists($mod, $attributes) ? $oldColumn : $column; $constraints = (array) $this->{$method}($blueprint, $col); foreach ($constraints as $constraint) { diff --git a/src/Database/Schema/Grammars/SqlServerGrammar.php b/src/Database/Schema/Grammars/SqlServerGrammar.php index 526e6f397..629e597f4 100755 --- a/src/Database/Schema/Grammars/SqlServerGrammar.php +++ b/src/Database/Schema/Grammars/SqlServerGrammar.php @@ -43,10 +43,12 @@ public function compileChange(Blueprint $blueprint, Fluent $command) $oldColumn = new ColumnDefinition($oldColumn); } + $attributes = $column->getAttributes(); + foreach ($this->modifiers as $modifier) { if (method_exists($this, $method = "modify{$modifier}")) { $mod = strtolower($modifier); - $col = isset($oldColumn->{$mod}) && !array_key_exists($mod, $column->getAttributes()) ? $oldColumn : $column; + $col = isset($oldColumn->{$mod}) && !array_key_exists($mod, $attributes) ? $oldColumn : $column; $sql .= $this->{$method}($blueprint, $col); } } From e87c9fb467498b3a901fd3369c1dea3efb27541c Mon Sep 17 00:00:00 2001 From: Jonas Pardon Date: Thu, 8 Oct 2026 16:56:59 +0200 Subject: [PATCH 05/11] Fix changing a column on PostgreSQL and SQL Server The ->change() shims compiled the kept attributes as a new column definition, which PostgreSQL and SQL Server reject. Copy them onto the changed column instead and let Laravel compile it, as for MySQL and MariaDB. Co-Authored-By: Claude Opus 5.5 --- phpstan-baseline.neon | 10 --- .../Schema/Grammars/PostgresGrammar.php | 64 ++++++++------- .../Schema/Grammars/SqlServerGrammar.php | 62 ++++++++------- .../Grammars/PostgresSchemaGrammarTest.php | 79 ++++++++++++++++++- .../Grammars/SqlServerSchemaGrammarTest.php | 65 +++++++++++++++ 5 files changed, 213 insertions(+), 67 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 7b668ebcd..305eb9ee5 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -754,16 +754,6 @@ parameters: identifier: method.childParameterType path: src/Database/Relations/MorphToMany.php - - - message: '#^Access to an undefined property Illuminate\\Database\\Schema\\ColumnDefinition\:\:\$name\.$#' - identifier: property.notFound - path: src/Database/Schema/Grammars/PostgresGrammar.php - - - - message: '#^Access to an undefined property Illuminate\\Database\\Schema\\ColumnDefinition\:\:\$name\.$#' - identifier: property.notFound - path: src/Database/Schema/Grammars/SqlServerGrammar.php - - message: '#^Call to an undefined method Illuminate\\Database\\Eloquent\\Model\:\:getSortOrderColumn\(\)\.$#' identifier: method.notFound diff --git a/src/Database/Schema/Grammars/PostgresGrammar.php b/src/Database/Schema/Grammars/PostgresGrammar.php index fc1175986..ab4851e33 100755 --- a/src/Database/Schema/Grammars/PostgresGrammar.php +++ b/src/Database/Schema/Grammars/PostgresGrammar.php @@ -2,8 +2,8 @@ namespace Winter\Storm\Database\Schema\Grammars; +use Illuminate\Database\Query\Expression; use Illuminate\Database\Schema\Blueprint; -use Illuminate\Database\Schema\ColumnDefinition; use Illuminate\Database\Schema\Grammars\PostgresGrammar as BasePostgresGrammar; use Illuminate\Support\Fluent; @@ -16,50 +16,56 @@ class PostgresGrammar extends BasePostgresGrammar * This restores Laravel previous behavior where existing column attributes are kept * unless they get changed by the new Blueprint. * + * The kept attributes are set on the column itself, so that the comment command that runs + * after the change keeps the existing comment as well. + * * @param \Illuminate\Database\Schema\Blueprint $blueprint * @param \Illuminate\Support\Fluent $command * @return array|string - * - * @throws \RuntimeException */ public function compileChange(Blueprint $blueprint, Fluent $command) { - $columns = []; - $schema = $this->connection->getSchemaBuilder(); - $table = $blueprint->getTable(); + /** @var \Illuminate\Database\Schema\ColumnDefinition $column */ + $column = $command->get('column'); - $oldColumns = collect($schema->getColumns($table)); - - foreach ($blueprint->getChangedColumns() as $column) { - $changes = ['type '.$this->getType($column).$this->modifyCollate($blueprint, $column)]; - - $oldColumn = $oldColumns->where('name', $column->name)->first(); - if (!$oldColumn instanceof ColumnDefinition) { - $oldColumn = new ColumnDefinition($oldColumn); - } + $existing = collect($this->connection->getSchemaBuilder()->getColumns($blueprint->getTable())) + ->firstWhere('name', $column->get('name')); + if ($existing) { $attributes = $column->getAttributes(); - foreach ($this->modifiers as $modifier) { - if ($modifier === 'Collate') { - continue; + foreach ($this->getKeptColumnAttributes($existing, $column) as $attribute => $value) { + if (!array_key_exists($attribute, $attributes)) { + $column[$attribute] = $value; } + } + } - if (method_exists($this, $method = "modify{$modifier}")) { - $mod = strtolower($modifier); - $col = isset($oldColumn->{$mod}) && ! array_key_exists($mod, $attributes) ? $oldColumn : $column; - $constraints = (array) $this->{$method}($blueprint, $col); + return parent::compileChange($blueprint, $command); + } - foreach ($constraints as $constraint) { - $changes[] = $constraint; - } - } - } + /** + * Get the attributes of an existing column, as reported by the schema builder, that a change keeps + * when the new definition doesn't set them. + * + * @param array|\ArrayAccess $existing + * @param \Illuminate\Support\Fluent $column + * @return array + */ + protected function getKeptColumnAttributes($existing, Fluent $column): array + { + $attributes = [ + 'nullable' => (bool) $existing['nullable'], + // PostgreSQL reports every default as an SQL expression, such as 'text'::character varying + 'default' => is_null($existing['default']) ? null : new Expression($existing['default']), + 'comment' => $existing['comment'], + ]; - $columns[] = implode(', ', $this->prefixArray('alter column '.$this->wrap($column->name), $changes)); + if (in_array($column->get('type'), ['char', 'string', 'tinyText', 'text', 'mediumText', 'longText', 'enum', 'set'])) { + $attributes['collation'] = $existing['collation']; } - return 'alter table '.$this->wrapTable($blueprint).' '.implode(', ', $columns); + return array_filter($attributes, fn ($value) => !is_null($value)); } /** diff --git a/src/Database/Schema/Grammars/SqlServerGrammar.php b/src/Database/Schema/Grammars/SqlServerGrammar.php index 629e597f4..2495ccd90 100755 --- a/src/Database/Schema/Grammars/SqlServerGrammar.php +++ b/src/Database/Schema/Grammars/SqlServerGrammar.php @@ -2,8 +2,8 @@ namespace Winter\Storm\Database\Schema\Grammars; +use Illuminate\Database\Query\Expression; use Illuminate\Database\Schema\Blueprint; -use Illuminate\Database\Schema\ColumnDefinition; use Illuminate\Database\Schema\Grammars\SqlServerGrammar as BaseSqlServerGrammar; use Illuminate\Support\Fluent; @@ -16,47 +16,55 @@ class SqlServerGrammar extends BaseSqlServerGrammar * This restores Laravel previous behavior where existing column attributes are kept * unless they get changed by the new Blueprint. * + * The kept attributes are set on the column itself, so that the default command that runs + * after the change adds the existing default back once the change has dropped its constraint. + * * @param \Illuminate\Database\Schema\Blueprint $blueprint * @param \Illuminate\Support\Fluent $command * @return array|string - * - * @throws \RuntimeException */ public function compileChange(Blueprint $blueprint, Fluent $command) { - $changes = (array) $this->compileDropDefaultConstraint($blueprint, $command); - $schema = $this->connection->getSchemaBuilder(); - $table = $blueprint->getTable(); + /** @var \Illuminate\Database\Schema\ColumnDefinition $column */ + $column = $command->get('column'); - $oldColumns = collect($schema->getColumns($table)); - - foreach ($blueprint->getChangedColumns() as $column) { - $sql = sprintf( - 'alter table %s alter column %s %s', - $this->wrapTable($blueprint), - $this->wrap($column->name), - $this->getType($column) - ); - - $oldColumn = $oldColumns->where('name', $column->name)->first(); - if (!$oldColumn instanceof ColumnDefinition) { - $oldColumn = new ColumnDefinition($oldColumn); - } + $existing = collect($this->connection->getSchemaBuilder()->getColumns($blueprint->getTable())) + ->firstWhere('name', $column->get('name')); + if ($existing) { $attributes = $column->getAttributes(); - foreach ($this->modifiers as $modifier) { - if (method_exists($this, $method = "modify{$modifier}")) { - $mod = strtolower($modifier); - $col = isset($oldColumn->{$mod}) && !array_key_exists($mod, $attributes) ? $oldColumn : $column; - $sql .= $this->{$method}($blueprint, $col); + foreach ($this->getKeptColumnAttributes($existing, $column) as $attribute => $value) { + if (!array_key_exists($attribute, $attributes)) { + $column[$attribute] = $value; } } + } + + return parent::compileChange($blueprint, $command); + } + + /** + * Get the attributes of an existing column, as reported by the schema builder, that a change keeps + * when the new definition doesn't set them. + * + * @param array|\ArrayAccess $existing + * @param \Illuminate\Support\Fluent $column + * @return array + */ + protected function getKeptColumnAttributes($existing, Fluent $column): array + { + $attributes = [ + 'nullable' => (bool) $existing['nullable'], + // SQL Server reports a default as the expression of its constraint, such as ('text') + 'default' => is_null($existing['default']) ? null : new Expression($existing['default']), + ]; - $changes[] = $sql; + if (in_array($column->get('type'), ['char', 'string', 'tinyText', 'text', 'mediumText', 'longText', 'enum', 'set'])) { + $attributes['collation'] = $existing['collation']; } - return $changes; + return array_filter($attributes, fn ($value) => !is_null($value)); } /** diff --git a/tests/Database/Schema/Grammars/PostgresSchemaGrammarTest.php b/tests/Database/Schema/Grammars/PostgresSchemaGrammarTest.php index 4d86330db..ba7857a21 100644 --- a/tests/Database/Schema/Grammars/PostgresSchemaGrammarTest.php +++ b/tests/Database/Schema/Grammars/PostgresSchemaGrammarTest.php @@ -48,7 +48,7 @@ public function testNullableInitialModifierAddDefault() $statements = $this->runBlueprint($changedBlueprint); $parts = explode(', ', $statements[0]); $this->assertSame('alter table "users" alter column "name" type varchar(255)', $parts[0]); - $this->assertSame('alter column "name" null', $parts[1]); + $this->assertSame('alter column "name" drop not null', $parts[1]); $this->assertSame("alter column \"name\" set default 'admin'", $parts[2]); } @@ -94,4 +94,81 @@ public function testChangeCanRemoveDefaultWithNull() $statements = $this->runBlueprint($changedBlueprint); $this->assertSame('alter table "users" alter column "name" type varchar(255), alter column "name" drop not null, alter column "name" drop default, alter column "name" drop identity if exists', $statements[0]); } + + public function testChangeKeepsADefault() + { + $this->existingColumns([$this->postgresColumn('role', 'character varying(255)', default: "'O''Brien'::character varying")]); + + $blueprint = $this->getBlueprint('users'); + $blueprint->string('role', 100)->change(); + + $this->assertSame( + 'alter table "users" alter column "role" type varchar(100), alter column "role" set not null, alter column "role" set default \'O\'\'Brien\'::character varying, alter column "role" drop identity if exists', + $this->runBlueprint($blueprint)[0] + ); + } + + public function testChangeKeepsACurrentTimestampDefault() + { + $this->existingColumns([$this->postgresColumn('created_at', 'timestamp(0) without time zone', default: 'CURRENT_TIMESTAMP')]); + + $blueprint = $this->getBlueprint('users'); + $blueprint->timestamp('created_at')->nullable()->change(); + + $this->assertSame( + 'alter table "users" alter column "created_at" type timestamp(0) without time zone, alter column "created_at" drop not null, alter column "created_at" set default CURRENT_TIMESTAMP, alter column "created_at" drop identity if exists', + $this->runBlueprint($blueprint)[0] + ); + } + + public function testChangeKeepsTheCollation() + { + $this->existingColumns([$this->postgresColumn('code', 'character varying(255)', collation: 'C')]); + + $blueprint = $this->getBlueprint('users'); + $blueprint->string('code', 100)->change(); + + $this->assertSame( + 'alter table "users" alter column "code" type varchar(100) collate "C", alter column "code" set not null, alter column "code" drop default, alter column "code" drop identity if exists', + $this->runBlueprint($blueprint)[0] + ); + } + + public function testChangeKeepsTheComment() + { + $this->existingColumns([$this->postgresColumn('note', 'character varying(255)', comment: 'a note')]); + + $blueprint = $this->getBlueprint('users'); + $blueprint->string('note', 100)->change(); + + $this->assertSame('comment on column "users"."note" is \'a note\'', $this->runBlueprint($blueprint)[1]); + } + + public function testChangingTwoColumnsCompilesOneStatementPerColumn() + { + $this->existingColumns([$this->postgresColumn('first', 'character varying(255)'), $this->postgresColumn('last', 'character varying(255)')]); + + $blueprint = $this->getBlueprint('users'); + $blueprint->string('first', 100)->change(); + $blueprint->string('last', 100)->change(); + + $this->assertSame([ + 'alter table "users" alter column "first" type varchar(100), alter column "first" set not null, alter column "first" drop default, alter column "first" drop identity if exists', + 'alter table "users" alter column "last" type varchar(100), alter column "last" set not null, alter column "last" drop default, alter column "last" drop identity if exists', + 'comment on column "users"."first" is NULL', + 'comment on column "users"."last" is NULL', + ], $this->runBlueprint($blueprint)); + } + + /** + * A column as PostgreSQL's getColumns() reports it: every default as an SQL expression. + */ + protected function postgresColumn(string $name, string $type, bool $nullable = false, ?string $default = null, ?string $collation = null, ?string $comment = null): array + { + return compact('name', 'type', 'nullable', 'default', 'collation', 'comment') + [ + 'type_name' => strtok($type, '('), + 'auto_increment' => false, + 'generation' => null, + ]; + } } diff --git a/tests/Database/Schema/Grammars/SqlServerSchemaGrammarTest.php b/tests/Database/Schema/Grammars/SqlServerSchemaGrammarTest.php index 8c6845244..7d1f682e9 100644 --- a/tests/Database/Schema/Grammars/SqlServerSchemaGrammarTest.php +++ b/tests/Database/Schema/Grammars/SqlServerSchemaGrammarTest.php @@ -87,4 +87,69 @@ public function testChangeCanRemoveDefaultWithNull() $this->assertCount(2, $statements); $this->assertSame('alter table "users" alter column "name" nvarchar(255) null', $statements[1]); } + + public function testChangeKeepsADefault() + { + $this->existingColumns([$this->sqlServerColumn('role', 'nvarchar(510)', default: "('O''Brien')", collation: 'SQL_Latin1_General_CP1_CI_AS')]); + + $blueprint = $this->getBlueprint('users'); + $blueprint->string('role', 100)->change(); + + $statements = $this->runBlueprint($blueprint); + $this->assertSame('alter table "users" alter column "role" nvarchar(100) collate SQL_Latin1_General_CP1_CI_AS not null', $statements[1]); + $this->assertSame('alter table "users" add default (\'O\'\'Brien\') for "role"', $statements[2]); + } + + public function testChangeKeepsACurrentTimestampDefault() + { + $this->existingColumns([$this->sqlServerColumn('created_at', 'datetime', default: '(getdate())')]); + + $blueprint = $this->getBlueprint('users'); + $blueprint->timestamp('created_at')->nullable()->change(); + + $statements = $this->runBlueprint($blueprint); + $this->assertSame('alter table "users" alter column "created_at" datetime null', $statements[1]); + $this->assertSame('alter table "users" add default (getdate()) for "created_at"', $statements[2]); + } + + public function testChangeKeepsTheCollation() + { + $this->existingColumns([$this->sqlServerColumn('code', 'nvarchar(510)', collation: 'Latin1_General_BIN')]); + + $blueprint = $this->getBlueprint('users'); + $blueprint->string('code', 100)->change(); + + $statements = $this->runBlueprint($blueprint); + $this->assertCount(2, $statements); + $this->assertSame('alter table "users" alter column "code" nvarchar(100) collate Latin1_General_BIN not null', $statements[1]); + } + + public function testChangingTwoColumnsCompilesOneStatementPerColumn() + { + $this->existingColumns([$this->sqlServerColumn('first', 'nvarchar(510)'), $this->sqlServerColumn('last', 'nvarchar(510)')]); + + $blueprint = $this->getBlueprint('users'); + $blueprint->string('first', 100)->change(); + $blueprint->string('last', 100)->change(); + + $statements = $this->runBlueprint($blueprint); + $this->assertCount(4, $statements); + $this->assertStringContainsString("[name] in ('first')", $statements[0]); + $this->assertSame('alter table "users" alter column "first" nvarchar(100) not null', $statements[1]); + $this->assertStringContainsString("[name] in ('last')", $statements[2]); + $this->assertSame('alter table "users" alter column "last" nvarchar(100) not null', $statements[3]); + } + + /** + * A column as SQL Server's getColumns() reports it: a default as the expression of its constraint. + */ + protected function sqlServerColumn(string $name, string $type, bool $nullable = false, ?string $default = null, ?string $collation = null): array + { + return compact('name', 'type', 'nullable', 'default', 'collation') + [ + 'type_name' => strtok($type, '('), + 'auto_increment' => false, + 'comment' => null, + 'generation' => null, + ]; + } } From 3c493b8654a9ff320fa29748efcdd04fb7208795 Mon Sep 17 00:00:00 2001 From: Jonas Pardon Date: Fri, 9 Oct 2026 08:27:20 +0200 Subject: [PATCH 06/11] Change the column itself on MySQL and MariaDB too Co-Authored-By: Claude Opus 5.5 --- src/Database/Schema/Grammars/Concerns/MySqlBasedGrammar.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Database/Schema/Grammars/Concerns/MySqlBasedGrammar.php b/src/Database/Schema/Grammars/Concerns/MySqlBasedGrammar.php index 41ba6a3d8..b3b0a1a06 100644 --- a/src/Database/Schema/Grammars/Concerns/MySqlBasedGrammar.php +++ b/src/Database/Schema/Grammars/Concerns/MySqlBasedGrammar.php @@ -24,7 +24,7 @@ trait MySqlBasedGrammar public function compileChange(Blueprint $blueprint, Fluent $command) { /** @var \Illuminate\Database\Schema\ColumnDefinition $column */ - $column = clone $command->get('column'); + $column = $command->get('column'); $existing = collect($this->connection->getSchemaBuilder()->getColumns($blueprint->getTable())) ->firstWhere('name', $column->get('name')); @@ -39,7 +39,7 @@ public function compileChange(Blueprint $blueprint, Fluent $command) } } - return parent::compileChange($blueprint, new Fluent(['column' => $column] + $command->getAttributes())); + return parent::compileChange($blueprint, $command); } /** From 673da55880e56eaa0d05c3e1dfd64f3ca10c9ead Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Thu, 22 Oct 2026 12:12:25 -0600 Subject: [PATCH 07/11] Only keep the unsigned attribute for a numeric column getKeptColumnAttributes() copied unsigned whenever the existing column's reported type contained it, without checking the type the column is being changed to, so changing an unsigned numeric column to a type that doesn't set unsigned itself emitted it on that type: `$table->string('code', 50)->change()` over an `int unsigned` column compiled to `modify `code` varchar(50) unsigned not null`, which MySQL rejects with a 1064 syntax error. The same happened for text, date, dateTime, timestamp and json. Guard it on the new type like the collation already is. Winter 1.2 and wip/1.3 never kept unsigned at all, so this only affected this branch. --- .../Grammars/Concerns/MySqlBasedGrammar.php | 6 +++++- .../Grammars/MySqlSchemaGrammarTest.php | 21 +++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/Database/Schema/Grammars/Concerns/MySqlBasedGrammar.php b/src/Database/Schema/Grammars/Concerns/MySqlBasedGrammar.php index b3b0a1a06..038cafda3 100644 --- a/src/Database/Schema/Grammars/Concerns/MySqlBasedGrammar.php +++ b/src/Database/Schema/Grammars/Concerns/MySqlBasedGrammar.php @@ -56,9 +56,13 @@ protected function getKeptColumnAttributes($existing, Fluent $column): array 'nullable' => (bool) $existing['nullable'], 'default' => $this->getKeptColumnDefault($existing['default']), 'comment' => $existing['comment'], - 'unsigned' => str_contains(strtolower((string) $existing['type']), 'unsigned') ?: null, ]; + // Only keep the unsigned attribute while the column stays a numeric column, as no other type accepts it + if (in_array($column->get('type'), ['boolean', 'tinyInteger', 'smallInteger', 'mediumInteger', 'integer', 'bigInteger', 'decimal', 'float', 'double'])) { + $attributes['unsigned'] = str_contains(strtolower((string) $existing['type']), 'unsigned') ?: null; + } + // Like Laravel 9, only keep the collation while the column stays a text column if (in_array($column->get('type'), ['char', 'string', 'tinyText', 'text', 'mediumText', 'longText', 'enum', 'set'])) { $attributes['collation'] = $existing['collation']; diff --git a/tests/Database/Schema/Grammars/MySqlSchemaGrammarTest.php b/tests/Database/Schema/Grammars/MySqlSchemaGrammarTest.php index f054bf819..ec14d2420 100644 --- a/tests/Database/Schema/Grammars/MySqlSchemaGrammarTest.php +++ b/tests/Database/Schema/Grammars/MySqlSchemaGrammarTest.php @@ -116,6 +116,27 @@ public function testChangeKeepsACurrentTimestampDefault() $this->assertSame('alter table `users` modify `created_at` timestamp null default CURRENT_TIMESTAMP', $this->runBlueprint($blueprint)[0]); } + public function testChangeKeepsTheUnsignedAttribute() + { + $this->existingColumns([$this->mysqlColumn('amount', 'decimal(8,2) unsigned')]); + + $blueprint = $this->getBlueprint('users'); + $blueprint->decimal('amount', 10, 2)->change(); + + $this->assertSame('alter table `users` modify `amount` decimal(10, 2) unsigned not null', $this->runBlueprint($blueprint)[0]); + } + + public function testChangeToANonNumericTypeDropsTheUnsignedAttribute() + { + // Only a numeric column accepts the unsigned attribute, so keeping it here would be a syntax error + $this->existingColumns([$this->mysqlColumn('code', 'int unsigned')]); + + $blueprint = $this->getBlueprint('users'); + $blueprint->string('code', 50)->change(); + + $this->assertSame('alter table `users` modify `code` varchar(50) not null', $this->runBlueprint($blueprint)[0]); + } + public function testChangingTwoColumnsCompilesOneStatementPerColumn() { $this->existingColumns([$this->mysqlColumn('first', 'varchar(255)'), $this->mysqlColumn('last', 'varchar(255)')]); From 03dc1578590274604bc5e5ed3f45a6f265021d76 Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Thu, 22 Oct 2026 12:13:28 -0600 Subject: [PATCH 08/11] Make the existing column helper independent of call order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit existingColumns() registered a second getSchemaBuilder expectation on the connection mock. Mockery uses the first matching expectation that is still eligible, so the one getBlueprint() registers won the race whenever it was registered first, and the helper silently reported the blueprint's own columns instead of the given rows — a test written in the natural order would quietly assert the wrong thing. Keep the rows on the test case and let the single getSchemaBuilder mock prefer them, so the order no longer matters. --- tests/GrammarTestCase.php | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/GrammarTestCase.php b/tests/GrammarTestCase.php index 8150b6b20..d34b3c558 100644 --- a/tests/GrammarTestCase.php +++ b/tests/GrammarTestCase.php @@ -14,6 +14,13 @@ class GrammarTestCase extends TestCase protected $grammarClass = null; protected $builderClass = null; + /** + * The rows existingColumns() reports as the table's existing columns, if any. + * + * @var array>|null + */ + protected $reportedColumns = null; + public function setUp(): void { parent::setUp(); @@ -57,7 +64,7 @@ protected function getSchemaGrammar() protected function getSchemaBuilder($blueprint) { return m::mock(Builder::class) - ->shouldReceive('getColumns')->andReturn($blueprint->getColumns()) + ->shouldReceive('getColumns')->andReturn($this->reportedColumns ?? $blueprint->getColumns()) ->shouldReceive('getForeignKeys')->andReturn([]) ->shouldReceive('getIndexes')->andReturn([]) ->shouldReceive('parseSchemaAndTable')->andReturn([null, $blueprint->getTable()]) @@ -66,17 +73,10 @@ protected function getSchemaBuilder($blueprint) /** * Report the given rows, in the format of Builder::getColumns(), as the table's existing columns. - * Call this before getBlueprint(). */ protected function existingColumns(array $columns): void { - $builder = m::mock(Builder::class) - ->shouldReceive('getColumns')->andReturn($columns) - ->shouldReceive('getForeignKeys')->andReturn([]) - ->shouldReceive('getIndexes')->andReturn([]) - ->getMock(); - - $this->connection->shouldReceive('getSchemaBuilder')->andReturn($builder); + $this->reportedColumns = $columns; } protected function runBlueprint(Blueprint $blueprint) From 696a476a766f1da7c7f76c66481289f90e0a3a52 Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Thu, 22 Oct 2026 12:14:23 -0600 Subject: [PATCH 09/11] Put the new MariaDB test in the Winter\Storm\Tests namespace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Postgres, SQL Server and SQLite grammar tests next to it declare Winter\Storm\Tests\Database\Schema\Grammars, which is what autoload-dev maps tests/ to; the new file copied the bare Tests\ namespace from MySqlSchemaGrammarTest instead. Cosmetic only — PHPUnit finds these by file either way, and the tests/ tree has many other classes outside the mapped namespace. --- tests/Database/Schema/Grammars/MariaDbSchemaGrammarTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/Database/Schema/Grammars/MariaDbSchemaGrammarTest.php b/tests/Database/Schema/Grammars/MariaDbSchemaGrammarTest.php index 2ccf123dc..0f44d9715 100644 --- a/tests/Database/Schema/Grammars/MariaDbSchemaGrammarTest.php +++ b/tests/Database/Schema/Grammars/MariaDbSchemaGrammarTest.php @@ -1,9 +1,10 @@ Date: Thu, 22 Oct 2026 12:15:22 -0600 Subject: [PATCH 10/11] Cover a MariaDB server reached through the mysql driver getKeptColumnDefault() treats a reported default as an SQL expression either when the grammar is the MariaDB one or when a MySqlConnection reports isMaria(), but nothing exercised the second arm: GrammarTestCase mocks Illuminate\Database\Connection, so the instanceof check short-circuits, and the MariaDB tests take the grammar arm. Mock a MySqlConnection that reports isMaria() instead. Without that arm the column comes back with `default 'NULL'`, which is the corruption this branch set out to fix. --- .../Grammars/MySqlSchemaGrammarTest.php | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/Database/Schema/Grammars/MySqlSchemaGrammarTest.php b/tests/Database/Schema/Grammars/MySqlSchemaGrammarTest.php index ec14d2420..ef518c694 100644 --- a/tests/Database/Schema/Grammars/MySqlSchemaGrammarTest.php +++ b/tests/Database/Schema/Grammars/MySqlSchemaGrammarTest.php @@ -2,7 +2,9 @@ namespace Tests\Database\Schema\Grammars; +use Illuminate\Database\MySqlConnection; use Illuminate\Database\Schema\MySqlBuilder; +use Mockery as m; use Winter\Storm\Database\Schema\Grammars\MySqlGrammar; use Winter\Storm\Tests\GrammarTestCase; @@ -137,6 +139,26 @@ public function testChangeToANonNumericTypeDropsTheUnsignedAttribute() $this->assertSame('alter table `users` modify `code` varchar(50) not null', $this->runBlueprint($blueprint)[0]); } + public function testChangeOnAMariaDbServerTreatsTheReportedDefaultAsAnExpression() + { + // The mysql driver can point at a MariaDB server, which reports no default as the expression NULL + $this->connection = m::mock(MySqlConnection::class) + ->shouldReceive('getTablePrefix')->andReturn('') + ->shouldReceive('getServerVersion')->andReturn('11.4.13') + ->shouldReceive('isMaria')->andReturn(true) + ->shouldReceive('scalar')->andReturn('') + ->getMock(); + + $this->connection->shouldReceive('getSchemaGrammar')->andReturn($this->getSchemaGrammar()); + + $this->existingColumns([$this->mysqlColumn('name', 'varchar(255)', nullable: true, default: 'NULL')]); + + $blueprint = $this->getBlueprint('users'); + $blueprint->string('name', 100)->change(); + + $this->assertSame('alter table `users` modify `name` varchar(100) null', $this->runBlueprint($blueprint)[0]); + } + public function testChangingTwoColumnsCompilesOneStatementPerColumn() { $this->existingColumns([$this->mysqlColumn('first', 'varchar(255)'), $this->mysqlColumn('last', 'varchar(255)')]); From dda509e55efb126ec8c50d7061db8c29fa4753b2 Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Thu, 22 Oct 2026 12:17:26 -0600 Subject: [PATCH 11/11] Share the kept column attribute logic between the grammars The existing column lookup, the array_key_exists copy loop, the null filter and the list of text column types were repeated in MySqlBasedGrammar, PostgresGrammar and SqlServerGrammar, so the collation type list in particular had three copies to keep in step. Move them into a KeepsExistingColumnAttributes concern that declares getKeptColumnAttributes() abstract, leaving each grammar with only the attributes it keeps and the form its server reports them in. No behaviour change. --- .../KeepsExistingColumnAttributes.php | 60 +++++++++++++++++++ .../Grammars/Concerns/MySqlBasedGrammar.php | 29 ++------- .../Schema/Grammars/PostgresGrammar.php | 28 ++------- .../Schema/Grammars/SqlServerGrammar.php | 28 ++------- 4 files changed, 78 insertions(+), 67 deletions(-) create mode 100644 src/Database/Schema/Grammars/Concerns/KeepsExistingColumnAttributes.php diff --git a/src/Database/Schema/Grammars/Concerns/KeepsExistingColumnAttributes.php b/src/Database/Schema/Grammars/Concerns/KeepsExistingColumnAttributes.php new file mode 100644 index 000000000..877c94644 --- /dev/null +++ b/src/Database/Schema/Grammars/Concerns/KeepsExistingColumnAttributes.php @@ -0,0 +1,60 @@ +connection->getSchemaBuilder()->getColumns($blueprint->getTable())) + ->firstWhere('name', $column->get('name')); + + if (!$existing) { + return; + } + + $attributes = $column->getAttributes(); + + foreach ($this->getKeptColumnAttributes($existing, $column) as $attribute => $value) { + if (!is_null($value) && !array_key_exists($attribute, $attributes)) { + $column[$attribute] = $value; + } + } + } + + /** + * Whether the column is being changed to a type that accepts a collation. + * + * Like Laravel 9, the collation is only kept while the column stays a text column. + */ + protected function changesToATextColumn(Fluent $column): bool + { + return in_array($column->get('type'), [ + 'char', 'string', 'tinyText', 'text', 'mediumText', 'longText', 'enum', 'set', + ]); + } +} diff --git a/src/Database/Schema/Grammars/Concerns/MySqlBasedGrammar.php b/src/Database/Schema/Grammars/Concerns/MySqlBasedGrammar.php index 038cafda3..201b96c3e 100644 --- a/src/Database/Schema/Grammars/Concerns/MySqlBasedGrammar.php +++ b/src/Database/Schema/Grammars/Concerns/MySqlBasedGrammar.php @@ -10,6 +10,8 @@ trait MySqlBasedGrammar { + use KeepsExistingColumnAttributes; + /** * Compile a change column command into a series of SQL statements. * @@ -26,30 +28,12 @@ public function compileChange(Blueprint $blueprint, Fluent $command) /** @var \Illuminate\Database\Schema\ColumnDefinition $column */ $column = $command->get('column'); - $existing = collect($this->connection->getSchemaBuilder()->getColumns($blueprint->getTable())) - ->firstWhere('name', $column->get('name')); - - if ($existing) { - $attributes = $column->getAttributes(); - - foreach ($this->getKeptColumnAttributes($existing, $column) as $attribute => $value) { - if (!array_key_exists($attribute, $attributes)) { - $column[$attribute] = $value; - } - } - } + $this->keepExistingColumnAttributes($blueprint, $column); return parent::compileChange($blueprint, $command); } - /** - * Get the attributes of an existing column, as reported by the schema builder, that a change keeps - * when the new definition doesn't set them. - * - * @param array|\ArrayAccess $existing - * @param \Illuminate\Support\Fluent $column - * @return array - */ + /** @inheritDoc */ protected function getKeptColumnAttributes($existing, Fluent $column): array { $attributes = [ @@ -63,12 +47,11 @@ protected function getKeptColumnAttributes($existing, Fluent $column): array $attributes['unsigned'] = str_contains(strtolower((string) $existing['type']), 'unsigned') ?: null; } - // Like Laravel 9, only keep the collation while the column stays a text column - if (in_array($column->get('type'), ['char', 'string', 'tinyText', 'text', 'mediumText', 'longText', 'enum', 'set'])) { + if ($this->changesToATextColumn($column)) { $attributes['collation'] = $existing['collation']; } - return array_filter($attributes, fn ($value) => !is_null($value)); + return $attributes; } /** diff --git a/src/Database/Schema/Grammars/PostgresGrammar.php b/src/Database/Schema/Grammars/PostgresGrammar.php index ab4851e33..532095fa5 100755 --- a/src/Database/Schema/Grammars/PostgresGrammar.php +++ b/src/Database/Schema/Grammars/PostgresGrammar.php @@ -9,6 +9,8 @@ class PostgresGrammar extends BasePostgresGrammar { + use Concerns\KeepsExistingColumnAttributes; + /** * Compile a change column command into a series of SQL statements. * @@ -28,30 +30,12 @@ public function compileChange(Blueprint $blueprint, Fluent $command) /** @var \Illuminate\Database\Schema\ColumnDefinition $column */ $column = $command->get('column'); - $existing = collect($this->connection->getSchemaBuilder()->getColumns($blueprint->getTable())) - ->firstWhere('name', $column->get('name')); - - if ($existing) { - $attributes = $column->getAttributes(); - - foreach ($this->getKeptColumnAttributes($existing, $column) as $attribute => $value) { - if (!array_key_exists($attribute, $attributes)) { - $column[$attribute] = $value; - } - } - } + $this->keepExistingColumnAttributes($blueprint, $column); return parent::compileChange($blueprint, $command); } - /** - * Get the attributes of an existing column, as reported by the schema builder, that a change keeps - * when the new definition doesn't set them. - * - * @param array|\ArrayAccess $existing - * @param \Illuminate\Support\Fluent $column - * @return array - */ + /** @inheritDoc */ protected function getKeptColumnAttributes($existing, Fluent $column): array { $attributes = [ @@ -61,11 +45,11 @@ protected function getKeptColumnAttributes($existing, Fluent $column): array 'comment' => $existing['comment'], ]; - if (in_array($column->get('type'), ['char', 'string', 'tinyText', 'text', 'mediumText', 'longText', 'enum', 'set'])) { + if ($this->changesToATextColumn($column)) { $attributes['collation'] = $existing['collation']; } - return array_filter($attributes, fn ($value) => !is_null($value)); + return $attributes; } /** diff --git a/src/Database/Schema/Grammars/SqlServerGrammar.php b/src/Database/Schema/Grammars/SqlServerGrammar.php index 2495ccd90..50f92f888 100755 --- a/src/Database/Schema/Grammars/SqlServerGrammar.php +++ b/src/Database/Schema/Grammars/SqlServerGrammar.php @@ -9,6 +9,8 @@ class SqlServerGrammar extends BaseSqlServerGrammar { + use Concerns\KeepsExistingColumnAttributes; + /** * Compile a change column command into a series of SQL statements. * @@ -28,30 +30,12 @@ public function compileChange(Blueprint $blueprint, Fluent $command) /** @var \Illuminate\Database\Schema\ColumnDefinition $column */ $column = $command->get('column'); - $existing = collect($this->connection->getSchemaBuilder()->getColumns($blueprint->getTable())) - ->firstWhere('name', $column->get('name')); - - if ($existing) { - $attributes = $column->getAttributes(); - - foreach ($this->getKeptColumnAttributes($existing, $column) as $attribute => $value) { - if (!array_key_exists($attribute, $attributes)) { - $column[$attribute] = $value; - } - } - } + $this->keepExistingColumnAttributes($blueprint, $column); return parent::compileChange($blueprint, $command); } - /** - * Get the attributes of an existing column, as reported by the schema builder, that a change keeps - * when the new definition doesn't set them. - * - * @param array|\ArrayAccess $existing - * @param \Illuminate\Support\Fluent $column - * @return array - */ + /** @inheritDoc */ protected function getKeptColumnAttributes($existing, Fluent $column): array { $attributes = [ @@ -60,11 +44,11 @@ protected function getKeptColumnAttributes($existing, Fluent $column): array 'default' => is_null($existing['default']) ? null : new Expression($existing['default']), ]; - if (in_array($column->get('type'), ['char', 'string', 'tinyText', 'text', 'mediumText', 'longText', 'enum', 'set'])) { + if ($this->changesToATextColumn($column)) { $attributes['collation'] = $existing['collation']; } - return array_filter($attributes, fn ($value) => !is_null($value)); + return $attributes; } /**