From 97385d172149c4909270226b2e20bf55f2a7b9d4 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sat, 18 Jan 2025 12:29:21 +0700 Subject: [PATCH 1/5] Fix `ColumnDefinitionParser` --- .../AbstractColumnDefinitionBuilder.php | 23 ++++++++++--- src/Syntax/ColumnDefinitionParser.php | 8 ++--- tests/AbstractColumnDefinitionParserTest.php | 32 +++++++++++++++++++ .../Db/Syntax/ColumnDefinitionParserTest.php | 17 ++-------- 4 files changed, 56 insertions(+), 24 deletions(-) create mode 100644 tests/AbstractColumnDefinitionParserTest.php diff --git a/src/QueryBuilder/AbstractColumnDefinitionBuilder.php b/src/QueryBuilder/AbstractColumnDefinitionBuilder.php index b0894c30f..eb21ac721 100644 --- a/src/QueryBuilder/AbstractColumnDefinitionBuilder.php +++ b/src/QueryBuilder/AbstractColumnDefinitionBuilder.php @@ -65,6 +65,22 @@ public function buildAlter(ColumnInterface $column): string return $this->build($column); } + /** + * Check if the database column type allow scale specification. + */ + protected function isAllowScale(string $dbType): bool + { + return in_array(strtolower($dbType), static::TYPES_WITH_SCALE, true); + } + + /** + * Check if the database column type allow size specification. + */ + protected function isAllowSize(string $dbType): bool + { + return in_array(strtolower($dbType), static::TYPES_WITH_SIZE, true); + } + /** * Builds the auto increment clause for the column. * @@ -254,10 +270,7 @@ protected function buildType(ColumnInterface $column): string { $dbType = $this->getDbType($column); - if (empty($dbType) - || $dbType[-1] === ')' - || !in_array(strtolower($dbType), static::TYPES_WITH_SIZE, true) - ) { + if (empty($dbType) || $dbType[-1] === ')' || !$this->isAllowSize($dbType)) { return $dbType; } @@ -269,7 +282,7 @@ protected function buildType(ColumnInterface $column): string $scale = $column->getScale(); - if ($scale === null || !in_array(strtolower($dbType), static::TYPES_WITH_SCALE, true)) { + if ($scale === null || !$this->isAllowScale($dbType)) { return "$dbType($size)"; } diff --git a/src/Syntax/ColumnDefinitionParser.php b/src/Syntax/ColumnDefinitionParser.php index c6280fa04..c165fe294 100644 --- a/src/Syntax/ColumnDefinitionParser.php +++ b/src/Syntax/ColumnDefinitionParser.php @@ -16,7 +16,7 @@ /** * Parses column definition string. For example, `string(255)` or `int unsigned`. */ -final class ColumnDefinitionParser +class ColumnDefinitionParser { /** * Parses column definition string. @@ -62,7 +62,7 @@ public function parse(string $definition): array /** * @psalm-return array{enumValues: list} */ - private function enumInfo(string $values): array + protected function enumInfo(string $values): array { preg_match_all("/'([^']*)'/", $values, $matches); @@ -80,7 +80,7 @@ private function enumInfo(string $values): array * unsigned?: bool * } */ - private function extraInfo(string $extra): array + protected function extraInfo(string $extra): array { if (empty($extra)) { return []; @@ -137,7 +137,7 @@ private function extraInfo(string $extra): array /** * @psalm-return array{size: int, scale?: int} */ - private function sizeInfo(string $size): array + protected function sizeInfo(string $size): array { $values = explode(',', $size); diff --git a/tests/AbstractColumnDefinitionParserTest.php b/tests/AbstractColumnDefinitionParserTest.php new file mode 100644 index 000000000..f3d5e50d4 --- /dev/null +++ b/tests/AbstractColumnDefinitionParserTest.php @@ -0,0 +1,32 @@ +createColumnDefinitionParser(); + + $this->assertSame($expected, $parser->parse($definition)); + } +} diff --git a/tests/Db/Syntax/ColumnDefinitionParserTest.php b/tests/Db/Syntax/ColumnDefinitionParserTest.php index 543e0391a..46a4deff8 100644 --- a/tests/Db/Syntax/ColumnDefinitionParserTest.php +++ b/tests/Db/Syntax/ColumnDefinitionParserTest.php @@ -4,24 +4,11 @@ namespace Yiisoft\Db\Tests\Db\Syntax; -use PHPUnit\Framework\TestCase; -use Yiisoft\Db\Syntax\ColumnDefinitionParser; -use Yiisoft\Db\Tests\Support\TestTrait; +use Yiisoft\Db\Tests\AbstractColumnDefinitionParserTest; /** * @group db */ -final class ColumnDefinitionParserTest extends TestCase +final class ColumnDefinitionParserTest extends AbstractColumnDefinitionParserTest { - use TestTrait; - - /** - * @dataProvider \Yiisoft\Db\Tests\Provider\ColumnDefinitionParserProvider::parse - */ - public function testParse(string $definition, array $expected): void - { - $parser = new ColumnDefinitionParser(); - - $this->assertSame($expected, $parser->parse($definition)); - } } From 9bbd3da012dce531c665950d469e79bd058ef814 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sat, 18 Jan 2025 12:33:07 +0700 Subject: [PATCH 2/5] rearrange methods --- .../AbstractColumnDefinitionBuilder.php | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/QueryBuilder/AbstractColumnDefinitionBuilder.php b/src/QueryBuilder/AbstractColumnDefinitionBuilder.php index eb21ac721..2e50ae3f6 100644 --- a/src/QueryBuilder/AbstractColumnDefinitionBuilder.php +++ b/src/QueryBuilder/AbstractColumnDefinitionBuilder.php @@ -65,22 +65,6 @@ public function buildAlter(ColumnInterface $column): string return $this->build($column); } - /** - * Check if the database column type allow scale specification. - */ - protected function isAllowScale(string $dbType): bool - { - return in_array(strtolower($dbType), static::TYPES_WITH_SCALE, true); - } - - /** - * Check if the database column type allow size specification. - */ - protected function isAllowSize(string $dbType): bool - { - return in_array(strtolower($dbType), static::TYPES_WITH_SIZE, true); - } - /** * Builds the auto increment clause for the column. * @@ -321,4 +305,20 @@ protected function getDefaultUuidExpression(): string { return ''; } + + /** + * Check if the database column type allow scale specification. + */ + protected function isAllowScale(string $dbType): bool + { + return in_array(strtolower($dbType), static::TYPES_WITH_SCALE, true); + } + + /** + * Check if the database column type allow size specification. + */ + protected function isAllowSize(string $dbType): bool + { + return in_array(strtolower($dbType), static::TYPES_WITH_SIZE, true); + } } From 7c8a149c6cf4cf192b85054401ab943e14252301 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sat, 18 Jan 2025 14:44:47 +0700 Subject: [PATCH 3/5] Update CHANGELOG.md [skip ci] --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b3ee76a6..6bff6da9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,10 +39,10 @@ - Enh #875: Ignore "Packets out of order..." warnings in `AbstractPdoCommand::internalExecute()` method (@Tigrov) - Enh #877: Separate column type constants (@Tigrov) - New #878: Realize `ColumnBuilder` class (@Tigrov) -- New #878, #900, #914: Realize `ColumnDefinitionParser` class (@Tigrov) +- New #878, #900, #914, #922: Realize `ColumnDefinitionParser` class (@Tigrov) - Enh #881: Refactor `ColumnSchemaInterface` and `AbstractColumnSchema` (@Tigrov) - New #882: Move `ArrayColumnSchema` and `StructuredColumnSchema` classes from `db-pgsql` package (@Tigrov) -- New #883, #901: Add `ColumnDefinitionBuilder` class and `QueryBuilderInterface::buildColumnDefinition()` method (@Tigrov) +- New #883, #901, #922: Add `ColumnDefinitionBuilder` class and `QueryBuilderInterface::buildColumnDefinition()` method (@Tigrov) - Enh #885: Refactor `AbstractDsn` class (@Tigrov) - Chg #889: Update `AbstractDMLQueryBuilder::insertBatch()` method (@Tigrov) - Enh #890: Add properties of `AbstractColumnSchema` class to constructor (@Tigrov) From 1ba6c6f689a76a0a71522d653dc555fc981d007d Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Sun, 19 Jan 2025 10:41:55 +0300 Subject: [PATCH 4/5] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6bff6da9f..5622db824 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,7 +39,7 @@ - Enh #875: Ignore "Packets out of order..." warnings in `AbstractPdoCommand::internalExecute()` method (@Tigrov) - Enh #877: Separate column type constants (@Tigrov) - New #878: Realize `ColumnBuilder` class (@Tigrov) -- New #878, #900, #914, #922: Realize `ColumnDefinitionParser` class (@Tigrov) +- New #878, #900, #914, #922: Implement `ColumnDefinitionParser` class (@Tigrov) - Enh #881: Refactor `ColumnSchemaInterface` and `AbstractColumnSchema` (@Tigrov) - New #882: Move `ArrayColumnSchema` and `StructuredColumnSchema` classes from `db-pgsql` package (@Tigrov) - New #883, #901, #922: Add `ColumnDefinitionBuilder` class and `QueryBuilderInterface::buildColumnDefinition()` method (@Tigrov) From 2ff7555be953909ea267f50ef2f98768cd141ec3 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sun, 19 Jan 2025 16:49:21 +0700 Subject: [PATCH 5/5] Revert some changes --- .../AbstractColumnDefinitionBuilder.php | 23 ++++--------------- 1 file changed, 5 insertions(+), 18 deletions(-) diff --git a/src/QueryBuilder/AbstractColumnDefinitionBuilder.php b/src/QueryBuilder/AbstractColumnDefinitionBuilder.php index 2e50ae3f6..b0894c30f 100644 --- a/src/QueryBuilder/AbstractColumnDefinitionBuilder.php +++ b/src/QueryBuilder/AbstractColumnDefinitionBuilder.php @@ -254,7 +254,10 @@ protected function buildType(ColumnInterface $column): string { $dbType = $this->getDbType($column); - if (empty($dbType) || $dbType[-1] === ')' || !$this->isAllowSize($dbType)) { + if (empty($dbType) + || $dbType[-1] === ')' + || !in_array(strtolower($dbType), static::TYPES_WITH_SIZE, true) + ) { return $dbType; } @@ -266,7 +269,7 @@ protected function buildType(ColumnInterface $column): string $scale = $column->getScale(); - if ($scale === null || !$this->isAllowScale($dbType)) { + if ($scale === null || !in_array(strtolower($dbType), static::TYPES_WITH_SCALE, true)) { return "$dbType($size)"; } @@ -305,20 +308,4 @@ protected function getDefaultUuidExpression(): string { return ''; } - - /** - * Check if the database column type allow scale specification. - */ - protected function isAllowScale(string $dbType): bool - { - return in_array(strtolower($dbType), static::TYPES_WITH_SCALE, true); - } - - /** - * Check if the database column type allow size specification. - */ - protected function isAllowSize(string $dbType): bool - { - return in_array(strtolower($dbType), static::TYPES_WITH_SIZE, true); - } }