From 3d2f9958cff523e2f3c5722204d1b7fa221d46ef Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Thu, 4 Jul 2024 17:12:27 +0530 Subject: [PATCH 1/5] Initial commit to create PR --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 1cd2090b..3bb79307 100644 --- a/README.md +++ b/README.md @@ -568,3 +568,4 @@ Professional support, consulting as well as software development services are av https://www.cebe.cc/en/contact Development of this library is sponsored by [cebe.:cloud: "Your Professional Deployment Platform"](https://cebe.cloud). + From dbcf4a98471a8093436272951672cd3727a51bc6 Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Thu, 4 Jul 2024 19:16:42 +0530 Subject: [PATCH 2/5] WIP --- README.md | 3 +-- src/lib/ColumnToCode.php | 5 ++++- src/lib/migrations/PostgresMigrationBuilder.php | 1 + 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 3bb79307..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: @@ -568,4 +568,3 @@ Professional support, consulting as well as software development services are av https://www.cebe.cc/en/contact Development of this library is sponsored by [cebe.:cloud: "Your Professional Deployment Platform"](https://cebe.cloud). - diff --git a/src/lib/ColumnToCode.php b/src/lib/ColumnToCode.php index e9a570bc..2e686201 100644 --- a/src/lib/ColumnToCode.php +++ b/src/lib/ColumnToCode.php @@ -345,7 +345,9 @@ private function resolve():void $fluentSize = $this->column->size ? '(' . $this->column->size . ')' : '()'; $rawSize = $this->column->size ? '(' . $this->column->size . ')' : ''; $this->rawParts['nullable'] = $this->column->allowNull ? 'NULL' : 'NOT NULL'; - $this->fluentParts['nullable'] = $this->column->allowNull === true ? 'null()' : 'notNull()'; +// if (!ApiGenerator::isPostgres()) { + $this->fluentParts['nullable'] = $this->column->allowNull === true ? 'null()' : 'notNull()'; +// } if (array_key_exists($dbType, self::INT_TYPE_MAP)) { $this->fluentParts['type'] = self::INT_TYPE_MAP[$dbType] . $fluentSize; $this->rawParts['type'] = @@ -384,6 +386,7 @@ private function getIsBuiltinType($type, $dbType) if ($this->isEnum()) { return false; } + if ($this->fromDb === true) { return isset( (new ColumnSchemaBuilder(''))->categoryMap[$type] diff --git a/src/lib/migrations/PostgresMigrationBuilder.php b/src/lib/migrations/PostgresMigrationBuilder.php index b8c9324d..953c4305 100644 --- a/src/lib/migrations/PostgresMigrationBuilder.php +++ b/src/lib/migrations/PostgresMigrationBuilder.php @@ -72,6 +72,7 @@ protected function buildColumnChanges(ColumnSchema $current, ColumnSchema $desir $this->migration->addDownCode($this->recordBuilder->alterColumnTypeFromDb($tableName, $current, $addUsing)); } if (in_array('allowNull', $changed, true)) { +// TODO if last up code contains `null()` string then do execute below statement, same for notNull() and same in down code if ($desired->allowNull === true) { $this->migration->addUpCode($this->recordBuilder->dropColumnNotNull($tableName, $desired)); $this->migration->addDownCode($this->recordBuilder->setColumnNotNull($tableName, $current), true); From be26ed5ca72e56a9ae2d518233305859ca092b32 Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Fri, 5 Jul 2024 16:26:23 +0530 Subject: [PATCH 3/5] Fix style --- src/lib/ColumnToCode.php | 2 +- src/lib/migrations/PostgresMigrationBuilder.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/ColumnToCode.php b/src/lib/ColumnToCode.php index 2e686201..d602a24b 100644 --- a/src/lib/ColumnToCode.php +++ b/src/lib/ColumnToCode.php @@ -346,7 +346,7 @@ private function resolve():void $rawSize = $this->column->size ? '(' . $this->column->size . ')' : ''; $this->rawParts['nullable'] = $this->column->allowNull ? 'NULL' : 'NOT NULL'; // if (!ApiGenerator::isPostgres()) { - $this->fluentParts['nullable'] = $this->column->allowNull === true ? 'null()' : 'notNull()'; + $this->fluentParts['nullable'] = $this->column->allowNull === true ? 'null()' : 'notNull()'; // } if (array_key_exists($dbType, self::INT_TYPE_MAP)) { $this->fluentParts['type'] = self::INT_TYPE_MAP[$dbType] . $fluentSize; diff --git a/src/lib/migrations/PostgresMigrationBuilder.php b/src/lib/migrations/PostgresMigrationBuilder.php index 953c4305..6f487812 100644 --- a/src/lib/migrations/PostgresMigrationBuilder.php +++ b/src/lib/migrations/PostgresMigrationBuilder.php @@ -72,7 +72,7 @@ protected function buildColumnChanges(ColumnSchema $current, ColumnSchema $desir $this->migration->addDownCode($this->recordBuilder->alterColumnTypeFromDb($tableName, $current, $addUsing)); } if (in_array('allowNull', $changed, true)) { -// TODO if last up code contains `null()` string then do execute below statement, same for notNull() and same in down code + // TODO if last up code contains `null()` string then do execute below statement, same for notNull() and same in down code if ($desired->allowNull === true) { $this->migration->addUpCode($this->recordBuilder->dropColumnNotNull($tableName, $desired)); $this->migration->addDownCode($this->recordBuilder->setColumnNotNull($tableName, $current), true); From 7897d0dc672986008c026d299c8e181377600498 Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Sun, 7 Jul 2024 20:15:27 +0530 Subject: [PATCH 4/5] Fix this issue --- src/lib/ColumnToCode.php | 4 ++-- src/lib/migrations/MigrationRecordBuilder.php | 10 ++++++++++ src/lib/migrations/PostgresMigrationBuilder.php | 14 +++++++++----- .../m200000_000000_change_table_fruits.php | 2 -- .../m200000_000001_change_table_editcolumns.php | 4 ++-- .../m200000_000001_create_table_editcolumns.php | 2 +- .../x_db_type/fresh/pgsql/x_db_type_pgsql.yaml | 1 + .../m200000_000001_create_table_editcolumns.php | 2 +- .../app/models/pgsqlmodel/base/Editcolumn.php | 2 +- 9 files changed, 27 insertions(+), 14 deletions(-) diff --git a/src/lib/ColumnToCode.php b/src/lib/ColumnToCode.php index d602a24b..67fcc30d 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 @@ -468,7 +468,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 6f487812..3ea1a28d 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) + ) { // TODO if last up code contains `null()` string then do execute below statement, same for notNull() and same in down code if ($desired->allowNull === true) { $this->migration->addUpCode($this->recordBuilder->dropColumnNotNull($tableName, $desired)); @@ -81,6 +85,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) @@ -97,9 +104,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'], From e73d9a0d5e19e7397067859d3daba6a4890f339f Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Sun, 7 Jul 2024 20:18:34 +0530 Subject: [PATCH 5/5] Cleanup --- src/lib/ColumnToCode.php | 2 -- src/lib/migrations/PostgresMigrationBuilder.php | 1 - 2 files changed, 3 deletions(-) diff --git a/src/lib/ColumnToCode.php b/src/lib/ColumnToCode.php index 67fcc30d..b7394388 100644 --- a/src/lib/ColumnToCode.php +++ b/src/lib/ColumnToCode.php @@ -345,9 +345,7 @@ private function resolve():void $fluentSize = $this->column->size ? '(' . $this->column->size . ')' : '()'; $rawSize = $this->column->size ? '(' . $this->column->size . ')' : ''; $this->rawParts['nullable'] = $this->column->allowNull ? 'NULL' : 'NOT NULL'; -// if (!ApiGenerator::isPostgres()) { $this->fluentParts['nullable'] = $this->column->allowNull === true ? 'null()' : 'notNull()'; -// } if (array_key_exists($dbType, self::INT_TYPE_MAP)) { $this->fluentParts['type'] = self::INT_TYPE_MAP[$dbType] . $fluentSize; $this->rawParts['type'] = diff --git a/src/lib/migrations/PostgresMigrationBuilder.php b/src/lib/migrations/PostgresMigrationBuilder.php index 3ea1a28d..956ac9d5 100644 --- a/src/lib/migrations/PostgresMigrationBuilder.php +++ b/src/lib/migrations/PostgresMigrationBuilder.php @@ -76,7 +76,6 @@ protected function buildColumnChanges(ColumnSchema $current, ColumnSchema $desir if (in_array('allowNull', $changed, true) && ($forUp === false || $forDown === false) ) { - // TODO if last up code contains `null()` string then do execute below statement, same for notNull() and same in down code if ($desired->allowNull === true) { $this->migration->addUpCode($this->recordBuilder->dropColumnNotNull($tableName, $desired)); $this->migration->addDownCode($this->recordBuilder->setColumnNotNull($tableName, $current), true);