From cc5bb71f75d389a0035e083e25c3897586b0bbd2 Mon Sep 17 00:00:00 2001 From: Manu311 Date: Tue, 3 Sep 2019 18:36:37 +0200 Subject: [PATCH] Fix #17449: Ensure `CHECK` statement goes after `COMMENT` in MySQL `QueryBuilder::addCommentOnColumn()` --- framework/CHANGELOG.md | 1 + framework/db/mysql/QueryBuilder.php | 11 ++++++++++- tests/README.md | 2 +- tests/data/config-docker.php | 4 ++-- tests/docker-compose.yml | 4 ++-- tests/framework/db/mysql/QueryBuilderTest.php | 24 ++++++++++++++++++++++++ 6 files changed, 40 insertions(+), 6 deletions(-) diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md index 1f5e0d1..4eaa458 100644 --- a/framework/CHANGELOG.md +++ b/framework/CHANGELOG.md @@ -11,6 +11,7 @@ Yii Framework 2 Change Log - Bug #17355: Fixed Pjax after request event bug (kamarton) - Bug #17522: `DbManager::isEmptyUserId()` is now protected (samdark) - Bug #17434: Fixed Internet Explorer 11 AJAX redirect bug in case of 301 and 302 response codes (`XMLHttpRequest: Network Error 0x800c0008`) (kamarton) +- Bug #17449: Ensure `CHECK` statement goes after `COMMENT` in MySQL `QueryBuilder::addCommentOnColumn()` (Manu311) 2.0.25 August 13, 2019 diff --git a/framework/db/mysql/QueryBuilder.php b/framework/db/mysql/QueryBuilder.php index e41a120..14d729b 100644 --- a/framework/db/mysql/QueryBuilder.php +++ b/framework/db/mysql/QueryBuilder.php @@ -306,11 +306,20 @@ class QueryBuilder extends \yii\db\QueryBuilder $definition = trim(preg_replace("/COMMENT '(?:''|[^'])*'/i", '', $this->getColumnDefinition($table, $column))); - return 'ALTER TABLE ' . $this->db->quoteTableName($table) + $checkRegex = '/CHECK *(\(([^()]|(?-2))*\))/'; + $check = preg_match($checkRegex, $definition, $checkMatches); + if ($check === 1) { + $definition = preg_replace($checkRegex, '', $definition); + } + $alterSql = 'ALTER TABLE ' . $this->db->quoteTableName($table) . ' CHANGE ' . $this->db->quoteColumnName($column) . ' ' . $this->db->quoteColumnName($column) . (empty($definition) ? '' : ' ' . $definition) . ' COMMENT ' . $this->db->quoteValue($comment); + if ($check === 1) { + $alterSql .= ' ' . $checkMatches[0]; + } + return $alterSql; } /** diff --git a/tests/README.md b/tests/README.md index ff680f5..cc72b0e 100644 --- a/tests/README.md +++ b/tests/README.md @@ -168,4 +168,4 @@ Execute jobs via shell runner (with docker-compose support) $ gitlab-runner exec shell build $ gitlab-runner exec shell test - \ No newline at end of file + diff --git a/tests/data/config-docker.php b/tests/data/config-docker.php index 86c3f83..ebdf671 100644 --- a/tests/data/config-docker.php +++ b/tests/data/config-docker.php @@ -29,8 +29,8 @@ $config = [ ], 'mysql' => [ 'dsn' => 'mysql:host=mysql;dbname=yiitest', - 'username' => 'travis', - 'password' => 'travis', + 'username' => 'root', + 'password' => 'secret', 'fixture' => __DIR__ . '/mysql.sql', ], 'sqlite' => [ diff --git a/tests/docker-compose.yml b/tests/docker-compose.yml index 1e22318..13d86eb 100644 --- a/tests/docker-compose.yml +++ b/tests/docker-compose.yml @@ -10,8 +10,8 @@ services: volumes: - ../tests/data/config-docker.php:/project/tests/data/config.php # Enable for debugging, enabling tests might be slow for file access (data) on host-volume - #- ../tests:/project/tests - # - ../framework:/project/framework + - ../tests:/project/tests + - ../framework:/project/framework # Tmpfs volume (experimental, asset tests may fail) #tmpfs: # - /project/tests/runtime diff --git a/tests/framework/db/mysql/QueryBuilderTest.php b/tests/framework/db/mysql/QueryBuilderTest.php index 93f341d1..ebb89d7 100644 --- a/tests/framework/db/mysql/QueryBuilderTest.php +++ b/tests/framework/db/mysql/QueryBuilderTest.php @@ -343,4 +343,28 @@ class QueryBuilderTest extends \yiiunit\framework\db\QueryBuilderTest return $items; } + + public function testIssue17449() + { + $db = $this->getConnection(); + $pdo = $db->pdo; + $pdo->exec('DROP TABLE IF EXISTS `issue_17449`'); + + $tableQuery = <<createCommand($tableQuery)->execute(); + + $actual = $db->createCommand()->addCommentOnColumn('issue_17449', 'test_column', 'Some comment')->rawSql; + + $checkPos = stripos($actual, 'check'); + if ($checkPos === false) { + $this->markTestSkipped("The used MySql-Server removed or moved the CHECK from the column line, so the original bug doesn't affect it"); + } + $commentPos = stripos($actual, 'comment'); + $this->assertNotFalse($commentPos); + $this->assertLessThan($checkPos, $commentPos); + } }