diff --git a/CHANGELOG.md b/CHANGELOG.md index ab6aba6bf..a0788a485 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -107,7 +107,7 @@ - Enh #991: Improve types in `ConnectionInterface::transaction()` (@kikara) - Chg #998: Add `yiisoft/db-implementation` virtual package as dependency (@vjik) - Chg #999: Remove `requireTransaction()` method and `$isolationLevel` property from `AbstractCommand` (@vjik) -- Enh #1000: Prepare values in `Columns` (@vjik) +- Chg #1000, #1007: Add `Equals` condition. Remove `Hash` condition in favor `Equals` and `In` usage (@vjik) - Chg #1001: Remove `ParamInterface` (@vjik) - Chg #1001: Add public properties `$type` and `$value` to `Param` class instead of `getType()` and `getValue()` methods that were removed (@vjik) - Chg #1002: Remove specific condition interfaces (@vjik) diff --git a/UPGRADE.md b/UPGRADE.md index 87a29b31f..c41acad0e 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -261,3 +261,4 @@ Each table column has its own class in the `Yiisoft\Db\Schema\Column` namespace - Remove `AbstractConjunctionCondition` and `AbstractOverlapsConditionBuilder`; - Change namespace of condition and condition builder classes; - Remove `AbstractDsn` and `AbstractDsnSocket` classes and `DsnInterface` interface; +- Remove `Hash` condition; diff --git a/src/QueryBuilder/AbstractDQLQueryBuilder.php b/src/QueryBuilder/AbstractDQLQueryBuilder.php index b5001ccb1..9e308b736 100644 --- a/src/QueryBuilder/AbstractDQLQueryBuilder.php +++ b/src/QueryBuilder/AbstractDQLQueryBuilder.php @@ -22,7 +22,6 @@ use Yiisoft\Db\Expression\CaseExpressionBuilder; use Yiisoft\Db\Expression\StructuredExpression; use Yiisoft\Db\Expression\StructuredExpressionBuilder; -use Yiisoft\Db\QueryBuilder\Condition\Columns; use Yiisoft\Db\QueryBuilder\Condition\ConditionInterface; use Yiisoft\Db\QueryBuilder\Condition\Simple; use Yiisoft\Db\Query\Query; @@ -33,6 +32,7 @@ use function array_filter; use function array_merge; use function array_shift; +use function count; use function implode; use function is_array; use function is_bool; @@ -449,11 +449,19 @@ public function createConditionFromArray(array $condition): ConditionInterface return $className::fromArrayDefinition($operator, $condition); } - /** - * Key-value format: 'column1' => 'value1', 'column2' => 'value2', ... - * @psalm-var array $condition - */ - return new Columns($condition); + $conditions = []; + foreach ($condition as $column => $value) { + if (!is_string($column)) { + throw new InvalidArgumentException('Condition array must have string keys.'); + } + if (is_iterable($value) || $value instanceof QueryInterface) { + $conditions[] = new Condition\In($column, 'IN', $value); + continue; + } + $conditions[] = new Condition\Equals($column, $value); + } + + return count($conditions) === 1 ? $conditions[0] : new Condition\AndX($conditions); } public function getExpressionBuilder(ExpressionInterface $expression): object @@ -509,6 +517,7 @@ protected function defaultConditionClasses(): array 'NOT' => Condition\Not::class, 'AND' => Condition\AndX::class, 'OR' => Condition\OrX::class, + '=' => Condition\Equals::class, 'BETWEEN' => Condition\Between::class, 'NOT BETWEEN' => Condition\Between::class, 'IN' => Condition\In::class, @@ -545,9 +554,9 @@ protected function defaultExpressionBuilders(): array Condition\Between::class => Condition\Builder\BetweenBuilder::class, Condition\In::class => Condition\Builder\InBuilder::class, Condition\Like::class => Condition\Builder\LikeBuilder::class, + Condition\Equals::class => Condition\Builder\EqualsBuilder::class, Condition\Exists::class => Condition\Builder\ExistsBuilder::class, Simple::class => Condition\Builder\SimpleBuilder::class, - Columns::class => Condition\Builder\ColumnsBuilder::class, Condition\BetweenColumns::class => Condition\Builder\BetweenColumnsBuilder::class, JsonExpression::class => JsonExpressionBuilder::class, ArrayExpression::class => ArrayExpressionBuilder::class, diff --git a/src/QueryBuilder/Condition/Builder/ColumnsBuilder.php b/src/QueryBuilder/Condition/Builder/ColumnsBuilder.php deleted file mode 100644 index 50ef1835a..000000000 --- a/src/QueryBuilder/Condition/Builder/ColumnsBuilder.php +++ /dev/null @@ -1,62 +0,0 @@ - - */ -class ColumnsBuilder implements ExpressionBuilderInterface -{ - public function __construct( - private readonly QueryBuilderInterface $queryBuilder, - ) { - } - - /** - * Build SQL for {@see Columns}. - * - * @param Columns $expression - * - * @throws Exception - * @throws InvalidArgumentException - * @throws InvalidConfigException - * @throws NotSupportedException - */ - public function build(ExpressionInterface $expression, array &$params = []): string - { - $parts = []; - foreach ($expression->values as $column => $value) { - if (is_iterable($value) || $value instanceof QueryInterface) { - /** IN condition */ - $parts[] = $this->queryBuilder->buildCondition(new In($column, 'IN', $value), $params); - } else { - $column = $this->queryBuilder->getQuoter()->quoteColumnName($column); - - $parts[] = $value === null - ? "$column IS NULL" - : $column . '=' . $this->queryBuilder->buildValue($value, $params); - } - } - - return (count($parts) === 1) ? $parts[0] : ('(' . implode(') AND (', $parts) . ')'); - } -} diff --git a/src/QueryBuilder/Condition/Builder/EqualsBuilder.php b/src/QueryBuilder/Condition/Builder/EqualsBuilder.php new file mode 100644 index 000000000..91aa51127 --- /dev/null +++ b/src/QueryBuilder/Condition/Builder/EqualsBuilder.php @@ -0,0 +1,68 @@ + + */ +class EqualsBuilder implements ExpressionBuilderInterface +{ + public function __construct( + private readonly QueryBuilderInterface $queryBuilder, + ) { + } + + /** + * Build SQL for {@see Equals}. + * + * @param Equals $expression + * + * @throws Exception + * @throws InvalidConfigException + * @throws NotSupportedException + */ + public function build(ExpressionInterface $expression, array &$params = []): string + { + $column = $this->prepareColumn($expression->column, $params); + $value = $this->prepareValue($expression->value, $params); + + return $value === null + ? "$column IS NULL" + : "$column = $value"; + } + + /** + * @throws InvalidConfigException + * @throws NotSupportedException + * @throws Exception + */ + private function prepareColumn(string|ExpressionInterface $column, array &$params): string + { + if ($column instanceof ExpressionInterface) { + return $this->queryBuilder->buildExpression($column, $params); + } + + return $this->queryBuilder->getQuoter()->quoteColumnName($column); + } + + private function prepareValue(mixed $value, array &$params): string|null + { + if ($value === null) { + return null; + } + + return $this->queryBuilder->buildValue($value, $params); + } +} diff --git a/src/QueryBuilder/Condition/Columns.php b/src/QueryBuilder/Condition/Columns.php deleted file mode 100644 index 51c613ef1..000000000 --- a/src/QueryBuilder/Condition/Columns.php +++ /dev/null @@ -1,30 +0,0 @@ - $values - */ - public function __construct( - public readonly array $values = [], - ) { - } - - /** - * Creates a condition based on the given operator and operands. - */ - public static function fromArrayDefinition(string $operator, array $operands): self - { - /** @psalm-var array $operands */ - return new self($operands); - } -} diff --git a/src/QueryBuilder/Condition/Equals.php b/src/QueryBuilder/Condition/Equals.php new file mode 100644 index 000000000..c0f51d22c --- /dev/null +++ b/src/QueryBuilder/Condition/Equals.php @@ -0,0 +1,50 @@ +assertSame( DbHelper::replaceQuotes( <<getDriverName(), ), @@ -1520,7 +1520,7 @@ public function testBuildWithWhereExistsArrayParameters(): void $this->assertSame( DbHelper::replaceQuotes( <<getDriverName(), ), @@ -1648,15 +1648,22 @@ public function testCreateOverlapsConditionFromArrayWithInvalidColumn(): void public function testCreateOverlapsConditionFromArrayWithInvalidValues(): void { - $db = $this->getConnection(); - $qb = $db->getQueryBuilder(); + $qb = $this->getConnection()->getQueryBuilder(); $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage('Operator "JSON OVERLAPS" requires values to be iterable or ExpressionInterface.'); - $qb->createConditionFromArray(['json overlaps', 'column', 1]); } + public function testCreateConditionFromArrayWithIntegerKeys(): void + { + $qb = $this->getConnection()->getQueryBuilder(); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Condition array must have string keys.'); + $qb->createConditionFromArray(['id' => 45, 9 => 'hello']); + } + /** * @dataProvider \Yiisoft\Db\Tests\Provider\QueryBuilderProvider::createIndex */ @@ -2416,7 +2423,7 @@ public function testOverrideParameters1(): void $this->assertEquals([':id', ':qp2', ':qp2_0',], array_keys($command->getParams())); $this->assertEquals( DbHelper::replaceQuotes( - 'SELECT * FROM [[animal]] WHERE (id = 1 AND type = \'test\') AND ([[type]]=\'test1\')', + 'SELECT * FROM [[animal]] WHERE (id = 1 AND type = \'test\') AND ([[type]] = \'test1\')', $db->getDriverName() ), $command->getRawSql() @@ -2442,7 +2449,7 @@ public function testOverrideParameters2(): void $this->assertEquals([':qp1', ':qp1_0',], array_keys($command->getParams())); $this->assertEquals( DbHelper::replaceQuotes( - 'SELECT * FROM [[animal]] WHERE (id = 1) AND ([[type]]=\'test2\')', + 'SELECT * FROM [[animal]] WHERE (id = 1) AND ([[type]] = \'test2\')', $db->getDriverName() ), $command->getRawSql() diff --git a/tests/Db/Command/CommandTest.php b/tests/Db/Command/CommandTest.php index 78c329c85..ce65ac283 100644 --- a/tests/Db/Command/CommandTest.php +++ b/tests/Db/Command/CommandTest.php @@ -307,7 +307,7 @@ public function testDelete(): void $this->assertSame( DbHelper::replaceQuotes( <<getDriverName(), ), @@ -720,7 +720,7 @@ public function testUpdate(): void $command = $db->createCommand(); $command->update('{{table}}', ['name' => 'John'], ['id' => 1]); - $this->assertSame('UPDATE [table] SET [name]=:qp0 WHERE [id]=1', $command->getSql()); + $this->assertSame('UPDATE [table] SET [name]=:qp0 WHERE [id] = 1', $command->getSql()); $this->assertSame([':qp0' => 'John'], $command->getParams()); } diff --git a/tests/Db/QueryBuilder/Condition/Builder/EqualsBuilderTest.php b/tests/Db/QueryBuilder/Condition/Builder/EqualsBuilderTest.php new file mode 100644 index 000000000..bd7886be9 --- /dev/null +++ b/tests/Db/QueryBuilder/Condition/Builder/EqualsBuilderTest.php @@ -0,0 +1,148 @@ +getConnection()->getQueryBuilder(); + $equalsCondition = new Equals('id', 42); + $params = []; + + $result = (new EqualsBuilder($qb))->build($equalsCondition, $params); + + assertSame('[id] = 42', $result); + assertSame([], $params); + } + + public function testBuildWithExpressionColumn(): void + { + $qb = $this->getConnection()->getQueryBuilder(); + $columnExpression = new Expression('UPPER(name)'); + $equalsCondition = new Equals($columnExpression, 'JOHN'); + $params = []; + + $result = (new EqualsBuilder($qb))->build($equalsCondition, $params); + + assertSame('UPPER(name) = :qp0', $result); + assertEquals([':qp0' => new Param('JOHN', DataType::STRING)], $params); + } + + public function testBuildWithFunctionColumn(): void + { + $qb = $this->getConnection()->getQueryBuilder(); + $equalsCondition = new Equals('COUNT(*)', 5); + $params = []; + + $result = (new EqualsBuilder($qb))->build($equalsCondition, $params); + + assertSame('COUNT(*) = 5', $result); + assertSame([], $params); + } + + public function testBuildWithNullValue(): void + { + $qb = $this->getConnection()->getQueryBuilder(); + $equalsCondition = new Equals('status', null); + $params = []; + + $result = (new EqualsBuilder($qb))->build($equalsCondition, $params); + + assertSame('[status] IS NULL', $result); + assertSame([], $params); + } + + public function testBuildWithExpressionValue(): void + { + $qb = $this->getConnection()->getQueryBuilder(); + $valueExpression = new Expression('NOW()'); + $equalsCondition = new Equals('created_at', $valueExpression); + $params = []; + + $result = (new EqualsBuilder($qb))->build($equalsCondition, $params); + + assertSame('[created_at] = NOW()', $result); + assertSame([], $params); + } + + public static function dataColumnTypes(): iterable + { + yield 'simple column' => ['name', '[name]']; + yield 'column with table prefix' => ['user.name', '[user].[name]']; + yield 'column with function' => ['COUNT(*)', 'COUNT(*)']; + yield 'column with expression' => ['UPPER(title)', 'UPPER(title)']; + yield 'column with subquery' => ['(SELECT id FROM users)', '(SELECT id FROM users)']; + } + + #[DataProvider('dataColumnTypes')] + public function testBuildWithDifferentColumnTypes(string $column, string $expectedColumn): void + { + $qb = $this->getConnection()->getQueryBuilder(); + $equalsCondition = new Equals($column, 5); + $params = []; + + $result = (new EqualsBuilder($qb))->build($equalsCondition, $params); + + assertSame($expectedColumn . ' = 5', $result); + assertSame([], $params); + } + + public static function dataValueTypes(): iterable + { + yield 'string value' => ['hello', ':qp0', [':qp0' => new Param('hello', DataType::STRING)]]; + yield 'integer value' => [42, '42', []]; + yield 'float value' => [3.14, '3.14', []]; + yield 'boolean true' => [true, 'TRUE', []]; + yield 'boolean false' => [false, 'FALSE', []]; + yield 'null value' => [null, null, []]; + } + + #[DataProvider('dataValueTypes')] + public function testBuildWithDifferentValueTypes(mixed $value, ?string $expectedValue, array $expectedParams): void + { + $qb = $this->getConnection()->getQueryBuilder(); + $equalsCondition = new Equals('column', $value); + $params = []; + + $result = (new EqualsBuilder($qb))->build($equalsCondition, $params); + + ($value === null) + ? assertSame('[column] IS NULL', $result) + : assertSame('[column] = ' . $expectedValue, $result); + assertEquals($expectedParams, $params); + } + + public function testBuildWithComplexExpression(): void + { + $qb = $this->getConnection()->getQueryBuilder(); + $columnExpression = new Expression('COALESCE(name, :default)', [':default' => 'Unknown']); + $valueExpression = new Expression('UPPER(:value)', [':value' => 'john']); + $equalsCondition = new Equals($columnExpression, $valueExpression); + $params = []; + + $result = (new EqualsBuilder($qb))->build($equalsCondition, $params); + + assertSame('COALESCE(name, :default) = UPPER(:value)', $result); + assertSame([':default' => 'Unknown', ':value' => 'john'], $params); + } +} diff --git a/tests/Db/QueryBuilder/Condition/ColumnsTest.php b/tests/Db/QueryBuilder/Condition/ColumnsTest.php deleted file mode 100644 index 646e602e9..000000000 --- a/tests/Db/QueryBuilder/Condition/ColumnsTest.php +++ /dev/null @@ -1,28 +0,0 @@ - false, 'active' => true]); - - $this->assertSame(['expired' => false, 'active' => true], $condition->values); - } - - public function testFromArrayDefinition(): void - { - $condition = Columns::fromArrayDefinition('AND', ['expired' => false, 'active' => true]); - - $this->assertSame(['expired' => false, 'active' => true], $condition->values); - } -} diff --git a/tests/Db/QueryBuilder/Condition/EqualsTest.php b/tests/Db/QueryBuilder/Condition/EqualsTest.php new file mode 100644 index 000000000..07d31bf4b --- /dev/null +++ b/tests/Db/QueryBuilder/Condition/EqualsTest.php @@ -0,0 +1,58 @@ +column); + assertSame(25, $condition->value); + } + + public function testFromArrayDefinitionMissingFirstOperand(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage("Operator '=' requires first operand as column."); + + Equals::fromArrayDefinition('=', []); + } + + public function testFromArrayDefinitionMissingSecondOperand(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage("Operator '=' requires second operand as value."); + + Equals::fromArrayDefinition('=', ['column']); + } + + public static function dataFromArrayDefinitionInvalidColumn(): iterable + { + yield [123]; + yield [['hello']]; + yield [null]; + } + + #[DataProvider('dataFromArrayDefinitionInvalidColumn')] + public function testFromArrayDefinitionInvalidColumn(mixed $column): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage("Operator '=' requires column to be string or ExpressionInterface."); + + Equals::fromArrayDefinition('=', [$column, 'value']); + } +} diff --git a/tests/Provider/QueryBuilderProvider.php b/tests/Provider/QueryBuilderProvider.php index 503dec90c..33ddb976e 100644 --- a/tests/Provider/QueryBuilderProvider.php +++ b/tests/Provider/QueryBuilderProvider.php @@ -314,7 +314,7 @@ public static function buildCondition(): array ['expired' => false], (new Query(static::getDb()))->select('count(*) > 1')->from('queue'), ], - '([[expired]]=FALSE) AND ((SELECT count(*) > 1 FROM [[queue]]))', + '([[expired]] = FALSE) AND ((SELECT count(*) > 1 FROM [[queue]]))', [], ], @@ -404,7 +404,7 @@ public static function buildCondition(): array 'id', (new Query(static::getDb()))->select('id')->from('users')->where(['active' => 1]), ], - '[[id]] IN (SELECT [[id]] FROM [[users]] WHERE [[active]]=1)', + '[[id]] IN (SELECT [[id]] FROM [[users]] WHERE [[active]] = 1)', [], ], [ @@ -413,7 +413,7 @@ public static function buildCondition(): array 'id', (new Query(static::getDb()))->select('id')->from('users')->where(['active' => 1]), ], - '[[id]] NOT IN (SELECT [[id]] FROM [[users]] WHERE [[active]]=1)', + '[[id]] NOT IN (SELECT [[id]] FROM [[users]] WHERE [[active]] = 1)', [], ], [['in', 'id', 1], '[[id]]=:qp0', [':qp0' => 1]], @@ -504,7 +504,7 @@ public static function buildCondition(): array 'in', (new Query(static::getDb()))->select('id')->from('users')->where(['active' => 1]), ), - '([[id]]) IN (SELECT [[id]] FROM [[users]] WHERE [[active]]=1)', + '([[id]]) IN (SELECT [[id]] FROM [[users]] WHERE [[active]] = 1)', [], ], 'inCondition-custom-3' => [ @@ -528,7 +528,7 @@ public static function buildCondition(): array 'in', (new Query(static::getDb()))->select('id')->from('users')->where(['active' => 1]), ), - '(id) IN (SELECT [[id]] FROM [[users]] WHERE [[active]]=1)', + '(id) IN (SELECT [[id]] FROM [[users]] WHERE [[active]] = 1)', [], ], @@ -538,7 +538,7 @@ public static function buildCondition(): array 'exists', (new Query(static::getDb()))->select('id')->from('users')->where(['active' => 1]), ], - 'EXISTS (SELECT [[id]] FROM [[users]] WHERE [[active]]=1)', + 'EXISTS (SELECT [[id]] FROM [[users]] WHERE [[active]] = 1)', [], ], [ @@ -546,11 +546,11 @@ public static function buildCondition(): array 'not exists', (new Query(static::getDb()))->select('id')->from('users')->where(['active' => 1]), ], - 'NOT EXISTS (SELECT [[id]] FROM [[users]] WHERE [[active]]=1)', [], + 'NOT EXISTS (SELECT [[id]] FROM [[users]] WHERE [[active]] = 1)', [], ], /* simple conditions */ - [['=', 'a', 'b'], '[[a]] = :qp0', [':qp0' => 'b']], + [['=', 'a', 'b'], '[[a]] = :qp0', [':qp0' => new Param('b', DataType::STRING)]], [['>', 'a', 1], '[[a]] > :qp0', [':qp0' => 1]], [['>=', 'a', 'b'], '[[a]] >= :qp0', [':qp0' => 'b']], [['<', 'a', 2], '[[a]] < :qp0', [':qp0' => 2]], @@ -573,28 +573,28 @@ public static function buildCondition(): array 'date', (new Query(static::getDb()))->select('max(date)')->from('test')->where(['id' => 5]), ], - '[[date]] = (SELECT max(date) FROM [[test]] WHERE [[id]]=5)', + '[[date]] = (SELECT max(date) FROM [[test]] WHERE [[id]] = 5)', [], ], - [['=', 'a', null], '[[a]] = NULL', []], + [['=', 'a', null], '[[a]] IS NULL', []], /* operand1 is Expression */ [ ['=', new Expression('date'), '2019-08-01'], 'date = :qp0', - [':qp0' => '2019-08-01'], + [':qp0' => new Param('2019-08-01', DataType::STRING)], ], [ ['=', (new Query(static::getDb()))->select('COUNT(*)')->from('test')->where(['id' => 6]), 0], - '(SELECT COUNT(*) FROM [[test]] WHERE [[id]]=6) = :qp0', - [':qp0' => 0], + '(SELECT COUNT(*) FROM [[test]] WHERE [[id]] = 6) = 0', + [], ], /* columns */ - [['a' => 1, 'b' => 2], '([[a]]=1) AND ([[b]]=2)', []], + [['a' => 1, 'b' => 2], '([[a]] = 1) AND ([[b]] = 2)', []], [ ['a' => new Expression('CONCAT(col1, col2)'), 'b' => 2], - '([[a]]=CONCAT(col1, col2)) AND ([[b]]=2)', + '([[a]] = CONCAT(col1, col2)) AND ([[b]] = 2)', [], ], [['a' => null], '[[a]] IS NULL', []], @@ -628,7 +628,7 @@ public static function buildCondition(): array /* json conditions */ 'search by property in JSON column' => [ ['=', new Expression("(json_col->>'$.someKey')"), 42], - "(json_col->>'$.someKey') = :qp0", [':qp0' => 42], + "(json_col->>'$.someKey') = 42", [], ], ]; @@ -983,7 +983,7 @@ public static function delete(): array ['is_enabled' => false, 'power' => new Expression('WRONG_POWER()')], DbHelper::replaceQuotes( << 'bar'], DbHelper::replaceQuotes( << 'boolean'], DbHelper::replaceQuotes( << 1, ':qp1' => $paramA, ':pv2' => $paramB], + [':qp0' => $paramA, ':pv1' => $paramB], 'b', ], ];