From de8fc633d06998c1d63e5ee3c4bc083bb43c6675 Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Thu, 19 Jun 2025 19:11:27 +0530 Subject: [PATCH 1/4] Create PR --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 1ff3e2b4..1466f904 100644 --- a/README.md +++ b/README.md @@ -997,3 +997,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 43e752390034765f3d8bc002cfb9d788ac5d161e Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Thu, 19 Jun 2025 20:37:29 +0530 Subject: [PATCH 2/4] Fix this issue and failing tests --- README.md | 1 - .../migrations/PostgresMigrationBuilder.php | 7 +++-- ...m200000_000002_create_table_blog_posts.php | 2 -- ...0000_000004_create_table_post_comments.php | 2 -- .../m200000_000001_create_table_fruits.php | 1 - .../index.php | 14 ++++++++++ .../index.yml | 27 +++++++++++++++++++ .../m200000_000000_create_table_fruits.php | 22 +++++++++++++++ .../m200000_000001_create_table_b123s.php | 1 - .../m200000_000002_create_table_a123s.php | 1 - .../m200000_000003_create_table_accounts.php | 1 - .../m200000_000005_create_table_domains.php | 2 -- .../m200000_000006_create_table_e123s.php | 1 - .../m200000_000007_create_table_routings.php | 3 --- .../m200000_000001_create_table_postxes.php | 4 --- tests/unit/issues/Issue62Test.php | 26 ++++++++++++++++++ 16 files changed, 94 insertions(+), 21 deletions(-) create mode 100644 tests/specs/issue_fix/62_unnecessary_sql_statement_for_comment_on_column_in_pgsql/index.php create mode 100644 tests/specs/issue_fix/62_unnecessary_sql_statement_for_comment_on_column_in_pgsql/index.yml create mode 100644 tests/specs/issue_fix/62_unnecessary_sql_statement_for_comment_on_column_in_pgsql/pgsql/migrations_pgsql_db/m200000_000000_create_table_fruits.php create mode 100644 tests/unit/issues/Issue62Test.php diff --git a/README.md b/README.md index 1466f904..1ff3e2b4 100644 --- a/README.md +++ b/README.md @@ -997,4 +997,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/migrations/PostgresMigrationBuilder.php b/src/lib/migrations/PostgresMigrationBuilder.php index 4f2379cf..ce507fa2 100644 --- a/src/lib/migrations/PostgresMigrationBuilder.php +++ b/src/lib/migrations/PostgresMigrationBuilder.php @@ -286,9 +286,12 @@ public function handleCommentsMigration() { $tableAlias = $this->model->getTableAlias(); foreach ($this->newColumns as $column) { + /** @var ColumnSchema $column */ if ($column->comment) { - $this->migration - ->addUpCode($this->recordBuilder->addCommentOnColumn($tableAlias, $column->name, $column->comment)); + if (!empty($column->xDbType) && is_string($column->xDbType)) { # see \cebe\yii2openapi\lib\migrations\MigrationRecordBuilder::createTable() + $this->migration + ->addUpCode($this->recordBuilder->addCommentOnColumn($tableAlias, $column->name, $column->comment)); + } } } } diff --git a/tests/specs/blog/migrations_pgsql_db/m200000_000002_create_table_blog_posts.php b/tests/specs/blog/migrations_pgsql_db/m200000_000002_create_table_blog_posts.php index 9dbe55cf..a7266997 100644 --- a/tests/specs/blog/migrations_pgsql_db/m200000_000002_create_table_blog_posts.php +++ b/tests/specs/blog/migrations_pgsql_db/m200000_000002_create_table_blog_posts.php @@ -21,8 +21,6 @@ public function safeUp() $this->createIndex('blog_posts_slug_key', '{{%blog_posts}}', 'slug', true); $this->addForeignKey('fk_blog_posts_category_id_categories_id', '{{%blog_posts}}', 'category_id', '{{%categories}}', 'id'); $this->addForeignKey('fk_blog_posts_created_by_id_users_id', '{{%blog_posts}}', 'created_by_id', '{{%users}}', 'id'); - $this->addCommentOnColumn('{{%blog_posts}}', 'category_id', 'Category of posts'); - $this->addCommentOnColumn('{{%blog_posts}}', 'created_by_id', 'The User'); } public function safeDown() diff --git a/tests/specs/blog/migrations_pgsql_db/m200000_000004_create_table_post_comments.php b/tests/specs/blog/migrations_pgsql_db/m200000_000004_create_table_post_comments.php index 2d924e52..1229677b 100644 --- a/tests/specs/blog/migrations_pgsql_db/m200000_000004_create_table_post_comments.php +++ b/tests/specs/blog/migrations_pgsql_db/m200000_000004_create_table_post_comments.php @@ -17,8 +17,6 @@ public function safeUp() ]); $this->addForeignKey('fk_post_comments_post_id_blog_posts_uid', '{{%post_comments}}', 'post_id', '{{%blog_posts}}', 'uid'); $this->addForeignKey('fk_post_comments_author_id_users_id', '{{%post_comments}}', 'author_id', '{{%users}}', 'id'); - $this->addCommentOnColumn('{{%post_comments}}', 'post_id', 'A blog post (uid used as pk for test purposes)'); - $this->addCommentOnColumn('{{%post_comments}}', 'author_id', 'The User'); } public function safeDown() diff --git a/tests/specs/issue_fix/60_description_of_a_property_in_spec_must_correspond_to_db_table_column_comment/pgsql/migrations_pgsql_db/m200000_000001_create_table_fruits.php b/tests/specs/issue_fix/60_description_of_a_property_in_spec_must_correspond_to_db_table_column_comment/pgsql/migrations_pgsql_db/m200000_000001_create_table_fruits.php index 21467feb..5607fa87 100644 --- a/tests/specs/issue_fix/60_description_of_a_property_in_spec_must_correspond_to_db_table_column_comment/pgsql/migrations_pgsql_db/m200000_000001_create_table_fruits.php +++ b/tests/specs/issue_fix/60_description_of_a_property_in_spec_must_correspond_to_db_table_column_comment/pgsql/migrations_pgsql_db/m200000_000001_create_table_fruits.php @@ -12,7 +12,6 @@ public function safeUp() 'name' => $this->text()->null()->defaultValue(null)->comment('desc with \' quote'), 0 => '"description" double precision NULL DEFAULT NULL', ]); - $this->addCommentOnColumn('{{%fruits}}', 'name', 'desc with \' quote'); $this->addCommentOnColumn('{{%fruits}}', 'description', 'desc \' 2'); } diff --git a/tests/specs/issue_fix/62_unnecessary_sql_statement_for_comment_on_column_in_pgsql/index.php b/tests/specs/issue_fix/62_unnecessary_sql_statement_for_comment_on_column_in_pgsql/index.php new file mode 100644 index 00000000..362d2641 --- /dev/null +++ b/tests/specs/issue_fix/62_unnecessary_sql_statement_for_comment_on_column_in_pgsql/index.php @@ -0,0 +1,14 @@ + '@specs/issue_fix/62_unnecessary_sql_statement_for_comment_on_column_in_pgsql/index.yml', + 'generateUrls' => false, + 'generateModels' => false, + 'excludeModels' => [ + 'Error', + ], + 'generateControllers' => false, + 'generateMigrations' => true, + 'generateModelFaker' => false, // `generateModels` must be `true` in order to use `generateModelFaker` as `true` +]; + diff --git a/tests/specs/issue_fix/62_unnecessary_sql_statement_for_comment_on_column_in_pgsql/index.yml b/tests/specs/issue_fix/62_unnecessary_sql_statement_for_comment_on_column_in_pgsql/index.yml new file mode 100644 index 00000000..cc68ab56 --- /dev/null +++ b/tests/specs/issue_fix/62_unnecessary_sql_statement_for_comment_on_column_in_pgsql/index.yml @@ -0,0 +1,27 @@ +openapi: 3.0.3 +x-description-is-comment: true +info: + title: 'Description of a property in spec must correspond to DB TABLE COLUMN COMMENT #60' + version: 1.0.0 + +components: + schemas: + Fruit: + type: object + properties: + id: + type: integer + name: + type: string + description: desc with ' quote + description: + type: number + x-db-type: double precision + description: desc ' 2 + +paths: + '/': + get: + responses: + '200': + description: OK diff --git a/tests/specs/issue_fix/62_unnecessary_sql_statement_for_comment_on_column_in_pgsql/pgsql/migrations_pgsql_db/m200000_000000_create_table_fruits.php b/tests/specs/issue_fix/62_unnecessary_sql_statement_for_comment_on_column_in_pgsql/pgsql/migrations_pgsql_db/m200000_000000_create_table_fruits.php new file mode 100644 index 00000000..60d448af --- /dev/null +++ b/tests/specs/issue_fix/62_unnecessary_sql_statement_for_comment_on_column_in_pgsql/pgsql/migrations_pgsql_db/m200000_000000_create_table_fruits.php @@ -0,0 +1,22 @@ +createTable('{{%fruits}}', [ + 'id' => $this->primaryKey(), + 'name' => $this->text()->null()->defaultValue(null)->comment('desc with \' quote'), + 0 => '"description" double precision NULL DEFAULT NULL', + ]); + $this->addCommentOnColumn('{{%fruits}}', 'description', 'desc \' 2'); + } + + public function safeDown() + { + $this->dropTable('{{%fruits}}'); + } +} diff --git a/tests/specs/relations_in_faker/app/migrations_pgsql_db/m200000_000001_create_table_b123s.php b/tests/specs/relations_in_faker/app/migrations_pgsql_db/m200000_000001_create_table_b123s.php index 688d4e58..7fe423f5 100644 --- a/tests/specs/relations_in_faker/app/migrations_pgsql_db/m200000_000001_create_table_b123s.php +++ b/tests/specs/relations_in_faker/app/migrations_pgsql_db/m200000_000001_create_table_b123s.php @@ -13,7 +13,6 @@ public function safeUp() 'c123_id' => $this->integer()->null()->defaultValue(null)->comment('desc'), ]); $this->addForeignKey('fk_b123s_c123_id_c123s_id', '{{%b123s}}', 'c123_id', '{{%c123s}}', 'id'); - $this->addCommentOnColumn('{{%b123s}}', 'c123_id', 'desc'); } public function safeDown() diff --git a/tests/specs/relations_in_faker/app/migrations_pgsql_db/m200000_000002_create_table_a123s.php b/tests/specs/relations_in_faker/app/migrations_pgsql_db/m200000_000002_create_table_a123s.php index f93343c6..4b2aad61 100644 --- a/tests/specs/relations_in_faker/app/migrations_pgsql_db/m200000_000002_create_table_a123s.php +++ b/tests/specs/relations_in_faker/app/migrations_pgsql_db/m200000_000002_create_table_a123s.php @@ -13,7 +13,6 @@ public function safeUp() 'b123_id' => $this->integer()->null()->defaultValue(null)->comment('desc'), ]); $this->addForeignKey('fk_a123s_b123_id_b123s_id', '{{%a123s}}', 'b123_id', '{{%b123s}}', 'id'); - $this->addCommentOnColumn('{{%a123s}}', 'b123_id', 'desc'); } public function safeDown() diff --git a/tests/specs/relations_in_faker/app/migrations_pgsql_db/m200000_000003_create_table_accounts.php b/tests/specs/relations_in_faker/app/migrations_pgsql_db/m200000_000003_create_table_accounts.php index 271f8a7b..843678d6 100644 --- a/tests/specs/relations_in_faker/app/migrations_pgsql_db/m200000_000003_create_table_accounts.php +++ b/tests/specs/relations_in_faker/app/migrations_pgsql_db/m200000_000003_create_table_accounts.php @@ -11,7 +11,6 @@ public function safeUp() 'id' => $this->primaryKey(), 'name' => $this->string(40)->notNull()->comment('account name'), ]); - $this->addCommentOnColumn('{{%accounts}}', 'name', 'account name'); } public function safeDown() diff --git a/tests/specs/relations_in_faker/app/migrations_pgsql_db/m200000_000005_create_table_domains.php b/tests/specs/relations_in_faker/app/migrations_pgsql_db/m200000_000005_create_table_domains.php index c5a0e7f2..871b473f 100644 --- a/tests/specs/relations_in_faker/app/migrations_pgsql_db/m200000_000005_create_table_domains.php +++ b/tests/specs/relations_in_faker/app/migrations_pgsql_db/m200000_000005_create_table_domains.php @@ -14,8 +14,6 @@ public function safeUp() 0 => '"created_at" timestamp NOT NULL', ]); $this->addForeignKey('fk_domains_account_id_accounts_id', '{{%domains}}', 'account_id', '{{%accounts}}', 'id'); - $this->addCommentOnColumn('{{%domains}}', 'name', 'domain or sub-domain name, in DNS syntax, IDN are converted'); - $this->addCommentOnColumn('{{%domains}}', 'account_id', 'user account'); } public function safeDown() diff --git a/tests/specs/relations_in_faker/app/migrations_pgsql_db/m200000_000006_create_table_e123s.php b/tests/specs/relations_in_faker/app/migrations_pgsql_db/m200000_000006_create_table_e123s.php index 55cb62af..59d643a4 100644 --- a/tests/specs/relations_in_faker/app/migrations_pgsql_db/m200000_000006_create_table_e123s.php +++ b/tests/specs/relations_in_faker/app/migrations_pgsql_db/m200000_000006_create_table_e123s.php @@ -13,7 +13,6 @@ public function safeUp() 'b123_id' => $this->integer()->null()->defaultValue(null)->comment('desc'), ]); $this->addForeignKey('fk_e123s_b123_id_b123s_id', '{{%e123s}}', 'b123_id', '{{%b123s}}', 'id'); - $this->addCommentOnColumn('{{%e123s}}', 'b123_id', 'desc'); } public function safeDown() diff --git a/tests/specs/relations_in_faker/app/migrations_pgsql_db/m200000_000007_create_table_routings.php b/tests/specs/relations_in_faker/app/migrations_pgsql_db/m200000_000007_create_table_routings.php index bd09ae48..f880d830 100644 --- a/tests/specs/relations_in_faker/app/migrations_pgsql_db/m200000_000007_create_table_routings.php +++ b/tests/specs/relations_in_faker/app/migrations_pgsql_db/m200000_000007_create_table_routings.php @@ -21,9 +21,6 @@ public function safeUp() $this->addForeignKey('fk_routings_domain_id_domains_id', '{{%routings}}', 'domain_id', '{{%domains}}', 'id'); $this->addForeignKey('fk_routings_d123_id_d123s_id', '{{%routings}}', 'd123_id', '{{%d123s}}', 'id'); $this->addForeignKey('fk_routings_a123_id_a123s_id', '{{%routings}}', 'a123_id', '{{%a123s}}', 'id'); - $this->addCommentOnColumn('{{%routings}}', 'domain_id', 'domain'); - $this->addCommentOnColumn('{{%routings}}', 'd123_id', 'desc'); - $this->addCommentOnColumn('{{%routings}}', 'a123_id', 'desc'); } public function safeDown() diff --git a/tests/specs/x_on_x_fk_constraint/app/migrations_pgsql_db/m200000_000001_create_table_postxes.php b/tests/specs/x_on_x_fk_constraint/app/migrations_pgsql_db/m200000_000001_create_table_postxes.php index 5593db95..777920de 100644 --- a/tests/specs/x_on_x_fk_constraint/app/migrations_pgsql_db/m200000_000001_create_table_postxes.php +++ b/tests/specs/x_on_x_fk_constraint/app/migrations_pgsql_db/m200000_000001_create_table_postxes.php @@ -19,10 +19,6 @@ public function safeUp() $this->addForeignKey('fk_postxes_user_2_id_userxes_id', '{{%postxes}}', 'user_2_id', '{{%userxes}}', 'id', 'SET NULL', 'CASCADE'); $this->addForeignKey('fk_postxes_user_3_id_userxes_id', '{{%postxes}}', 'user_3_id', '{{%userxes}}', 'id', 'SET NULL'); $this->addForeignKey('fk_postxes_user_4_id_userxes_id', '{{%postxes}}', 'user_4_id', '{{%userxes}}', 'id'); - $this->addCommentOnColumn('{{%postxes}}', 'user_id', 'x on-x (update|delete) foreign key constraint'); - $this->addCommentOnColumn('{{%postxes}}', 'user_2_id', 'x on-x (update|delete) foreign key constraint'); - $this->addCommentOnColumn('{{%postxes}}', 'user_3_id', 'x on-x (update|delete) foreign key constraint'); - $this->addCommentOnColumn('{{%postxes}}', 'user_4_id', 'x on-x (update|delete) foreign key constraint'); } public function safeDown() diff --git a/tests/unit/issues/Issue62Test.php b/tests/unit/issues/Issue62Test.php new file mode 100644 index 00000000..651a2031 --- /dev/null +++ b/tests/unit/issues/Issue62Test.php @@ -0,0 +1,26 @@ +changeDbToPgsql(); + + $testFile = Yii::getAlias("@specs/issue_fix/62_unnecessary_sql_statement_for_comment_on_column_in_pgsql/index.php"); + $this->runGenerator($testFile, 'pgsql'); + $actualFiles = FileHelper::findFiles(Yii::getAlias('@app'), [ + 'recursive' => true, + ]); + $expectedFiles = FileHelper::findFiles(Yii::getAlias("@specs/issue_fix/62_unnecessary_sql_statement_for_comment_on_column_in_pgsql/pgsql"), [ + 'recursive' => true, + ]); + $this->checkFiles($actualFiles, $expectedFiles); + } +} From 7952011069f049edc7658fe90445d1a0db99dc58 Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Fri, 20 Jun 2025 17:55:39 +0530 Subject: [PATCH 3/4] Refactor --- src/lib/migrations/MigrationRecordBuilder.php | 9 +++++++-- src/lib/migrations/PostgresMigrationBuilder.php | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/lib/migrations/MigrationRecordBuilder.php b/src/lib/migrations/MigrationRecordBuilder.php index 14df1f6c..db50b58b 100644 --- a/src/lib/migrations/MigrationRecordBuilder.php +++ b/src/lib/migrations/MigrationRecordBuilder.php @@ -69,7 +69,7 @@ public function __construct(Schema $dbSchema) /** * @param string $tableAlias - * @param array|\yii\db\ColumnSchema $columns + * @param array|ColumnSchema $columns * @return string * @throws \yii\base\InvalidConfigException */ @@ -77,7 +77,7 @@ public function createTable(string $tableAlias, array $columns):string { $codeColumns = []; foreach ($columns as $columnName => $cebeDbColumnSchema) { - if (!empty($cebeDbColumnSchema->xDbType) && is_string($cebeDbColumnSchema->xDbType)) { + if (static::isXDbType($cebeDbColumnSchema)) { $name = static::quote($columnName); $codeColumns[] = $name.' '.$this->columnToCode($tableAlias, $cebeDbColumnSchema, false)->getCode(); } else { @@ -393,4 +393,9 @@ public function renameColumn(string $table, string $fromColumn, string $toColumn { return sprintf(self::RENAME_COLUMN, $table, $fromColumn, $toColumn); } + + public static function isXDbType(ColumnSchema $columnSchema): bool + { + return (!empty($columnSchema->xDbType) && is_string($columnSchema->xDbType)); + } } diff --git a/src/lib/migrations/PostgresMigrationBuilder.php b/src/lib/migrations/PostgresMigrationBuilder.php index ce507fa2..847fb81b 100644 --- a/src/lib/migrations/PostgresMigrationBuilder.php +++ b/src/lib/migrations/PostgresMigrationBuilder.php @@ -288,7 +288,7 @@ public function handleCommentsMigration() foreach ($this->newColumns as $column) { /** @var ColumnSchema $column */ if ($column->comment) { - if (!empty($column->xDbType) && is_string($column->xDbType)) { # see \cebe\yii2openapi\lib\migrations\MigrationRecordBuilder::createTable() + if (MigrationRecordBuilder::isXDbType($column)) { $this->migration ->addUpCode($this->recordBuilder->addCommentOnColumn($tableAlias, $column->name, $column->comment)); } From 9db43da898c9e25e7e7bc3ea76f5c2a823d314fc Mon Sep 17 00:00:00 2001 From: Sohel Ahmed Mesaniya Date: Fri, 20 Jun 2025 18:21:53 +0530 Subject: [PATCH 4/4] Refactor method name --- src/lib/migrations/MigrationRecordBuilder.php | 4 ++-- src/lib/migrations/PostgresMigrationBuilder.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib/migrations/MigrationRecordBuilder.php b/src/lib/migrations/MigrationRecordBuilder.php index db50b58b..8a2940a4 100644 --- a/src/lib/migrations/MigrationRecordBuilder.php +++ b/src/lib/migrations/MigrationRecordBuilder.php @@ -77,7 +77,7 @@ public function createTable(string $tableAlias, array $columns):string { $codeColumns = []; foreach ($columns as $columnName => $cebeDbColumnSchema) { - if (static::isXDbType($cebeDbColumnSchema)) { + if (static::isXDbTypePresent($cebeDbColumnSchema)) { $name = static::quote($columnName); $codeColumns[] = $name.' '.$this->columnToCode($tableAlias, $cebeDbColumnSchema, false)->getCode(); } else { @@ -394,7 +394,7 @@ public function renameColumn(string $table, string $fromColumn, string $toColumn return sprintf(self::RENAME_COLUMN, $table, $fromColumn, $toColumn); } - public static function isXDbType(ColumnSchema $columnSchema): bool + public static function isXDbTypePresent(ColumnSchema $columnSchema): bool { return (!empty($columnSchema->xDbType) && is_string($columnSchema->xDbType)); } diff --git a/src/lib/migrations/PostgresMigrationBuilder.php b/src/lib/migrations/PostgresMigrationBuilder.php index 847fb81b..cc4d8509 100644 --- a/src/lib/migrations/PostgresMigrationBuilder.php +++ b/src/lib/migrations/PostgresMigrationBuilder.php @@ -288,7 +288,7 @@ public function handleCommentsMigration() foreach ($this->newColumns as $column) { /** @var ColumnSchema $column */ if ($column->comment) { - if (MigrationRecordBuilder::isXDbType($column)) { + if (MigrationRecordBuilder::isXDbTypePresent($column)) { $this->migration ->addUpCode($this->recordBuilder->addCommentOnColumn($tableAlias, $column->name, $column->comment)); }