Skip to content

Commit

Permalink
Refactoring of batchInsert (for remove using of Quoter) (#363)
Browse files Browse the repository at this point in the history 
* Refactoring of batchInsert (for remove using of Quoter)

* styleci fixes

* styleci fixes
  • Loading branch information
@darkdef
darkdef committed Sep 20, 2022
1 parent 4f7d246 commit a5450ec
Show file tree
Hide file tree
Showing 5 changed files with 193 additions and 48 deletions.
51 changes: 28 additions & 23 deletions src/QueryBuilder/DMLQueryBuilder.php
View file
Expand Up @@ -14,9 +14,9 @@
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;
use Yiisoft\Strings\NumericHelper;

use function array_combine;
use function array_diff;
Expand Down Expand Up @@ -60,30 +60,20 @@ public function batchInsert(string $table, array $columns, iterable|Generator $r

/** @psalm-var array<array-key, array<array-key, string>> $rows */
foreach ($rows as $row) {
$vs = [];

foreach ($row as $i => $value) {
if (isset($columns[$i], $columnSchemas[$columns[$i]])) {
/** @var mixed */
$value = $columnSchemas[$columns[$i]]->dbTypecast($value);
$placeholders = [];
foreach ($row as $index => $value) {
if (isset($columns[$index], $columnSchemas[$columns[$index]])) {
/** @var mixed $value */
$value = $this->getTypecastValue($value, $columnSchemas[$columns[$index]]);
}
if (is_string($value)) {
/** @var string */
$value = $this->quoter->quoteValue($value);
} elseif (is_float($value)) {
/* ensure type cast always has . as decimal separator in all locales */
$value = NumericHelper::normalize((string) $value);
} elseif ($value === false) {
$value = 0;
} elseif ($value === null) {
$value = 'NULL';
} elseif ($value instanceof ExpressionInterface) {
$value = $this->queryBuilder->buildExpression($value, $params);

if ($value instanceof ExpressionInterface) {
$placeholders[] = $this->queryBuilder->buildExpression($value, $params);
} else {
$placeholders[] = $this->queryBuilder->bindParam($value, $params);
}
/** @var string */
$vs[] = $value;
}
$values[] = '(' . implode(', ', $vs) . ')';
$values[] = '(' . implode(', ', $placeholders) . ')';
}

if (empty($values)) {
Expand Down Expand Up @@ -230,7 +220,7 @@ protected function prepareInsertValues(string $table, array|QueryInterface $colu
foreach ($columns as $name => $value) {
$names[] = $this->quoter->quoteColumnName($name);
/** @var mixed $value */
$value = isset($columnSchemas[$name]) ? $columnSchemas[$name]->dbTypecast($value) : $value;
$value = $this->getTypecastValue($value, $columnSchemas[$name] ?? null);

if ($value instanceof ExpressionInterface) {
$placeholders[] = $this->queryBuilder->buildExpression($value, $params);
Expand Down Expand Up @@ -392,4 +382,19 @@ static function (Constraint $constraint) use ($quoter, $columns, &$columnNames)
/** @psalm-var array $columnNames */
return array_unique($columnNames);
}

/**
* @param mixed $value
* @param ColumnSchemaInterface|null $columnSchema
*
* @return mixed
*/
protected function getTypecastValue(mixed $value, ColumnSchemaInterface $columnSchema = null): mixed
{
if ($columnSchema) {
return $columnSchema->dbTypecast($value);
}

return $value;
}
}
9 changes: 9 additions & 0 deletions src/Schema/Quoter.php
View file
Expand Up @@ -50,6 +50,15 @@ public function ensureNameQuoted(string $name): string
return $name;
}

public function ensureColumnName(string $name): string
{
if (strrpos($name, '.') !== false) {
$parts = explode('.', $name);
$name = $parts[count($parts)-1];
}
return preg_replace('|^\[\[([_\w\-. ]+)\]\]$|', '\1', $name);
}

public function quoteColumnName(string $name): string
{
if (str_contains($name, '(') || str_contains($name, '[[')) {
Expand Down
9 changes: 9 additions & 0 deletions src/Schema/QuoterInterface.php
View file
Expand Up @@ -24,6 +24,15 @@ public function getTableNameParts(string $name): array;
*/
public function ensureNameQuoted(string $name): string;

/**
* Ensures name of column is wrapped with [[ and ]].
*
* @param string $name
*
* @return string
*/
public function ensureColumnName(string $name): string;

/**
* Quotes a column name for use in a query.
*
Expand Down
48 changes: 35 additions & 13 deletions src/TestSupport/Provider/QueryBuilderProvider.php
View file
Expand Up @@ -129,60 +129,82 @@ static function (QueryBuilderInterface $qb) use ($tableName2, $name2) {
public function batchInsertProvider(): array
{
return [
[
'simple' => [
'customer',
['email', 'name', 'address'],
[['test@example.com', 'silverfire', 'Kyiv {{city}}, Ukraine']],
DbHelper::replaceQuotes(
'expected' => DbHelper::replaceQuotes(
'INSERT INTO [[customer]] ([[email]], [[name]], [[address]])'
. " VALUES ('test@example.com', 'silverfire', 'Kyiv {{city}}, Ukraine')",
. ' VALUES (:qp0, :qp1, :qp2)',
$this->db->getDriver()->getDriverName(),
),
[
':qp0' => 'test@example.com',
':qp1' => 'silverfire',
':qp2' => 'Kyiv {{city}}, Ukraine',
],
],
'escape-danger-chars' => [
'customer',
['address'],
[["SQL-danger chars are escaped: '); --"]],
'expected' => DbHelper::replaceQuotes(
"INSERT INTO [[customer]] ([[address]]) VALUES ('SQL-danger chars are escaped: \'); --')",
'INSERT INTO [[customer]] ([[address]]) VALUES (:qp0)',
$this->db->getDriver()->getDriverName(),
),
[
':qp0' => "SQL-danger chars are escaped: '); --",
],
],
[
'customer2' => [
'customer',
['address'],
[],
'',
],
[
'customer3' => [
'customer',
[],
[['no columns passed']],
DbHelper::replaceQuotes(
"INSERT INTO [[customer]] () VALUES ('no columns passed')",
'expected' => DbHelper::replaceQuotes(
'INSERT INTO [[customer]] () VALUES (:qp0)',
$this->db->getDriver()->getDriverName(),
),
[
':qp0' => 'no columns passed',
],
],
'bool-false, bool2-null' => [
'type',
['bool_col', 'bool_col2'],
[[false, null]],
'expected' => DbHelper::replaceQuotes(
'INSERT INTO [[type]] ([[bool_col]], [[bool_col2]]) VALUES (0, NULL)',
'INSERT INTO [[type]] ([[bool_col]], [[bool_col2]]) VALUES (:qp0, :qp1)',
$this->db->getDriver()->getDriverName(),
),
[
':qp0' => 0,
':qp1' => null,
],
],
[
'wrong' => [
'{{%type}}',
['{{%type}}.[[float_col]]', '[[time]]'],
[[null, new Expression('now()')]],
'INSERT INTO {{%type}} ({{%type}}.[[float_col]], [[time]]) VALUES (NULL, now())',
[[null, new Expression('now()')], [null, new Expression('now()')]],
'expected' => 'INSERT INTO {{%type}} ({{%type}}.[[float_col]], [[time]]) VALUES (:qp0, now()), (:qp1, now())',
[
':qp0' => null,
':qp1' => null,
],
],
'bool-false, time-now()' => [
'{{%type}}',
['{{%type}}.[[bool_col]]', '[[time]]'],
[[false, new Expression('now()')]],
'expected' => 'INSERT INTO {{%type}} ({{%type}}.[[bool_col]], [[time]]) VALUES (0, now())',
'expected' => 'INSERT INTO {{%type}} ({{%type}}.[[bool_col]], [[time]]) VALUES (:qp0, now())',
[
':qp0' => null,
],
],
];
}
Expand Down
124 changes: 112 additions & 12 deletions src/TestSupport/TestCommandTrait.php
View file
Expand Up @@ -20,6 +20,7 @@
use Yiisoft\Db\Query\Data\DataReader;
use Yiisoft\Db\Query\Query;
use Yiisoft\Db\Schema\Schema;
use Yiisoft\Db\TestSupport\Helper\DbHelper;

use function call_user_func_array;
use function date;
Expand Down Expand Up @@ -240,6 +241,34 @@ public function testBatchInsert(): void
]
);
$this->assertEquals(2, $command->execute());
$result = (new Query($db))
->select(['email', 'name', 'address'])
->from('{{customer}}')
->where(['=', '[[email]]', 't1@example.com'])
->one();
$this->assertCount(3, $result);
$this->assertSame(
[
'email' => 't1@example.com',
'name' => 't1',
'address' => 't1 address',
],
$result,
);
$result = (new Query($db))
->select(['email', 'name', 'address'])
->from('{{customer}}')
->where(['=', '[[email]]', 't2@example.com'])
->one();
$this->assertCount(3, $result);
$this->assertSame(
[
'email' => 't2@example.com',
'name' => null,
'address' => '0',
],
$result,
);

/**
* @link https://github.com/yiisoft/yii2/issues/11693
Expand All @@ -253,6 +282,24 @@ public function testBatchInsert(): void
$this->assertEquals(0, $command->execute());
}

public function testBatchInsertWithManyData(): void
{
$attemptsInsertRows = 200;
$db = $this->getConnection(true);

$command = $db->createCommand();
for ($i = 0; $i < $attemptsInsertRows; $i++) {
$values[$i] = ['t' . $i . '@any.com', 't' . $i, 't' . $i . ' address'];
}

$command->batchInsert('{{customer}}', ['email', 'name', 'address'], $values);

$this->assertEquals($attemptsInsertRows, $command->execute());

$insertedRowsCount = (new Query($db))->from('{{customer}}')->count();
$this->assertGreaterThanOrEqual($attemptsInsertRows, $insertedRowsCount);
}

/**
* @throws Exception|InvalidConfigException|Throwable
*/
Expand Down Expand Up @@ -1088,43 +1135,96 @@ public function testDropView(): void

public function batchInsertSqlProviderTrait(): array
{
$db = $this->getConnection();

return [
'multirow' => [
'type',
['int_col', 'float_col', 'char_col', 'bool_col'],
'values' => [
['0', '0.0', 'test string', true,],
[false, 0, 'test string2', false,],
],
'expected' => DbHelper::replaceQuotes(
'INSERT INTO [[type]] ([[int_col]], [[float_col]], [[char_col]], [[bool_col]])'
. ' VALUES (:qp0, :qp1, :qp2, :qp3), (:qp4, :qp5, :qp6, :qp7)',
$db->getDriver()->getDriverName(),
),
'expectedParams' => [
':qp0' => 0,
':qp1' => 0.0,
':qp2' => 'test string',
':qp3' => true,
':qp4' => 0,
':qp5' => 0.0,
':qp6' => 'test string2',
':qp7' => false,
],
2,
],
'issue11242' => [
'type',
['int_col', 'float_col', 'char_col'],
[['', '', 'Kyiv {{city}}, Ukraine']],
['int_col', 'float_col', 'char_col', 'bool_col'],
'values' => [[1.0, 1.1, 'Kyiv {{city}}, Ukraine', true]],
/**
* {@see https://github.com/yiisoft/yii2/issues/11242}
*
* Make sure curly bracelets (`{{..}}`) in values will not be escaped
*/
'expected' => 'INSERT INTO `type` (`int_col`, `float_col`, `char_col`)'
. " VALUES (NULL, NULL, 'Kyiv {{city}}, Ukraine')",
'expected' => DbHelper::replaceQuotes(
'INSERT INTO [[type]] ([[int_col]], [[float_col]], [[char_col]], [[bool_col]])'
. ' VALUES (:qp0, :qp1, :qp2, :qp3)',
$db->getDriver()->getDriverName(),
),
'expectedParams' => [
':qp0' => 1,
':qp1' => 1.1,
':qp2' => 'Kyiv {{city}}, Ukraine',
':qp3' => true,
],
],
'wrongBehavior' => [
'{{%type}}',
['{{%type}}.[[int_col]]', '[[float_col]]', 'char_col'],
[['', '', 'Kyiv {{city}}, Ukraine']],
['{{%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.
* TODO: make it work. Impossible without BC breaking for public methods.
*/
'expected' => 'INSERT INTO `type` (`type`.`int_col`, `float_col`, `char_col`)'
. " VALUES ('', '', 'Kyiv {{city}}, Ukraine')",
'expected' => DbHelper::replaceQuotes(
'INSERT INTO [[type]] ([[type]].[[int_col]], [[float_col]], [[char_col]], [[bool_col]])'
. ' VALUES (:qp0, :qp1, :qp2, :qp3)',
$db->getDriver()->getDriverName(),
),
'expectedParams' => [
':qp0' => '0',
':qp1' => '0.0',
':qp2' => 'Kyiv {{city}}, Ukraine',
':qp3' => false,
],
],
'batchInsert binds params from expression' => [
'{{%type}}',
['int_col'],
['int_col', 'float_col', 'char_col', 'bool_col'],
/**
* This example is completely useless. This feature of batchInsert is intended to be used with complex
* expression objects, such as JsonExpression.
*/
[[new Expression(':qp1', [':qp1' => 42])]],
'expected' => 'INSERT INTO `type` (`int_col`) VALUES (:qp1)',
'expectedParams' => [':qp1' => 42],
'values' => [[new Expression(':exp1', [':exp1' => 42]), 1, 'test', false]],
'expected' => DbHelper::replaceQuotes(
'INSERT INTO [[type]] ([[int_col]], [[float_col]], [[char_col]], [[bool_col]])'
. ' VALUES (:exp1, :qp1, :qp2, :qp3)',
$db->getDriver()->getDriverName(),
),
'expectedParams' => [
':exp1' => 42,
':qp1' => 1.0,
':qp2' => 'test',
':qp3' => false,
],
],
];
}
Expand Down

0 comments on commit a5450ec

Please sign in to comment.