From 7145e81944525e1deb77c066507bd5b329c9ad4f Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sat, 19 Aug 2023 00:03:30 +0700 Subject: [PATCH 01/18] Refactor DMLQueryBuilder --- src/QueryBuilder/AbstractDMLQueryBuilder.php | 235 +++++++----------- src/QueryBuilder/DMLQueryBuilderInterface.php | 4 +- tests/Provider/CommandProvider.php | 12 +- tests/Provider/QueryBuilderProvider.php | 19 ++ 4 files changed, 116 insertions(+), 154 deletions(-) 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], From dbe39571874ed187094d90a50f68c3ee16b8803e Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sat, 19 Aug 2023 16:43:25 +0700 Subject: [PATCH 02/18] Get uniques using `getTableIndexes()` and `getTableUniques()` --- src/QueryBuilder/AbstractDMLQueryBuilder.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/QueryBuilder/AbstractDMLQueryBuilder.php b/src/QueryBuilder/AbstractDMLQueryBuilder.php index f8e9f728a..ea9a2402f 100644 --- a/src/QueryBuilder/AbstractDMLQueryBuilder.php +++ b/src/QueryBuilder/AbstractDMLQueryBuilder.php @@ -4,7 +4,6 @@ namespace Yiisoft\Db\QueryBuilder; -use Generator; use JsonException; use Yiisoft\Db\Constraint\Constraint; use Yiisoft\Db\Constraint\IndexConstraint; @@ -335,9 +334,7 @@ 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) { @@ -345,7 +342,7 @@ private function getTableUniqueColumnNames(string $name, array $columns, array & $constraints[] = $constraint; } } -*/ + $constraints = array_merge($constraints, $this->schema->getTableUniques($name)); /** From 57ece138c52b67f234109b3862c33cefaa721c45 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sat, 19 Aug 2023 18:32:44 +0700 Subject: [PATCH 03/18] Fix @psalm-var --- src/QueryBuilder/AbstractDMLQueryBuilder.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/QueryBuilder/AbstractDMLQueryBuilder.php b/src/QueryBuilder/AbstractDMLQueryBuilder.php index ea9a2402f..c3c131821 100644 --- a/src/QueryBuilder/AbstractDMLQueryBuilder.php +++ b/src/QueryBuilder/AbstractDMLQueryBuilder.php @@ -58,9 +58,10 @@ public function batchInsert(string $table, array $columns, iterable $rows, array $columns = $this->getNormalizeColumnNames($columns); $columnSchemas = $this->schema->getTableSchema($table)?->getColumns() ?? []; - /** @psalm-var array> $rows */ + /** @psalm-var array[] $rows */ foreach ($rows as $row) { $placeholders = []; + /** @psalm-var mixed $value */ foreach ($row as $i => $value) { if (isset($columns[$i], $columnSchemas[$columns[$i]])) { /** @psalm-var mixed $value */ From de9866553d1846b69150d5d571aea3ea3e800602 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sat, 19 Aug 2023 19:39:03 +0700 Subject: [PATCH 04/18] Fix #61 (point 2) --- src/QueryBuilder/AbstractDMLQueryBuilder.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/QueryBuilder/AbstractDMLQueryBuilder.php b/src/QueryBuilder/AbstractDMLQueryBuilder.php index c3c131821..62016aabe 100644 --- a/src/QueryBuilder/AbstractDMLQueryBuilder.php +++ b/src/QueryBuilder/AbstractDMLQueryBuilder.php @@ -60,9 +60,10 @@ public function batchInsert(string $table, array $columns, iterable $rows, array /** @psalm-var array[] $rows */ foreach ($rows as $row) { + $i = 0; $placeholders = []; /** @psalm-var mixed $value */ - foreach ($row as $i => $value) { + foreach ($row as $value) { if (isset($columns[$i], $columnSchemas[$columns[$i]])) { /** @psalm-var mixed $value */ $value = $columnSchemas[$columns[$i]]->dbTypecast($value); @@ -73,6 +74,8 @@ public function batchInsert(string $table, array $columns, iterable $rows, array } else { $placeholders[] = $this->queryBuilder->bindParam($value, $params); } + + ++$i; } $values[] = '(' . implode(', ', $placeholders) . ')'; } From ef2fa1b2595ed8729b87bb9c7e4b19c9bf00d0c4 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sun, 20 Aug 2023 07:49:32 +0700 Subject: [PATCH 05/18] Fix #61 (point 2) add test --- tests/Provider/QueryBuilderProvider.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/Provider/QueryBuilderProvider.php b/tests/Provider/QueryBuilderProvider.php index 0f09d5866..fb4bf7450 100644 --- a/tests/Provider/QueryBuilderProvider.php +++ b/tests/Provider/QueryBuilderProvider.php @@ -235,6 +235,18 @@ public static function batchInsert(): array })(), '', ], + 'with associative values' => [ + 'type', + ['int_col', 'int_col2'], + [['first' => 1, 'second' => 2]], + 'expected' => DbHelper::replaceQuotes( + << 1, ':qp1' => 2], + ], ]; } From cab52c58fe3b373c8f44105bd411d499dd268f6b Mon Sep 17 00:00:00 2001 From: Tigrov Date: Mon, 21 Aug 2023 21:37:26 +0700 Subject: [PATCH 06/18] Improve test --- tests/Provider/CommandProvider.php | 17 +++++++++++++++++ tests/Provider/QueryBuilderProvider.php | 12 ------------ 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/tests/Provider/CommandProvider.php b/tests/Provider/CommandProvider.php index 61aa84000..5d2ae6176 100644 --- a/tests/Provider/CommandProvider.php +++ b/tests/Provider/CommandProvider.php @@ -339,6 +339,23 @@ public static function batchInsert(): array ':qp3' => false, ], ], + 'with associative values' => [ + 'type', + ['int_col', 'float_col', 'char_col', 'bool_col'], + 'values' => [['int' => '1.0', 'float' => '2', 'char' => 10, 'bool' => 1]], + 'expected' => DbHelper::replaceQuotes( + << [ + ':qp0' => 1, + ':qp1' => 2.0, + ':qp2' => '10', + ':qp3' => true, + ], + ], ]; } diff --git a/tests/Provider/QueryBuilderProvider.php b/tests/Provider/QueryBuilderProvider.php index fb4bf7450..0f09d5866 100644 --- a/tests/Provider/QueryBuilderProvider.php +++ b/tests/Provider/QueryBuilderProvider.php @@ -235,18 +235,6 @@ public static function batchInsert(): array })(), '', ], - 'with associative values' => [ - 'type', - ['int_col', 'int_col2'], - [['first' => 1, 'second' => 2]], - 'expected' => DbHelper::replaceQuotes( - << 1, ':qp1' => 2], - ], ]; } From 230859a5ce7741322fe008cb9fc162ca5f6d685a Mon Sep 17 00:00:00 2001 From: Tigrov Date: Tue, 22 Aug 2023 08:46:30 +0700 Subject: [PATCH 07/18] Remove methods with `NotSupportedException` --- src/QueryBuilder/AbstractDMLQueryBuilder.php | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/src/QueryBuilder/AbstractDMLQueryBuilder.php b/src/QueryBuilder/AbstractDMLQueryBuilder.php index 62016aabe..da61ffd07 100644 --- a/src/QueryBuilder/AbstractDMLQueryBuilder.php +++ b/src/QueryBuilder/AbstractDMLQueryBuilder.php @@ -109,16 +109,6 @@ public function insert(string $table, QueryInterface|array $columns, array &$par . (!empty($placeholders) ? ' VALUES (' . implode(', ', $placeholders) . ')' : ' ' . $values); } - public function insertWithReturningPks(string $table, QueryInterface|array $columns, array &$params = []): string - { - throw new NotSupportedException(__METHOD__ . '() is not supported by this DBMS.'); - } - - public function resetSequence(string $table, int|string|null $value = null): string - { - throw new NotSupportedException(__METHOD__ . '() is not supported by this DBMS.'); - } - public function update(string $table, array $columns, array|string $condition, array &$params = []): string { [$lines, $params] = $this->prepareUpdateSets($table, $columns, $params); @@ -129,15 +119,6 @@ public function update(string $table, array $columns, array|string $condition, a return $where === '' ? $sql : $sql . ' ' . $where; } - public function upsert( - string $table, - QueryInterface|array $insertColumns, - bool|array $updateColumns, - array &$params - ): string { - throw new NotSupportedException(__METHOD__ . ' is not supported by this DBMS.'); - } - /** * Prepare select-subQuery and field names for `INSERT INTO ... SELECT` SQL statement. * From a6a7b64afc6589ea4394afb75b7511d6b1271a97 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Tue, 22 Aug 2023 08:59:11 +0700 Subject: [PATCH 08/18] Fix test issues --- tests/Support/Stub/DMLQueryBuilder.php | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/Support/Stub/DMLQueryBuilder.php b/tests/Support/Stub/DMLQueryBuilder.php index a91c17548..a3b5ebfe4 100644 --- a/tests/Support/Stub/DMLQueryBuilder.php +++ b/tests/Support/Stub/DMLQueryBuilder.php @@ -4,8 +4,28 @@ namespace Yiisoft\Db\Tests\Support\Stub; +use Yiisoft\Db\Exception\NotSupportedException; +use Yiisoft\Db\Query\QueryInterface; use Yiisoft\Db\QueryBuilder\AbstractDMLQueryBuilder; final class DMLQueryBuilder extends AbstractDMLQueryBuilder { + public function insertWithReturningPks(string $table, QueryInterface|array $columns, array &$params = []): string + { + throw new NotSupportedException(__METHOD__ . '() is not supported by this DBMS.'); + } + + public function resetSequence(string $table, int|string|null $value = null): string + { + throw new NotSupportedException(__METHOD__ . '() is not supported by this DBMS.'); + } + + public function upsert( + string $table, + QueryInterface|array $insertColumns, + bool|array $updateColumns, + array &$params + ): string { + throw new NotSupportedException(__METHOD__ . '() is not supported by this DBMS.'); + } } From adfae64c0678a9b3ad1eef3de204a276c90cd541 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Tue, 22 Aug 2023 09:22:49 +0700 Subject: [PATCH 09/18] Fix test issues --- tests/AbstractQueryBuilderTest.php | 4 ++++ tests/Db/Command/CommandTest.php | 4 ++++ tests/Db/QueryBuilder/QueryBuilderTest.php | 8 ++++++++ 3 files changed, 16 insertions(+) diff --git a/tests/AbstractQueryBuilderTest.php b/tests/AbstractQueryBuilderTest.php index 6a50636e9..bbf2cbcd8 100644 --- a/tests/AbstractQueryBuilderTest.php +++ b/tests/AbstractQueryBuilderTest.php @@ -1918,6 +1918,8 @@ public function testResetSequenceNoAssociatedException(): void $qb = $db->getQueryBuilder(); if ($db->getDriverName() === 'db') { + self::markTestSkipped('AbstractDMLQueryBuilder::resetSequence() was removed'); + $this->expectException(NotSupportedException::class); $this->expectExceptionMessage( 'Yiisoft\Db\QueryBuilder\AbstractDMLQueryBuilder::resetSequence() is not supported by this DBMS.' @@ -1942,6 +1944,8 @@ public function testResetSequenceTableNoExistException(): void $qb = $db->getQueryBuilder(); if ($db->getDriverName() === 'db') { + self::markTestSkipped('AbstractDMLQueryBuilder::resetSequence() was removed'); + $this->expectException(NotSupportedException::class); $this->expectExceptionMessage( 'Yiisoft\Db\QueryBuilder\AbstractDMLQueryBuilder::resetSequence() is not supported by this DBMS.' diff --git a/tests/Db/Command/CommandTest.php b/tests/Db/Command/CommandTest.php index d361a9ab8..4fd6bee41 100644 --- a/tests/Db/Command/CommandTest.php +++ b/tests/Db/Command/CommandTest.php @@ -645,6 +645,8 @@ public function testRenameTable(): void public function testResetSequence(): void { + self::markTestSkipped('AbstractDMLQueryBuilder::resetSequence() was removed'); + $db = $this->getConnection(); $this->expectException(NotSupportedException::class); @@ -700,6 +702,8 @@ public function testUpdate(): void public function testUpsert(): void { + self::markTestSkipped('AbstractDMLQueryBuilder::upsert() was removed'); + $db = $this->getConnection(); $command = $db->createCommand(); diff --git a/tests/Db/QueryBuilder/QueryBuilderTest.php b/tests/Db/QueryBuilder/QueryBuilderTest.php index 79a1e382b..51445185a 100644 --- a/tests/Db/QueryBuilder/QueryBuilderTest.php +++ b/tests/Db/QueryBuilder/QueryBuilderTest.php @@ -209,6 +209,8 @@ public function testInsertWithReturningPks( string $expectedSQL, array $expectedParams ): void { + self::markTestSkipped('AbstractDMLQueryBuilder::insertWithReturningPks() was removed'); + $db = $this->getConnection(); $qb = $db->getQueryBuilder(); @@ -226,6 +228,8 @@ public function testInsertWithReturningPks( */ public function testResetSequence(): void { + self::markTestSkipped('AbstractDMLQueryBuilder::resetSequence() was removed'); + $db = $this->getConnection(); $qb = $db->getQueryBuilder(); @@ -274,6 +278,8 @@ public function testUpsert( string|array $expectedSQL, array $expectedParams ): void { + self::markTestSkipped('AbstractDMLQueryBuilder::upsert() was removed'); + $db = $this->getConnection(); $actualParams = []; @@ -294,6 +300,8 @@ public function testUpsertExecute( array|QueryInterface $insertColumns, array|bool $updateColumns ): void { + self::markTestSkipped('AbstractDMLQueryBuilder::upsert() was removed'); + $db = $this->getConnection(); $this->expectException(NotSupportedException::class); From d07b871e23a5ede7716181a606d3966bcf3ab84c Mon Sep 17 00:00:00 2001 From: Tigrov Date: Tue, 22 Aug 2023 09:46:26 +0700 Subject: [PATCH 10/18] Revert "Remove methods with `NotSupportedException`" --- src/QueryBuilder/AbstractDMLQueryBuilder.php | 19 +++++++++++++++++++ tests/AbstractQueryBuilderTest.php | 4 ---- tests/Db/Command/CommandTest.php | 4 ---- tests/Db/QueryBuilder/QueryBuilderTest.php | 8 -------- tests/Support/Stub/DMLQueryBuilder.php | 20 -------------------- 5 files changed, 19 insertions(+), 36 deletions(-) diff --git a/src/QueryBuilder/AbstractDMLQueryBuilder.php b/src/QueryBuilder/AbstractDMLQueryBuilder.php index da61ffd07..62016aabe 100644 --- a/src/QueryBuilder/AbstractDMLQueryBuilder.php +++ b/src/QueryBuilder/AbstractDMLQueryBuilder.php @@ -109,6 +109,16 @@ public function insert(string $table, QueryInterface|array $columns, array &$par . (!empty($placeholders) ? ' VALUES (' . implode(', ', $placeholders) . ')' : ' ' . $values); } + public function insertWithReturningPks(string $table, QueryInterface|array $columns, array &$params = []): string + { + throw new NotSupportedException(__METHOD__ . '() is not supported by this DBMS.'); + } + + public function resetSequence(string $table, int|string|null $value = null): string + { + throw new NotSupportedException(__METHOD__ . '() is not supported by this DBMS.'); + } + public function update(string $table, array $columns, array|string $condition, array &$params = []): string { [$lines, $params] = $this->prepareUpdateSets($table, $columns, $params); @@ -119,6 +129,15 @@ public function update(string $table, array $columns, array|string $condition, a return $where === '' ? $sql : $sql . ' ' . $where; } + public function upsert( + string $table, + QueryInterface|array $insertColumns, + bool|array $updateColumns, + array &$params + ): string { + throw new NotSupportedException(__METHOD__ . ' is not supported by this DBMS.'); + } + /** * Prepare select-subQuery and field names for `INSERT INTO ... SELECT` SQL statement. * diff --git a/tests/AbstractQueryBuilderTest.php b/tests/AbstractQueryBuilderTest.php index bbf2cbcd8..6a50636e9 100644 --- a/tests/AbstractQueryBuilderTest.php +++ b/tests/AbstractQueryBuilderTest.php @@ -1918,8 +1918,6 @@ public function testResetSequenceNoAssociatedException(): void $qb = $db->getQueryBuilder(); if ($db->getDriverName() === 'db') { - self::markTestSkipped('AbstractDMLQueryBuilder::resetSequence() was removed'); - $this->expectException(NotSupportedException::class); $this->expectExceptionMessage( 'Yiisoft\Db\QueryBuilder\AbstractDMLQueryBuilder::resetSequence() is not supported by this DBMS.' @@ -1944,8 +1942,6 @@ public function testResetSequenceTableNoExistException(): void $qb = $db->getQueryBuilder(); if ($db->getDriverName() === 'db') { - self::markTestSkipped('AbstractDMLQueryBuilder::resetSequence() was removed'); - $this->expectException(NotSupportedException::class); $this->expectExceptionMessage( 'Yiisoft\Db\QueryBuilder\AbstractDMLQueryBuilder::resetSequence() is not supported by this DBMS.' diff --git a/tests/Db/Command/CommandTest.php b/tests/Db/Command/CommandTest.php index 4fd6bee41..d361a9ab8 100644 --- a/tests/Db/Command/CommandTest.php +++ b/tests/Db/Command/CommandTest.php @@ -645,8 +645,6 @@ public function testRenameTable(): void public function testResetSequence(): void { - self::markTestSkipped('AbstractDMLQueryBuilder::resetSequence() was removed'); - $db = $this->getConnection(); $this->expectException(NotSupportedException::class); @@ -702,8 +700,6 @@ public function testUpdate(): void public function testUpsert(): void { - self::markTestSkipped('AbstractDMLQueryBuilder::upsert() was removed'); - $db = $this->getConnection(); $command = $db->createCommand(); diff --git a/tests/Db/QueryBuilder/QueryBuilderTest.php b/tests/Db/QueryBuilder/QueryBuilderTest.php index 51445185a..79a1e382b 100644 --- a/tests/Db/QueryBuilder/QueryBuilderTest.php +++ b/tests/Db/QueryBuilder/QueryBuilderTest.php @@ -209,8 +209,6 @@ public function testInsertWithReturningPks( string $expectedSQL, array $expectedParams ): void { - self::markTestSkipped('AbstractDMLQueryBuilder::insertWithReturningPks() was removed'); - $db = $this->getConnection(); $qb = $db->getQueryBuilder(); @@ -228,8 +226,6 @@ public function testInsertWithReturningPks( */ public function testResetSequence(): void { - self::markTestSkipped('AbstractDMLQueryBuilder::resetSequence() was removed'); - $db = $this->getConnection(); $qb = $db->getQueryBuilder(); @@ -278,8 +274,6 @@ public function testUpsert( string|array $expectedSQL, array $expectedParams ): void { - self::markTestSkipped('AbstractDMLQueryBuilder::upsert() was removed'); - $db = $this->getConnection(); $actualParams = []; @@ -300,8 +294,6 @@ public function testUpsertExecute( array|QueryInterface $insertColumns, array|bool $updateColumns ): void { - self::markTestSkipped('AbstractDMLQueryBuilder::upsert() was removed'); - $db = $this->getConnection(); $this->expectException(NotSupportedException::class); diff --git a/tests/Support/Stub/DMLQueryBuilder.php b/tests/Support/Stub/DMLQueryBuilder.php index a3b5ebfe4..a91c17548 100644 --- a/tests/Support/Stub/DMLQueryBuilder.php +++ b/tests/Support/Stub/DMLQueryBuilder.php @@ -4,28 +4,8 @@ namespace Yiisoft\Db\Tests\Support\Stub; -use Yiisoft\Db\Exception\NotSupportedException; -use Yiisoft\Db\Query\QueryInterface; use Yiisoft\Db\QueryBuilder\AbstractDMLQueryBuilder; final class DMLQueryBuilder extends AbstractDMLQueryBuilder { - public function insertWithReturningPks(string $table, QueryInterface|array $columns, array &$params = []): string - { - throw new NotSupportedException(__METHOD__ . '() is not supported by this DBMS.'); - } - - public function resetSequence(string $table, int|string|null $value = null): string - { - throw new NotSupportedException(__METHOD__ . '() is not supported by this DBMS.'); - } - - public function upsert( - string $table, - QueryInterface|array $insertColumns, - bool|array $updateColumns, - array &$params - ): string { - throw new NotSupportedException(__METHOD__ . '() is not supported by this DBMS.'); - } } From a26b3acb277bf0318d3ff5698099b8fa7687485d Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sat, 26 Aug 2023 20:11:13 +0700 Subject: [PATCH 11/18] Add line to CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3691faf4c..cf3960422 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## 1.1.2 under development -- no changes in this release. +- Bug #746: Refactor `DMLQueryBuilder` and fix: unique indexes in `upsert()`, column names with table name and brackets, `batchInsert()` with associative arrays, enhanced documentation of `batchInsert()` and `update()` (@Tigrov) ## 1.1.1 August 16, 2023 From ae66957dbe5557b6c94c4a5e61dec6fa315a2a36 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Wed, 6 Sep 2023 10:01:06 +0700 Subject: [PATCH 12/18] Change order of checks --- src/QueryBuilder/AbstractDMLQueryBuilder.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/QueryBuilder/AbstractDMLQueryBuilder.php b/src/QueryBuilder/AbstractDMLQueryBuilder.php index 62016aabe..8a17d86ba 100644 --- a/src/QueryBuilder/AbstractDMLQueryBuilder.php +++ b/src/QueryBuilder/AbstractDMLQueryBuilder.php @@ -198,15 +198,15 @@ protected function prepareInsertSelectSubQuery(QueryInterface $columns, array $p */ protected function prepareInsertValues(string $table, array|QueryInterface $columns, array $params = []): array { + if (empty($columns)) { + return [[], [], 'DEFAULT VALUES', []]; + } + if ($columns instanceof QueryInterface) { [$names, $values, $params] = $this->prepareInsertSelectSubQuery($columns, $params); return [$names, [], $values, $params]; } - if (empty($columns)) { - return [[], [], 'DEFAULT VALUES', []]; - } - $names = []; $placeholders = []; $columns = $this->normalizeColumnNames($columns); From b82cb148d48a082c9d9c758fab87193b4776801f Mon Sep 17 00:00:00 2001 From: Tigrov Date: Wed, 4 Oct 2023 14:22:42 +0700 Subject: [PATCH 13/18] Improve performance of quoting column names up to 10% using `array_map()` --- src/QueryBuilder/AbstractDMLQueryBuilder.php | 32 +++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/src/QueryBuilder/AbstractDMLQueryBuilder.php b/src/QueryBuilder/AbstractDMLQueryBuilder.php index 8a17d86ba..1d6ea3f49 100644 --- a/src/QueryBuilder/AbstractDMLQueryBuilder.php +++ b/src/QueryBuilder/AbstractDMLQueryBuilder.php @@ -84,9 +84,10 @@ public function batchInsert(string $table, array $columns, iterable $rows, array return ''; } - foreach ($columns as $i => $name) { - $columns[$i] = $this->quoter->quoteColumnName($name); - } + $columns = array_map( + [$this->quoter, 'quoteColumnName'], + $columns, + ); return 'INSERT INTO ' . $this->quoter->quoteTableName($table) . ' (' . implode(', ', $columns) . ') VALUES ' . implode(', ', $values); @@ -298,9 +299,10 @@ protected function prepareUpsertColumns( /** @psalm-var array $insertColumns */ $insertNames = $this->getNormalizeColumnNames(array_keys($insertColumns)); - foreach ($insertNames as $i => $name) { - $insertNames[$i] = $this->quoter->quoteColumnName($name); - } + $insertNames = array_map( + [$this->quoter, 'quoteColumnName'], + $insertNames, + ); } /** @psalm-var string[] $uniqueNames */ @@ -374,13 +376,13 @@ static function (Constraint $constraint): string { array_filter( $constraints, static function (Constraint $constraint) use ($quoter, $columns, &$columnNames): bool { - /** @psalm-var string[] $getColumnNames */ - $getColumnNames = (array) $constraint->getColumnNames(); - $constraintColumnNames = []; + /** @psalm-var string[] $constraintColumnNames */ + $constraintColumnNames = (array) $constraint->getColumnNames(); - foreach ($getColumnNames as $columnName) { - $constraintColumnNames[] = $quoter->quoteColumnName($columnName); - } + $constraintColumnNames = array_map( + [$quoter, 'quoteColumnName'], + $constraintColumnNames, + ); $result = empty(array_diff($constraintColumnNames, $columns)); @@ -406,9 +408,9 @@ static function (Constraint $constraint) use ($quoter, $columns, &$columnNames): */ protected function normalizeColumnNames(array $columns): array { - /** @var string[] $columnList */ - $columnList = array_keys($columns); - $normalizedNames = $this->getNormalizeColumnNames($columnList); + /** @var string[] $columnNames */ + $columnNames = array_keys($columns); + $normalizedNames = $this->getNormalizeColumnNames($columnNames); return array_combine($normalizedNames, $columns); } From 13c2b9728f32cf2ec3eceda3bf506198c2316409 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sat, 14 Oct 2023 14:49:25 +0700 Subject: [PATCH 14/18] Fix `batchInsert()` with associative arrays --- src/QueryBuilder/AbstractDMLQueryBuilder.php | 68 ++++++++++++---- tests/Provider/CommandProvider.php | 86 +++++++++++++++++++- 2 files changed, 137 insertions(+), 17 deletions(-) diff --git a/src/QueryBuilder/AbstractDMLQueryBuilder.php b/src/QueryBuilder/AbstractDMLQueryBuilder.php index 1d6ea3f49..6feebd40b 100644 --- a/src/QueryBuilder/AbstractDMLQueryBuilder.php +++ b/src/QueryBuilder/AbstractDMLQueryBuilder.php @@ -18,17 +18,22 @@ use function array_combine; use function array_diff; +use function array_fill_keys; use function array_filter; +use function array_key_first; use function array_keys; use function array_map; use function array_merge; +use function array_slice; use function array_unique; use function array_values; use function implode; use function in_array; +use function is_array; use function is_string; use function json_encode; use function preg_match; +use function reset; use function sort; /** @@ -58,25 +63,58 @@ public function batchInsert(string $table, array $columns, iterable $rows, array $columns = $this->getNormalizeColumnNames($columns); $columnSchemas = $this->schema->getTableSchema($table)?->getColumns() ?? []; + if ($columns === []) { + $columnNames = []; + if (is_array($rows)) { + /** @psalm-var iterable $row */ + $row = reset($rows); + + if (is_array($row)) { + if (is_string(array_key_first($row))) { + $columnNames = array_keys($row); + } else { + $columnNames = array_slice(array_keys($columnSchemas), 0, count($row)); + } + + $columns = array_combine($columnNames, $columnNames); + } + } + } else { + $columnNames = array_values($columns); + } + + $columnKeys = array_fill_keys($columnNames, false); + /** @psalm-var array[] $rows */ foreach ($rows as $row) { $i = 0; - $placeholders = []; - /** @psalm-var mixed $value */ - foreach ($row as $value) { - if (isset($columns[$i], $columnSchemas[$columns[$i]])) { + $placeholders = $columnKeys; + /** + * @psalm-var string|int $key + * @psalm-var mixed $value + */ + foreach ($row as $key => $value) { + /** @psalm-var string|int $columnName */ + $columnName = $columns[$key] ?? ( + isset($columnKeys[$key]) + ? $key + : $columnNames[$i] ?? $i + ); + + if (isset($columnSchemas[$columnName])) { /** @psalm-var mixed $value */ - $value = $columnSchemas[$columns[$i]]->dbTypecast($value); + $value = $columnSchemas[$columnName]->dbTypecast($value); } if ($value instanceof ExpressionInterface) { - $placeholders[] = $this->queryBuilder->buildExpression($value, $params); + $placeholders[$columnName] = $this->queryBuilder->buildExpression($value, $params); } else { - $placeholders[] = $this->queryBuilder->bindParam($value, $params); + $placeholders[$columnName] = $this->queryBuilder->bindParam($value, $params); } ++$i; } + $values[] = '(' . implode(', ', $placeholders) . ')'; } @@ -84,13 +122,13 @@ public function batchInsert(string $table, array $columns, iterable $rows, array return ''; } - $columns = array_map( + $columnNames = array_map( [$this->quoter, 'quoteColumnName'], - $columns, + $columnNames, ); return 'INSERT INTO ' . $this->quoter->quoteTableName($table) - . ' (' . implode(', ', $columns) . ') VALUES ' . implode(', ', $values); + . ' (' . implode(', ', $columnNames) . ') VALUES ' . implode(', ', $values); } public function delete(string $table, array|string $condition, array &$params): string @@ -424,13 +462,11 @@ protected function normalizeColumnNames(array $columns): array */ protected function getNormalizeColumnNames(array $columns): array { - $normalizedNames = []; - - foreach ($columns as $name) { - $normalizedName = $this->quoter->ensureColumnName($name); - $normalizedNames[] = $this->quoter->unquoteSimpleColumnName($normalizedName); + foreach ($columns as &$name) { + $name = $this->quoter->ensureColumnName($name); + $name = $this->quoter->unquoteSimpleColumnName($name); } - return $normalizedNames; + return $columns; } } diff --git a/tests/Provider/CommandProvider.php b/tests/Provider/CommandProvider.php index 5d2ae6176..387fc353d 100644 --- a/tests/Provider/CommandProvider.php +++ b/tests/Provider/CommandProvider.php @@ -339,7 +339,7 @@ public static function batchInsert(): array ':qp3' => false, ], ], - 'with associative values' => [ + 'with associative values with different keys' => [ 'type', ['int_col', 'float_col', 'char_col', 'bool_col'], 'values' => [['int' => '1.0', 'float' => '2', 'char' => 10, 'bool' => 1]], @@ -356,6 +356,90 @@ public static function batchInsert(): array ':qp3' => true, ], ], + 'with associative values with different keys and columns with keys' => [ + 'type', + ['a' => 'int_col', 'b' => 'float_col', 'c' => 'char_col', 'd' => 'bool_col'], + 'values' => [['int' => '1.0', 'float' => '2', 'char' => 10, 'bool' => 1]], + 'expected' => DbHelper::replaceQuotes( + << [ + ':qp0' => 1, + ':qp1' => 2.0, + ':qp2' => '10', + ':qp3' => true, + ], + ], + 'with associative values with keys of column names' => [ + 'type', + ['int_col', 'float_col', 'char_col', 'bool_col'], + 'values' => [['bool_col' => 1, 'char_col' => 10, 'int_col' => '1.0', 'float_col' => '2']], + 'expected' => DbHelper::replaceQuotes( + << [ + ':qp0' => true, + ':qp1' => '10', + ':qp2' => 1, + ':qp3' => 2.0, + ], + ], + 'with associative values with keys of column keys' => [ + 'type', + ['int' => 'int_col', 'float' => 'float_col', 'char' => 'char_col', 'bool' => 'bool_col'], + 'values' => [['bool' => 1, 'char' => 10, 'int' => '1.0', 'float' => '2']], + 'expected' => DbHelper::replaceQuotes( + << [ + ':qp0' => true, + ':qp1' => '10', + ':qp2' => 1, + ':qp3' => 2.0, + ], + ], + 'with shuffled indexes of values' => [ + 'type', + ['int_col', 'float_col', 'char_col', 'bool_col'], + 'values' => [[3 => 1, 2 => 10, 0 => '1.0', 1 => '2']], + 'expected' => DbHelper::replaceQuotes( + << [ + ':qp0' => true, + ':qp1' => '10', + ':qp2' => 1, + ':qp3' => 2.0, + ], + ], + 'with empty columns' => [ + 'customer', + [], + 'values' => [['10.0', 'info@gmail.com']], + 'expected' => DbHelper::replaceQuotes( + << [ + ':qp0' => 10, + ':qp1' => 'info@gmail.com', + ], + 4, + ], ]; } From 5ee6fb60f698d801a97f5045724b6dd3c4cbe40c Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sat, 4 Nov 2023 14:04:31 +0700 Subject: [PATCH 15/18] Update after merge --- src/QueryBuilder/AbstractDMLQueryBuilder.php | 38 ++------------------ tests/Provider/CommandProvider.php | 16 --------- 2 files changed, 3 insertions(+), 51 deletions(-) diff --git a/src/QueryBuilder/AbstractDMLQueryBuilder.php b/src/QueryBuilder/AbstractDMLQueryBuilder.php index 13ce224c0..b29d08efd 100644 --- a/src/QueryBuilder/AbstractDMLQueryBuilder.php +++ b/src/QueryBuilder/AbstractDMLQueryBuilder.php @@ -21,21 +21,16 @@ use function array_diff; use function array_fill_keys; use function array_filter; -use function array_key_first; use function array_keys; use function array_map; use function array_merge; -use function array_slice; 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; -use function reset; use function sort; /** @@ -63,32 +58,9 @@ public function batchInsert(string $table, array $columns, iterable $rows, array $values = []; $columns = $this->getNormalizeColumnNames('', $columns); - $columnSchemas = $this->schema->getTableSchema($table)?->getColumns() ?? []; - $values = []; - $columns = $this->getNormalizeColumnNames($columns); - $columnSchemas = $this->schema->getTableSchema($table)?->getColumns() ?? []; - - if ($columns === []) { - $columnNames = []; - if (is_array($rows)) { - /** @psalm-var iterable $row */ - $row = reset($rows); - - if (is_array($row)) { - if (is_string(array_key_first($row))) { - $columnNames = array_keys($row); - } else { - $columnNames = array_slice(array_keys($columnSchemas), 0, count($row)); - } - - $columns = array_combine($columnNames, $columnNames); - } - } - } else { - $columnNames = array_values($columns); - } - + $columnNames = array_values($columns); $columnKeys = array_fill_keys($columnNames, false); + $columnSchemas = $this->schema->getTableSchema($table)?->getColumns() ?? []; foreach ($rows as $row) { $i = 0; @@ -99,11 +71,7 @@ public function batchInsert(string $table, array $columns, iterable $rows, array */ foreach ($row as $key => $value) { /** @psalm-var string|int $columnName */ - $columnName = $columns[$key] ?? ( - isset($columnKeys[$key]) - ? $key - : $columnNames[$i] ?? $i - ); + $columnName = $columns[$key] ?? (isset($columnKeys[$key]) ? $key : $columnNames[$i] ?? $i); if (isset($columnSchemas[$columnName])) { /** @psalm-var mixed $value */ diff --git a/tests/Provider/CommandProvider.php b/tests/Provider/CommandProvider.php index 387fc353d..9f7408b73 100644 --- a/tests/Provider/CommandProvider.php +++ b/tests/Provider/CommandProvider.php @@ -424,22 +424,6 @@ public static function batchInsert(): array ':qp3' => 2.0, ], ], - 'with empty columns' => [ - 'customer', - [], - 'values' => [['10.0', 'info@gmail.com']], - 'expected' => DbHelper::replaceQuotes( - << [ - ':qp0' => 10, - ':qp1' => 'info@gmail.com', - ], - 4, - ], ]; } From 2fbfc10fd462cdf98190b90cf3d1dfad20957f33 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sat, 4 Nov 2023 14:08:04 +0700 Subject: [PATCH 16/18] Remove old comments --- CHANGELOG.md | 1 - src/QueryBuilder/AbstractDMLQueryBuilder.php | 5 +---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2055c1a18..34e983ff2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,6 @@ ## 1.1.2 under development -- Bug #746: Refactor `DMLQueryBuilder` and fix: unique indexes in `upsert()`, column names with table name and brackets, `batchInsert()` with associative arrays, enhanced documentation of `batchInsert()` and `update()` (@Tigrov) - Enh #746: Refactor `AbstractDMLQueryBuilder` (@Tigrov) - Bug #746: Fix `AbstractDMLQueryBuilder::upsert()` when unique index is not at the first position of inserted values (@Tigrov) - Bug #746: Typecast values in `AbstractDMLQueryBuilder::batchInsert()` if column names with table name and brackets (@Tigrov) diff --git a/src/QueryBuilder/AbstractDMLQueryBuilder.php b/src/QueryBuilder/AbstractDMLQueryBuilder.php index b29d08efd..d007d033d 100644 --- a/src/QueryBuilder/AbstractDMLQueryBuilder.php +++ b/src/QueryBuilder/AbstractDMLQueryBuilder.php @@ -65,10 +65,7 @@ public function batchInsert(string $table, array $columns, iterable $rows, array foreach ($rows as $row) { $i = 0; $placeholders = $columnKeys; - /** - * @psalm-var string|int $key - * @psalm-var mixed $value - */ + foreach ($row as $key => $value) { /** @psalm-var string|int $columnName */ $columnName = $columns[$key] ?? (isset($columnKeys[$key]) ? $key : $columnNames[$i] ?? $i); From 79b9bb70f7ed7a1d21bf854d42dd6b81d9d98379 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sat, 4 Nov 2023 14:18:15 +0700 Subject: [PATCH 17/18] Fix psalm --- src/QueryBuilder/AbstractDMLQueryBuilder.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/QueryBuilder/AbstractDMLQueryBuilder.php b/src/QueryBuilder/AbstractDMLQueryBuilder.php index d007d033d..5d57d6412 100644 --- a/src/QueryBuilder/AbstractDMLQueryBuilder.php +++ b/src/QueryBuilder/AbstractDMLQueryBuilder.php @@ -67,11 +67,10 @@ public function batchInsert(string $table, array $columns, iterable $rows, array $placeholders = $columnKeys; foreach ($row as $key => $value) { - /** @psalm-var string|int $columnName */ + /** @psalm-suppress MixedArrayTypeCoercion */ $columnName = $columns[$key] ?? (isset($columnKeys[$key]) ? $key : $columnNames[$i] ?? $i); - + /** @psalm-suppress MixedArrayTypeCoercion */ if (isset($columnSchemas[$columnName])) { - /** @psalm-var mixed $value */ $value = $columnSchemas[$columnName]->dbTypecast($value); } From 8e91e589120f2cabbb5454fbf6341f917edabf76 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sat, 11 Nov 2023 10:30:24 +0700 Subject: [PATCH 18/18] Add line to CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e92f048da..e1ef4bfa5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ - Bug #761: Quote aliases of CTE in `WITH` queries (@Tigrov) - Chg #765: Deprecate `SchemaInterface::TYPE_JSONB` (@Tigrov) - Enh #770: Move methods from concrete `Command` class to `AbstractPdoCommand` class (@Tigrov) +- Bug #769, #61: Fix `AbstractDMLQueryBuilder::batchInsert()` for values as associative arrays (@Tigrov) ## 1.1.1 August 16, 2023