diff --git a/src/QueryBuilder/AbstractDMLQueryBuilder.php b/src/QueryBuilder/AbstractDMLQueryBuilder.php index fad42c957..f8e9f728a 100644 --- a/src/QueryBuilder/AbstractDMLQueryBuilder.php +++ b/src/QueryBuilder/AbstractDMLQueryBuilder.php @@ -14,7 +14,6 @@ use Yiisoft\Db\Exception\NotSupportedException; use Yiisoft\Db\Expression\ExpressionInterface; use Yiisoft\Db\Query\QueryInterface; -use Yiisoft\Db\Schema\ColumnSchemaInterface; use Yiisoft\Db\Schema\QuoterInterface; use Yiisoft\Db\Schema\SchemaInterface; @@ -26,10 +25,8 @@ use function array_merge; use function array_unique; use function array_values; -use function count; use function implode; use function in_array; -use function is_array; use function is_string; use function json_encode; use function preg_match; @@ -52,28 +49,23 @@ public function __construct( ) { } - public function batchInsert(string $table, array $columns, iterable|Generator $rows, array &$params = []): string + public function batchInsert(string $table, array $columns, iterable $rows, array &$params = []): string { if (empty($rows)) { return ''; } - if (($tableSchema = $this->schema->getTableSchema($table)) !== null) { - $columnSchemas = $tableSchema->getColumns(); - } else { - $columnSchemas = []; - } - - $mappedNames = $this->getNormalizeColumnNames($table, $columns); $values = []; + $columns = $this->getNormalizeColumnNames($columns); + $columnSchemas = $this->schema->getTableSchema($table)?->getColumns() ?? []; /** @psalm-var array> $rows */ foreach ($rows as $row) { $placeholders = []; - foreach ($row as $index => $value) { - if (isset($columns[$index], $mappedNames[$columns[$index]], $columnSchemas[$mappedNames[$columns[$index]]])) { + foreach ($row as $i => $value) { + if (isset($columns[$i], $columnSchemas[$columns[$i]])) { /** @psalm-var mixed $value */ - $value = $this->getTypecastValue($value, $columnSchemas[$mappedNames[$columns[$index]]]); + $value = $columnSchemas[$columns[$i]]->dbTypecast($value); } if ($value instanceof ExpressionInterface) { @@ -90,11 +82,10 @@ public function batchInsert(string $table, array $columns, iterable|Generator $r } foreach ($columns as $i => $name) { - $columns[$i] = $this->quoter->quoteColumnName($mappedNames[$name]); + $columns[$i] = $this->quoter->quoteColumnName($name); } - return 'INSERT INTO ' - . $this->quoter->quoteTableName($table) + return 'INSERT INTO ' . $this->quoter->quoteTableName($table) . ' (' . implode(', ', $columns) . ') VALUES ' . implode(', ', $values); } @@ -108,17 +99,11 @@ public function delete(string $table, array|string $condition, array &$params): public function insert(string $table, QueryInterface|array $columns, array &$params = []): string { - /** - * @psalm-var string[] $names - * @psalm-var string[] $placeholders - * @psalm-var string $values - */ [$names, $placeholders, $values, $params] = $this->prepareInsertValues($table, $columns, $params); - return 'INSERT INTO ' - . $this->quoter->quoteTableName($table) + return 'INSERT INTO ' . $this->quoter->quoteTableName($table) . (!empty($names) ? ' (' . implode(', ', $names) . ')' : '') - . (!empty($placeholders) ? ' VALUES (' . implode(', ', $placeholders) . ')' : $values); + . (!empty($placeholders) ? ' VALUES (' . implode(', ', $placeholders) . ')' : ' ' . $values); } public function insertWithReturningPks(string $table, QueryInterface|array $columns, array &$params = []): string @@ -133,10 +118,9 @@ public function resetSequence(string $table, int|string|null $value = null): str public function update(string $table, array $columns, array|string $condition, array &$params = []): string { - /** @psalm-var string[] $lines */ [$lines, $params] = $this->prepareUpdateSets($table, $columns, $params); + $sql = 'UPDATE ' . $this->quoter->quoteTableName($table) . ' SET ' . implode(', ', $lines); - /** @psalm-var array $params */ $where = $this->queryBuilder->buildWhere($condition, $params); return $where === '' ? $sql : $sql . ' ' . $where; @@ -163,20 +147,21 @@ public function upsert( * @throws InvalidConfigException * @throws NotSupportedException * - * @return array Array of column names, values, and params. + * @return array Array of quoted column names, values, and params. + * @psalm-return array{0: string[], 1: string, 2: array} */ protected function prepareInsertSelectSubQuery(QueryInterface $columns, array $params = []): array { - if (empty($columns->getSelect()) || in_array('*', $columns->getSelect(), true)) { + /** @psalm-var string[] $select */ + $select = $columns->getSelect(); + + if (empty($select) || in_array('*', $select, true)) { throw new InvalidArgumentException('Expected select query object with enumerated (named) parameters'); } [$values, $params] = $this->queryBuilder->build($columns, $params); $names = []; - $values = ' ' . $values; - /** @psalm-var string[] $select */ - $select = $columns->getSelect(); foreach ($select as $title => $field) { if (is_string($title)) { @@ -204,37 +189,46 @@ protected function prepareInsertSelectSubQuery(QueryInterface $columns, array $p * @throws InvalidConfigException * @throws InvalidArgumentException * @throws NotSupportedException + * + * @return array Array of quoted column names, placeholders, values, and params. + * @psalm-return array{0: string[], 1: string[], 2: string, 3: array} */ protected function prepareInsertValues(string $table, array|QueryInterface $columns, array $params = []): array { - $tableSchema = $this->schema->getTableSchema($table); - $columnSchemas = $tableSchema !== null ? $tableSchema->getColumns() : []; + if ($columns instanceof QueryInterface) { + [$names, $values, $params] = $this->prepareInsertSelectSubQuery($columns, $params); + return [$names, [], $values, $params]; + } + + if (empty($columns)) { + return [[], [], 'DEFAULT VALUES', []]; + } + $names = []; $placeholders = []; - $values = ' DEFAULT VALUES'; + $columns = $this->normalizeColumnNames($columns); + $columnSchemas = $this->schema->getTableSchema($table)?->getColumns() ?? []; - if ($columns instanceof QueryInterface) { - [$names, $values, $params] = $this->prepareInsertSelectSubQuery($columns, $params); - } else { - $columns = $this->normalizeColumnNames($table, $columns); - /** - * @psalm-var mixed $value - * @psalm-var array $columns - */ - foreach ($columns as $name => $value) { - $names[] = $this->quoter->quoteColumnName($name); + /** + * @psalm-var mixed $value + * @psalm-var array $columns + */ + foreach ($columns as $name => $value) { + $names[] = $this->quoter->quoteColumnName($name); + + if (isset($columnSchemas[$name])) { /** @var mixed $value */ - $value = $this->getTypecastValue($value, $columnSchemas[$name] ?? null); + $value = $columnSchemas[$name]->dbTypecast($value); + } - if ($value instanceof ExpressionInterface) { - $placeholders[] = $this->queryBuilder->buildExpression($value, $params); - } else { - $placeholders[] = $this->queryBuilder->bindParam($value, $params); - } + if ($value instanceof ExpressionInterface) { + $placeholders[] = $this->queryBuilder->buildExpression($value, $params); + } else { + $placeholders[] = $this->queryBuilder->bindParam($value, $params); } } - return [$names, $placeholders, $values, $params]; + return [$names, $placeholders, '', $params]; } /** @@ -244,21 +238,25 @@ protected function prepareInsertValues(string $table, array|QueryInterface $colu * @throws InvalidConfigException * @throws InvalidArgumentException * @throws NotSupportedException + * + * @psalm-return array{0: string[], 1: array} */ protected function prepareUpdateSets(string $table, array $columns, array $params = []): array { - $tableSchema = $this->schema->getTableSchema($table); - $columnSchemas = $tableSchema !== null ? $tableSchema->getColumns() : []; $sets = []; - $columns = $this->normalizeColumnNames($table, $columns); + $columns = $this->normalizeColumnNames($columns); + $columnSchemas = $this->schema->getTableSchema($table)?->getColumns() ?? []; /** * @psalm-var array $columns * @psalm-var mixed $value */ foreach ($columns as $name => $value) { - /** @psalm-var mixed $value */ - $value = isset($columnSchemas[$name]) ? $columnSchemas[$name]->dbTypecast($value) : $value; + if (isset($columnSchemas[$name])) { + /** @psalm-var mixed $value */ + $value = $columnSchemas[$name]->dbTypecast($value); + } + if ($value instanceof ExpressionInterface) { $placeholder = $this->queryBuilder->buildExpression($value, $params); } else { @@ -272,7 +270,7 @@ protected function prepareUpdateSets(string $table, array $columns, array $param } /** - * Prepare column names and placeholders for "upsert" operation. + * Prepare column names and constraints for "upsert" operation. * * @throws Exception * @throws InvalidArgumentException @@ -281,6 +279,9 @@ protected function prepareUpdateSets(string $table, array $columns, array $param * @throws NotSupportedException * * @psalm-param Constraint[] $constraints + * + * @return array Array of unique, insert and update quoted column names. + * @psalm-return array{0: string[], 1: string[], 2: string[]|null} */ protected function prepareUpsertColumns( string $table, @@ -288,42 +289,29 @@ protected function prepareUpsertColumns( QueryInterface|bool|array $updateColumns, array &$constraints = [] ): array { - $insertNames = []; - - if (!$insertColumns instanceof QueryInterface) { - $insertColumns = $this->normalizeColumnNames($table, $insertColumns); - } - - if (is_array($updateColumns)) { - $updateColumns = $this->normalizeColumnNames($table, $updateColumns); - } - if ($insertColumns instanceof QueryInterface) { - /** @psalm-var list $insertNames */ [$insertNames] = $this->prepareInsertSelectSubQuery($insertColumns); } else { - /** @psalm-var array $insertColumns */ - foreach ($insertColumns as $key => $_value) { - $insertNames[] = $this->quoter->quoteColumnName($key); + /** @psalm-var array $insertColumns */ + $insertNames = $this->getNormalizeColumnNames(array_keys($insertColumns)); + + foreach ($insertNames as $i => $name) { + $insertNames[$i] = $this->quoter->quoteColumnName($name); } } /** @psalm-var string[] $uniqueNames */ $uniqueNames = $this->getTableUniqueColumnNames($table, $insertNames, $constraints); - foreach ($uniqueNames as $key => $name) { - $insertNames[$key] = $this->quoter->quoteColumnName($name); - } - - if ($updateColumns !== true) { - return [$uniqueNames, $insertNames, null]; + if ($updateColumns === true) { + return [$uniqueNames, $insertNames, array_diff($insertNames, $uniqueNames)]; } - return [$uniqueNames, $insertNames, array_diff($insertNames, $uniqueNames)]; + return [$uniqueNames, $insertNames, null]; } /** - * Returns all column names belonging to constraints enforcing uniqueness (`PRIMARY KEY`, `UNIQUE INDEX`, etc.) + * Returns all quoted column names belonging to constraints enforcing uniqueness (`PRIMARY KEY`, `UNIQUE INDEX`, etc.) * for the named table removing constraints which didn't cover the specified column list. * * The column list will be unique by column names. @@ -335,7 +323,7 @@ protected function prepareUpsertColumns( * * @throws JsonException * - * @return array The column list. + * @return array The quoted column names. * * @psalm-param Constraint[] $constraints */ @@ -347,7 +335,9 @@ private function getTableUniqueColumnNames(string $name, array $columns, array & $constraints[] = $primaryKey; } + // TODO remove getTableIndexes(), getTableUniques() should be enough /** @psalm-var IndexConstraint[] $tableIndexes */ +/* $tableIndexes = $this->schema->getTableIndexes($name); foreach ($tableIndexes as $constraint) { @@ -355,7 +345,7 @@ private function getTableUniqueColumnNames(string $name, array $columns, array & $constraints[] = $constraint; } } - +*/ $constraints = array_merge($constraints, $this->schema->getTableUniques($name)); /** @@ -365,9 +355,8 @@ private function getTableUniqueColumnNames(string $name, array $columns, array & */ $constraints = array_combine( array_map( - static function (Constraint $constraint) { - $columns = $constraint->getColumnNames() ?? []; - $columns = is_array($columns) ? $columns : [$columns]; + static function (Constraint $constraint): string { + $columns = (array) $constraint->getColumnNames(); sort($columns, SORT_STRING); return json_encode($columns, JSON_THROW_ON_ERROR); }, @@ -383,18 +372,16 @@ static function (Constraint $constraint) { $constraints = array_values( array_filter( $constraints, - static function (Constraint $constraint) use ($quoter, $columns, &$columnNames) { - /** @psalm-var string[]|string $getColumnNames */ - $getColumnNames = $constraint->getColumnNames() ?? []; + static function (Constraint $constraint) use ($quoter, $columns, &$columnNames): bool { + /** @psalm-var string[] $getColumnNames */ + $getColumnNames = (array) $constraint->getColumnNames(); $constraintColumnNames = []; - if (is_array($getColumnNames)) { - foreach ($getColumnNames as $columnName) { - $constraintColumnNames[] = $quoter->quoteColumnName($columnName); - } + foreach ($getColumnNames as $columnName) { + $constraintColumnNames[] = $quoter->quoteColumnName($columnName); } - $result = !array_diff($constraintColumnNames, $columns); + $result = empty(array_diff($constraintColumnNames, $columns)); if ($result) { $columnNames = array_merge((array) $columnNames, $constraintColumnNames); @@ -405,78 +392,40 @@ static function (Constraint $constraint) use ($quoter, $columns, &$columnNames) ) ); - /** @psalm-var Constraint[] $columnNames */ + /** @psalm-var string[] $columnNames */ return array_unique($columnNames); } /** - * @return mixed The typecast value of the given column. - */ - protected function getTypecastValue(mixed $value, ColumnSchemaInterface $columnSchema = null): mixed - { - if ($columnSchema) { - return $columnSchema->dbTypecast($value); - } - - return $value; - } - - /** - * Normalizes the column names for the given table. + * Normalizes the column names. * - * @param string $table The table to save the data into. - * @param array $columns The column data (name => value) to save into the table or instance of - * {@see QueryInterface} to perform `INSERT INTO ... SELECT` SQL statement. Passing of {@see QueryInterface}. + * @param array $columns The column data (name => value). * * @return array The normalized column names (name => value). */ - protected function normalizeColumnNames(string $table, array $columns): array + protected function normalizeColumnNames(array $columns): array { /** @var string[] $columnList */ $columnList = array_keys($columns); - $mappedNames = $this->getNormalizeColumnNames($table, $columnList); - - /** @psalm-var array $normalizedColumns */ - $normalizedColumns = []; - - /** - * @psalm-var string $name - * @psalm-var mixed $value - */ - foreach ($columns as $name => $value) { - $mappedName = $mappedNames[$name] ?? $name; - /** @psalm-var mixed */ - $normalizedColumns[$mappedName] = $value; - } + $normalizedNames = $this->getNormalizeColumnNames($columnList); - return $normalizedColumns; + return array_combine($normalizedNames, $columns); } /** - * Get a map of normalized columns + * Get normalized column names * - * @param string $table The table to save the data into. - * @param string[] $columns The column data (name => value) to save into the table or instance of - * {@see QueryInterface} to perform `INSERT INTO ... SELECT` SQL statement. Passing of {@see QueryInterface}. + * @param string[] $columns The column names. * - * @return string[] Map of normalized columns. + * @return string[] Normalized column names. */ - protected function getNormalizeColumnNames(string $table, array $columns): array + protected function getNormalizeColumnNames(array $columns): array { $normalizedNames = []; - $rawTableName = $this->schema->getRawTableName($table); foreach ($columns as $name) { - $parts = $this->quoter->getTableNameParts($name, true); - - if (count($parts) === 2 && $this->schema->getRawTableName($parts[0]) === $rawTableName) { - $normalizedName = $parts[count($parts) - 1]; - } else { - $normalizedName = $name; - } - $normalizedName = $this->quoter->ensureColumnName($normalizedName); - - $normalizedNames[$name] = $normalizedName; + $normalizedName = $this->quoter->ensureColumnName($name); + $normalizedNames[] = $this->quoter->unquoteSimpleColumnName($normalizedName); } return $normalizedNames; diff --git a/src/QueryBuilder/DMLQueryBuilderInterface.php b/src/QueryBuilder/DMLQueryBuilderInterface.php index 060682381..0da3da0e8 100644 --- a/src/QueryBuilder/DMLQueryBuilderInterface.php +++ b/src/QueryBuilder/DMLQueryBuilderInterface.php @@ -33,7 +33,7 @@ interface DMLQueryBuilderInterface * ``` * * @param string $table The table to insert new rows into. - * @param string[] $columns The column names. + * @param string[] $columns The column names of the table. * @param Generator|iterable $rows The rows to batch-insert into the table. * @param array $params The binding parameters. This parameter exists. * @@ -149,7 +149,7 @@ public function resetSequence(string $table, int|string|null $value = null): str * ``` * * @param string $table The table to update. - * @param array $columns The column data (name => value) to update. + * @param array $columns The column data (name => value) to update the table. * @param array|string $condition The condition to put in the `WHERE` part. Please refer to * {@see Query::where()} On how to specify condition. * @param array $params The binding parameters that will be modified by this method so that they can be bound to diff --git a/tests/Provider/CommandProvider.php b/tests/Provider/CommandProvider.php index 7f6999d80..61aa84000 100644 --- a/tests/Provider/CommandProvider.php +++ b/tests/Provider/CommandProvider.php @@ -301,16 +301,10 @@ public static function batchInsert(): array ':qp3' => true, ], ], - 'wrongBehavior' => [ + 'table name with column name with brackets' => [ '{{%type}}', ['{{%type}}.[[int_col]]', '[[float_col]]', 'char_col', 'bool_col'], 'values' => [['0', '0.0', 'Kyiv {{city}}, Ukraine', false]], - /** - * Test covers potentially wrong behavior and marks it as expected!. - * - * In case table name or table column is passed with curly or square bracelets, QueryBuilder can not - * determine the table schema and typecast values properly. - */ 'expected' => DbHelper::replaceQuotes( << [ - ':qp0' => '0', - ':qp1' => '0.0', + ':qp0' => 0, + ':qp1' => 0.0, ':qp2' => 'Kyiv {{city}}, Ukraine', ':qp3' => false, ], diff --git a/tests/Provider/QueryBuilderProvider.php b/tests/Provider/QueryBuilderProvider.php index f86d147d9..0f09d5866 100644 --- a/tests/Provider/QueryBuilderProvider.php +++ b/tests/Provider/QueryBuilderProvider.php @@ -213,6 +213,18 @@ public static function batchInsert(): array ), [':qp0' => null], ], + 'column table names are not checked' => [ + '{{%type}}', + ['{{%type}}.[[bool_col]]', '{{%another_table}}.[[bool_col2]]'], + [[true, false]], + 'expected' => DbHelper::replaceQuotes( + << null, ':qp1' => null], + ], 'empty-sql' => [ '{{%type}}', [], @@ -1107,6 +1119,13 @@ public static function upsert(): array '', [':qp0' => 'test@example.com', ':qp1' => 'bar {{city}}', ':qp2' => 1, ':qp3' => null], ], + 'regular values with unique at not the first position' => [ + 'T_upsert', + ['address' => 'bar {{city}}', 'email' => 'test@example.com', 'status' => 1, 'profile_id' => null], + true, + '', + [':qp0' => 'bar {{city}}', ':qp1' => 'test@example.com', ':qp2' => 1, ':qp3' => null], + ], 'regular values with update part' => [ 'T_upsert', ['email' => 'test@example.com', 'address' => 'bar {{city}}', 'status' => 1, 'profile_id' => null],