diff --git a/README.md b/README.md index 1cd2090b..d3756483 100644 --- a/README.md +++ b/README.md @@ -390,7 +390,7 @@ User: `NOT NULL` in DB migrations is determined by `nullable` and `required` properties of the OpenAPI schema. e.g. attribute = 'my_property'. -- If you define attribute neither "required" nor via "nullable", then it is by default `NULL`: +- If you define attribute neither "required" nor via "nullable", then it is by default `NULL` ([opposite of OpenAPI spec](https://swagger.io/specification/v3/?sbsearch=nullable)): ```yaml ExampleSchema: diff --git a/src/lib/ColumnToCode.php b/src/lib/ColumnToCode.php index e9a570bc..b7394388 100644 --- a/src/lib/ColumnToCode.php +++ b/src/lib/ColumnToCode.php @@ -69,7 +69,7 @@ class ColumnToCode * @var bool * Built In Type means the \cebe\yii2openapi\lib\items\Attribute::$type or \cebe\yii2openapi\lib\items\Attribute::$dbType is in list of Yii abstract data type list or not. And if is found we can use \yii\db\SchemaBuilderTrait methods to build migration instead of putting raw SQL */ - private $isBuiltinType = false; + public $isBuiltinType = false; /** * @var bool @@ -384,6 +384,7 @@ private function getIsBuiltinType($type, $dbType) if ($this->isEnum()) { return false; } + if ($this->fromDb === true) { return isset( (new ColumnSchemaBuilder(''))->categoryMap[$type] @@ -465,7 +466,7 @@ private function resolveDefaultValue():void private function isDefaultAllowed():bool { - // default expression with parenthases is allowed + // default expression with parentheses is allowed if ($this->column->defaultValue instanceof \yii\db\Expression) { return true; } diff --git a/src/lib/migrations/MigrationRecordBuilder.php b/src/lib/migrations/MigrationRecordBuilder.php index 98ee03b8..97481e9e 100644 --- a/src/lib/migrations/MigrationRecordBuilder.php +++ b/src/lib/migrations/MigrationRecordBuilder.php @@ -51,6 +51,12 @@ final class MigrationRecordBuilder */ private $dbSchema; + /** + * @var bool + * Only required for PgSQL alter column for set null/set default related statement + */ + public $isBuiltInType = false; + public function __construct(Schema $dbSchema) { $this->dbSchema = $dbSchema; @@ -134,6 +140,7 @@ public function alterColumnType(string $tableAlias, ColumnSchema $column, bool $ { if (property_exists($column, 'xDbType') && is_string($column->xDbType) && !empty($column->xDbType)) { $converter = $this->columnToCode($tableAlias, $column, false, false, true, true); + $this->isBuiltInType = $converter->isBuiltinType; return sprintf( ApiGenerator::isPostgres() ? self::ALTER_COLUMN_RAW_PGSQL : self::ALTER_COLUMN_RAW, $tableAlias, @@ -142,6 +149,7 @@ public function alterColumnType(string $tableAlias, ColumnSchema $column, bool $ ); } $converter = $this->columnToCode($tableAlias, $column, false); + $this->isBuiltInType = $converter->isBuiltinType; return sprintf(self::ALTER_COLUMN, $tableAlias, $column->name, $converter->getAlterExpression($addUsing)); } @@ -153,6 +161,7 @@ public function alterColumnTypeFromDb(string $tableAlias, ColumnSchema $column, { if (property_exists($column, 'xDbType') && is_string($column->xDbType) && !empty($column->xDbType)) { $converter = $this->columnToCode($tableAlias, $column, true, false, true, true); + $this->isBuiltInType = $converter->isBuiltinType; return sprintf( ApiGenerator::isPostgres() ? self::ALTER_COLUMN_RAW_PGSQL : self::ALTER_COLUMN_RAW, $tableAlias, @@ -161,6 +170,7 @@ public function alterColumnTypeFromDb(string $tableAlias, ColumnSchema $column, ); } $converter = $this->columnToCode($tableAlias, $column, true); + $this->isBuiltInType = $converter->isBuiltinType; return sprintf(self::ALTER_COLUMN, $tableAlias, $column->name, $converter->getAlterExpression($addUsing)); } diff --git a/src/lib/migrations/PostgresMigrationBuilder.php b/src/lib/migrations/PostgresMigrationBuilder.php index b8c9324d..956ac9d5 100644 --- a/src/lib/migrations/PostgresMigrationBuilder.php +++ b/src/lib/migrations/PostgresMigrationBuilder.php @@ -62,16 +62,20 @@ protected function buildColumnChanges(ColumnSchema $current, ColumnSchema $desir // This action require several steps and can't be applied during single transaction return; } - + $forUp = $forDown = false; if (!empty(array_intersect(['type', 'size' , 'dbType', 'phpType' , 'precision', 'scale', 'unsigned' ], $changed))) { $addUsing = $this->isNeedUsingExpression($current->dbType, $desired->dbType); $this->migration->addUpCode($this->recordBuilder->alterColumnType($tableName, $desired, $addUsing)); + $forUp = $this->recordBuilder->isBuiltInType; $this->migration->addDownCode($this->recordBuilder->alterColumnTypeFromDb($tableName, $current, $addUsing)); + $forDown = $this->recordBuilder->isBuiltInType; } - if (in_array('allowNull', $changed, true)) { + if (in_array('allowNull', $changed, true) + && ($forUp === false || $forDown === false) + ) { if ($desired->allowNull === true) { $this->migration->addUpCode($this->recordBuilder->dropColumnNotNull($tableName, $desired)); $this->migration->addDownCode($this->recordBuilder->setColumnNotNull($tableName, $current), true); @@ -80,6 +84,9 @@ protected function buildColumnChanges(ColumnSchema $current, ColumnSchema $desir $this->migration->addDownCode($this->recordBuilder->dropColumnNotNull($tableName, $current), true); } } + + $this->recordBuilder->isBuiltInType = $forUp = $forDown = false; + if (in_array('defaultValue', $changed, true)) { $upCode = $desired->defaultValue === null ? $this->recordBuilder->dropColumnDefault($tableName, $desired) @@ -96,9 +103,6 @@ protected function buildColumnChanges(ColumnSchema $current, ColumnSchema $desir } if ($isChangeFromEnum) { $this->migration->addUpCode($this->recordBuilder->dropEnum($tableName, $current->name)); - } - - if ($isChangeFromEnum) { $this->migration ->addDownCode($this->recordBuilder->createEnum($tableName, $current->name, $current->enumValues)); } diff --git a/tests/specs/issue_fix/wrong_migration_for_pgsql_is_generated_for_string_varchar_datatype_149/app/migrations_pgsql_db/m200000_000000_change_table_fruits.php b/tests/specs/issue_fix/wrong_migration_for_pgsql_is_generated_for_string_varchar_datatype_149/app/migrations_pgsql_db/m200000_000000_change_table_fruits.php index 574730fe..f89f7f63 100644 --- a/tests/specs/issue_fix/wrong_migration_for_pgsql_is_generated_for_string_varchar_datatype_149/app/migrations_pgsql_db/m200000_000000_change_table_fruits.php +++ b/tests/specs/issue_fix/wrong_migration_for_pgsql_is_generated_for_string_varchar_datatype_149/app/migrations_pgsql_db/m200000_000000_change_table_fruits.php @@ -8,12 +8,10 @@ class m200000_000000_change_table_fruits extends \yii\db\Migration public function safeUp() { $this->alterColumn('{{%fruits}}', 'name', $this->string(151)->notNull()); - $this->alterColumn('{{%fruits}}', 'name', "SET NOT NULL"); } public function safeDown() { $this->alterColumn('{{%fruits}}', 'name', $this->string(150)->null()); - $this->alterColumn('{{%fruits}}', 'name', "DROP NOT NULL"); } } diff --git a/tests/specs/x_db_type/edit_column/pgsql/app/migrations_pgsql_db/m200000_000001_change_table_editcolumns.php b/tests/specs/x_db_type/edit_column/pgsql/app/migrations_pgsql_db/m200000_000001_change_table_editcolumns.php index 5ad21a0a..f1890895 100644 --- a/tests/specs/x_db_type/edit_column/pgsql/app/migrations_pgsql_db/m200000_000001_change_table_editcolumns.php +++ b/tests/specs/x_db_type/edit_column/pgsql/app/migrations_pgsql_db/m200000_000001_change_table_editcolumns.php @@ -14,7 +14,6 @@ public function safeUp() $this->db->createCommand('ALTER TABLE {{%editcolumns}} ALTER COLUMN "name" SET DATA TYPE varchar(254)')->execute(); $this->alterColumn('{{%editcolumns}}', 'name', "SET DEFAULT 'Horse-2'"); $this->alterColumn('{{%editcolumns}}', 'string_col', 'text NULL USING "string_col"::text'); - $this->alterColumn('{{%editcolumns}}', 'string_col', "DROP NOT NULL"); $this->db->createCommand('ALTER TABLE {{%editcolumns}} ALTER COLUMN "dec_col" SET DATA TYPE decimal(12,2) USING "dec_col"::decimal(12,2)')->execute(); $this->alterColumn('{{%editcolumns}}', 'dec_col', "SET DEFAULT 3.14"); $this->alterColumn('{{%editcolumns}}', 'str_col_def', "SET NOT NULL"); @@ -25,6 +24,7 @@ public function safeUp() $this->alterColumn('{{%editcolumns}}', 'json_col_2', "SET NOT NULL"); $this->alterColumn('{{%editcolumns}}', 'json_col_2', "SET DEFAULT '[]'"); $this->db->createCommand('ALTER TABLE {{%editcolumns}} ALTER COLUMN "numeric_col" SET DATA TYPE double precision USING "numeric_col"::double precision')->execute(); + $this->alterColumn('{{%editcolumns}}', 'numeric_col', "SET NOT NULL"); } public function safeDown() @@ -39,7 +39,6 @@ public function safeDown() $this->dropColumn('{{%editcolumns}}', 'json_col_def_n'); $this->dropColumn('{{%editcolumns}}', 'first_name'); $this->alterColumn('{{%editcolumns}}', 'name', "SET DEFAULT 'Horse'"); - $this->alterColumn('{{%editcolumns}}', 'string_col', "SET NOT NULL"); $this->alterColumn('{{%editcolumns}}', 'dec_col', "DROP DEFAULT"); $this->alterColumn('{{%editcolumns}}', 'str_col_def', "DROP NOT NULL"); $this->alterColumn('{{%editcolumns}}', 'str_col_def', "SET DEFAULT 'hi there'"); @@ -47,5 +46,6 @@ public function safeDown() $this->alterColumn('{{%editcolumns}}', 'json_col', "DROP DEFAULT"); $this->alterColumn('{{%editcolumns}}', 'json_col_2', "DROP NOT NULL"); $this->alterColumn('{{%editcolumns}}', 'json_col_2', "DROP DEFAULT"); + $this->alterColumn('{{%editcolumns}}', 'numeric_col', "DROP NOT NULL"); } } diff --git a/tests/specs/x_db_type/fresh/pgsql/app/migrations_pgsql_db/m200000_000001_create_table_editcolumns.php b/tests/specs/x_db_type/fresh/pgsql/app/migrations_pgsql_db/m200000_000001_create_table_editcolumns.php index b8f286b1..fcf648ed 100644 --- a/tests/specs/x_db_type/fresh/pgsql/app/migrations_pgsql_db/m200000_000001_create_table_editcolumns.php +++ b/tests/specs/x_db_type/fresh/pgsql/app/migrations_pgsql_db/m200000_000001_create_table_editcolumns.php @@ -17,7 +17,7 @@ public function safeUp() 3 => '"str_col_def" varchar NOT NULL', 4 => '"json_col" text NOT NULL DEFAULT \'fox jumps over dog\'', 5 => '"json_col_2" jsonb NOT NULL DEFAULT \'[]\'', - 6 => '"numeric_col" double precision NULL DEFAULT NULL', + 6 => '"numeric_col" double precision NOT NULL', 7 => '"json_col_def_n" json NOT NULL DEFAULT \'[]\'', 8 => '"json_col_def_n_2" json NOT NULL DEFAULT \'[]\'', 9 => '"text_col_array" text[] NULL DEFAULT NULL', diff --git a/tests/specs/x_db_type/fresh/pgsql/x_db_type_pgsql.yaml b/tests/specs/x_db_type/fresh/pgsql/x_db_type_pgsql.yaml index ff629112..15233c63 100644 --- a/tests/specs/x_db_type/fresh/pgsql/x_db_type_pgsql.yaml +++ b/tests/specs/x_db_type/fresh/pgsql/x_db_type_pgsql.yaml @@ -516,6 +516,7 @@ components: numeric_col: type: string x-db-type: double precision + nullable: false json_col_def_n: type: string x-db-type: json diff --git a/tests/specs/x_db_type/new_column/pgsql/app/migrations_pgsql_db/m200000_000001_create_table_editcolumns.php b/tests/specs/x_db_type/new_column/pgsql/app/migrations_pgsql_db/m200000_000001_create_table_editcolumns.php index b8f286b1..fcf648ed 100644 --- a/tests/specs/x_db_type/new_column/pgsql/app/migrations_pgsql_db/m200000_000001_create_table_editcolumns.php +++ b/tests/specs/x_db_type/new_column/pgsql/app/migrations_pgsql_db/m200000_000001_create_table_editcolumns.php @@ -17,7 +17,7 @@ public function safeUp() 3 => '"str_col_def" varchar NOT NULL', 4 => '"json_col" text NOT NULL DEFAULT \'fox jumps over dog\'', 5 => '"json_col_2" jsonb NOT NULL DEFAULT \'[]\'', - 6 => '"numeric_col" double precision NULL DEFAULT NULL', + 6 => '"numeric_col" double precision NOT NULL', 7 => '"json_col_def_n" json NOT NULL DEFAULT \'[]\'', 8 => '"json_col_def_n_2" json NOT NULL DEFAULT \'[]\'', 9 => '"text_col_array" text[] NULL DEFAULT NULL', diff --git a/tests/specs/x_db_type/rules_and_more/pgsql/app/models/pgsqlmodel/base/Editcolumn.php b/tests/specs/x_db_type/rules_and_more/pgsql/app/models/pgsqlmodel/base/Editcolumn.php index 730bff5a..0add2f71 100644 --- a/tests/specs/x_db_type/rules_and_more/pgsql/app/models/pgsqlmodel/base/Editcolumn.php +++ b/tests/specs/x_db_type/rules_and_more/pgsql/app/models/pgsqlmodel/base/Editcolumn.php @@ -31,7 +31,7 @@ public function rules() { return [ 'trim' => [['name', 'tag', 'first_name', 'string_col', 'str_col_def', 'json_col'], 'trim'], - 'required' => [['name', 'str_col_def', 'json_col', 'json_col_2'], 'required'], + 'required' => [['name', 'str_col_def', 'json_col', 'json_col_2', 'numeric_col'], 'required'], 'name_string' => [['name'], 'string', 'max' => 254], 'name_default' => [['name'], 'default', 'value' => 'Horse-2'], 'tag_string' => [['tag'], 'string'],