Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor BaseBuilder *Batch() Methods #6536

Merged
merged 40 commits into from
Sep 27, 2022
Merged

Refactor BaseBuilder *Batch() Methods #6536

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
7962dc0
Added BaseBuilder::batchExecute() method
sclubricants Sep 11, 2022
4642ea0
Added BaseBuilder::setData() and ::setAlias()
sclubricants Sep 11, 2022
db30072
Added BaseBuilder::onConstraint()
sclubricants Sep 11, 2022
e5b44f2
Refactored updateBatch() to use new methods
sclubricants Sep 11, 2022
902749b
remove old updateBatch() method
sclubricants Sep 11, 2022
585a494
Refactor setUpdateBatch() method
sclubricants Sep 11, 2022
45a5ef1
Change method signature of _updateBatch() to match batchExecute()
sclubricants Sep 11, 2022
7fae12c
Update Tests
sclubricants Sep 11, 2022
961b111
Changed RawSql test
sclubricants Sep 11, 2022
8827114
Refactor BaseBuilder::insertBatch()
sclubricants Sep 12, 2022
719220c
Fix _insertBatch()
sclubricants Sep 12, 2022
10bdd49
Fix tests
sclubricants Sep 12, 2022
5c160f3
Refactored setInsertBatch() to use new methods
sclubricants Sep 12, 2022
ceee0dc
Fix SQLite and OCI8
sclubricants Sep 13, 2022
46c79c1
Add throw Exception in *Batch operations
sclubricants Sep 14, 2022
a93bfdc
Add RawSql as parameter type to constraints.
sclubricants Sep 14, 2022
a2ccd9c
Change setAlias() default parameter value.
sclubricants Sep 14, 2022
dd16d92
Move Exception for no data to batchExecute
sclubricants Sep 16, 2022
72d1634
Rename getValues() to formatValues()
sclubricants Sep 16, 2022
41938cd
Add logic to updateFields() to add additional fields for update
sclubricants Sep 16, 2022
26da2cf
Fix code mistake
sclubricants Sep 16, 2022
110add6
Update User Guide
sclubricants Sep 16, 2022
37c938b
Fix comment block
sclubricants Sep 16, 2022
6197efe
Add example to updateBatch() documentation
sclubricants Sep 16, 2022
cb1f87e
Fix example - arranging in order
sclubricants Sep 17, 2022
b7c5aeb
Update user_guide_src/source/database/query_builder.rst
sclubricants Sep 19, 2022
be83c2a
Update query_builder.rst
sclubricants Sep 19, 2022
bbd10d7
Update tests/system/Database/Live/UpdateTest.php
sclubricants Sep 19, 2022
5f82be0
Use str_replace instead of sprintf
sclubricants Sep 20, 2022
cea31cc
Add setAlias() to documentation
sclubricants Sep 23, 2022
b06a9af
Update change log
sclubricants Sep 25, 2022
b3c62e6
Update Documentation
sclubricants Sep 26, 2022
3ed6122
More updates to documentation
sclubricants Sep 26, 2022
5520c12
Revert removal of method documentation
sclubricants Sep 27, 2022
1e5033c
Update user_guide_src/source/changelogs/v4.3.0.rst
sclubricants Sep 27, 2022
7ba9b66
fix user guide
sclubricants Sep 27, 2022
a46b4f9
Update query_builder.rst
sclubricants Sep 27, 2022
c0aa82d
Update query_builder.rst
sclubricants Sep 27, 2022
5e4d7ef
Update query_builder.rst
sclubricants Sep 27, 2022
f6a7aeb
Added upgrade note
sclubricants Sep 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
409 changes: 218 additions & 191 deletions system/Database/BaseBuilder.php
View file
Open in desktop

Large diffs are not rendered by default.

12 changes: 3 additions & 9 deletions system/Database/MySQLi/Builder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,8 @@ protected function _fromTables(): string
/**
* Generates a platform-specific batch update string from the supplied data
*/
protected function _updateBatch(string $table, array $values, string $index): string
protected function _updateBatch(string $table, array $keys, array $values): string
{
// this is a work around until the rest of the platform is refactored
if ($index !== '') {
$this->QBOptions['constraints'] = [$index];
}
$keys = array_keys(current($values));

$sql = $this->QBOptions['sql'] ?? '';

// if this is the first iteration of batch then we need to build skeleton sql
Expand All @@ -89,7 +83,7 @@ protected function _updateBatch(string $table, array $values, string $index): st

$sql = 'UPDATE ' . $this->compileIgnore('update') . $table . "\n";

$sql .= 'INNER JOIN (' . "\n%s";
$sql .= 'INNER JOIN (' . "\n{:_table_:}";

$sql .= ') ' . $alias . "\n";

Expand Down Expand Up @@ -135,6 +129,6 @@ protected function _updateBatch(string $table, array $values, string $index): st
) . "\n";
}

return sprintf($sql, $data);
return str_replace('{:_table_:}', $data, $sql);
}
}
51 changes: 26 additions & 25 deletions system/Database/OCI8/Builder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,29 +68,36 @@ class Builder extends BaseBuilder
*/
protected function _insertBatch(string $table, array $keys, array $values): string
{
$insertKeys = implode(', ', $keys);
$hasPrimaryKey = in_array('PRIMARY', array_column($this->db->getIndexData($table), 'type'), true);
$sql = $this->QBOptions['sql'] ?? '';

// ORA-00001 measures
if ($hasPrimaryKey) {
$sql = 'INSERT INTO ' . $table . ' (' . $insertKeys . ") \n SELECT * FROM (\n";
$selectQueryValues = [];
// if this is the first iteration of batch then we need to build skeleton sql
if ($sql === '') {
$insertKeys = implode(', ', $keys);
$hasPrimaryKey = in_array('PRIMARY', array_column($this->db->getIndexData($table), 'type'), true);

foreach ($values as $value) {
$selectValues = implode(',', array_map(static fn ($value, $key) => $value . ' as ' . $key, explode(',', substr(substr($value, 1), 0, -1)), $keys));
$selectQueryValues[] = 'SELECT ' . $selectValues . ' FROM DUAL';
}
// ORA-00001 measures
$sql = 'INSERT' . ($hasPrimaryKey ? '' : ' ALL') . ' INTO ' . $table . ' (' . $insertKeys . ")\n{:_table_:}";

return $sql . implode("\n UNION ALL \n", $selectQueryValues) . "\n)";
$this->QBOptions['sql'] = $sql;
}

$sql = "INSERT ALL\n";

foreach ($values as $value) {
$sql .= ' INTO ' . $table . ' (' . $insertKeys . ') VALUES ' . $value . "\n";
if (isset($this->QBOptions['fromQuery'])) {
$data = $this->QBOptions['fromQuery'];
} else {
$data = implode(
" FROM DUAL UNION ALL\n",
array_map(
static fn ($value) => 'SELECT ' . implode(', ', array_map(
static fn ($key, $index) => $index . ' ' . $key,
$keys,
$value
)),
$values
)
) . " FROM DUAL\n";
}

return $sql . 'SELECT * FROM DUAL';
return str_replace('{:_table_:}', $data, $sql);
}

/**
Expand Down Expand Up @@ -232,14 +239,8 @@ protected function resetSelect()
/**
* Generates a platform-specific batch update string from the supplied data
*/
protected function _updateBatch(string $table, array $values, string $index): string
protected function _updateBatch(string $table, array $keys, array $values): string
{
// this is a work around until the rest of the platform is refactored
if ($index !== '') {
$this->QBOptions['constraints'] = [$index];
}
$keys = array_keys(current($values));

$sql = $this->QBOptions['sql'] ?? '';

// if this is the first iteration of batch then we need to build skeleton sql
Expand All @@ -263,7 +264,7 @@ protected function _updateBatch(string $table, array $values, string $index): st
// Oracle doesn't support ignore on updates so we will use MERGE
$sql = 'MERGE INTO ' . $table . "\n";

$sql .= 'USING (' . "\n%s";
$sql .= 'USING (' . "\n{:_table_:}";

$sql .= ') ' . $alias . "\n";

Expand Down Expand Up @@ -311,6 +312,6 @@ protected function _updateBatch(string $table, array $values, string $index): st
) . "\n";
}

return sprintf($sql, $data);
return str_replace('{:_table_:}', $data, $sql);
}
}
19 changes: 18 additions & 1 deletion system/Database/Postgre/Builder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,24 @@ protected function _insert(string $table, array $keys, array $unescapedKeys): st
*/
protected function _insertBatch(string $table, array $keys, array $values): string
{
return trim(sprintf('INSERT INTO %s (%s) VALUES %s %s', $table, implode(', ', $keys), implode(', ', $values), $this->compileIgnore('insert')));
$sql = $this->QBOptions['sql'] ?? '';

// if this is the first iteration of batch then we need to build skeleton sql
if ($sql === '') {
$sql = 'INSERT INTO ' . $table . '(' . implode(', ', $keys) . ")\n{:_table_:}\n";

$sql .= $this->compileIgnore('insert');

$this->QBOptions['sql'] = $sql;
}

if (isset($this->QBOptions['fromQuery'])) {
$data = $this->QBOptions['fromQuery'];
} else {
$data = 'VALUES ' . implode(', ', $this->formatValues($values));
}

return str_replace('{:_table_:}', $data, $sql);
}

/**
Expand Down
18 changes: 17 additions & 1 deletion system/Database/SQLSRV/Builder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,23 @@ protected function _insert(string $table, array $keys, array $unescapedKeys): st
*/
protected function _insertBatch(string $table, array $keys, array $values): string
{
return 'INSERT ' . $this->compileIgnore('insert') . 'INTO ' . $this->getFullName($table) . ' (' . implode(', ', $keys) . ') VALUES ' . implode(', ', $values);
$sql = $this->QBOptions['sql'] ?? '';

// if this is the first iteration of batch then we need to build skeleton sql
if ($sql === '') {
$sql = 'INSERT ' . $this->compileIgnore('insert') . 'INTO ' . $this->getFullName($table)
. ' (' . implode(', ', $keys) . ")\n{:_table_:}";

$this->QBOptions['sql'] = $sql;
}

if (isset($this->QBOptions['fromQuery'])) {
$data = $this->QBOptions['fromQuery'];
} else {
$data = 'VALUES ' . implode(', ', $this->formatValues($values));
}

return str_replace('{:_table_:}', $data, $sql);
}

/**
Expand Down
15 changes: 13 additions & 2 deletions system/Database/SQLite3/Builder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
namespace CodeIgniter\Database\SQLite3;

use CodeIgniter\Database\BaseBuilder;
use CodeIgniter\Database\Exceptions\DatabaseException;

/**
* Builder for SQLite3
Expand Down Expand Up @@ -74,16 +75,26 @@ protected function _truncate(string $table): string
/**
* Generates a platform-specific batch update string from the supplied data
*/
protected function _updateBatch(string $table, array $values, string $index): string
protected function _updateBatch(string $table, array $keys, array $values): string
{
if (version_compare($this->db->getVersion(), '3.33.0') >= 0) {
return parent::_updateBatch($table, $values, $index);
return parent::_updateBatch($table, $keys, $values);
}

$constraints = $this->QBOptions['constraints'] ?? [];

if (count($constraints) > 1 || isset($this->QBOptions['fromQuery'])) {
throw new DatabaseException('You are trying to use a feature which requires SQLite version 3.33 or higher.');
}

$index = current($constraints);

$ids = [];
$final = [];

foreach ($values as $val) {
$val = array_combine($keys, $val);

$ids[] = $val[$index];

foreach (array_keys($val) as $field) {
Expand Down
4 changes: 2 additions & 2 deletions tests/system/Database/Builder/InsertTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ public function testInsertBatchThrowsExceptionOnNoData()
$builder = $this->db->table('jobs');

$this->expectException(DatabaseException::class);
$this->expectExceptionMessage('You must use the "set" method to update an entry.');
$this->expectExceptionMessage('insertBatch() has no data.');
$builder->insertBatch();
}

Expand All @@ -274,7 +274,7 @@ public function testInsertBatchThrowsExceptionOnEmptyData()
$builder = $this->db->table('jobs');

$this->expectException(DatabaseException::class);
$this->expectExceptionMessage('insertBatch() called with no data');
$this->expectExceptionMessage('insertBatch() has no data.');
$builder->insertBatch([]);
}
}
37 changes: 25 additions & 12 deletions tests/system/Database/Builder/UpdateTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,11 +234,11 @@ public function testUpdateBatch()
$expected = <<<'EOF'
UPDATE "jobs"
SET
"name" = _u."name",
"description" = _u."description"
"description" = _u."description",
"name" = _u."name"
FROM (
SELECT 2 "id", 'Comedian' "name", 'There''s something in your teeth' "description" UNION ALL
SELECT 3 "id", 'Cab Driver' "name", 'I am yellow' "description"
SELECT 'There''s something in your teeth' "description", 2 "id", 'Comedian' "name" UNION ALL
SELECT 'I am yellow' "description", 3 "id", 'Cab Driver' "name"
) _u
WHERE "jobs"."id" = _u."id"
EOF;
Expand Down Expand Up @@ -273,11 +273,11 @@ public function testSetUpdateBatchWithoutEscape()
$expected = <<<'EOF'
UPDATE "jobs"
SET
"name" = _u."name",
"description" = _u."description"
"description" = _u."description",
"name" = _u."name"
FROM (
SELECT 2 "id", SUBSTRING(name, 1) "name", SUBSTRING(description, 3) "description" UNION ALL
SELECT 3 "id", SUBSTRING(name, 2) "name", SUBSTRING(description, 4) "description"
SELECT SUBSTRING(description, 3) "description", 2 "id", SUBSTRING(name, 1) "name" UNION ALL
SELECT SUBSTRING(description, 4) "description", 3 "id", SUBSTRING(name, 2) "name"
) _u
WHERE "jobs"."id" = _u."id"
EOF;
Expand All @@ -290,7 +290,7 @@ public function testUpdateBatchThrowsExceptionWithNoData()
$builder = new BaseBuilder('jobs', $this->db);

$this->expectException(DatabaseException::class);
$this->expectExceptionMessage('You must use the "set" method to update an entry.');
$this->expectExceptionMessage('updateBatch() has no data.');

$builder->updateBatch(null, 'id');
}
Expand All @@ -300,17 +300,30 @@ public function testUpdateBatchThrowsExceptionWithNoID()
$builder = new BaseBuilder('jobs', $this->db);

$this->expectException(DatabaseException::class);
$this->expectExceptionMessage('You must specify an index to match on for batch updates.');
$this->expectExceptionMessage('You must specify a constraint to match on for batch updates.');

$set = [
[
'id' => 2,
'name' => 'SUBSTRING(name, 1)',
'description' => 'SUBSTRING(description, 3)',
],
[
'id' => 3,
'name' => 'SUBSTRING(name, 2)',
'description' => 'SUBSTRING(description, 4)',
],
];

$builder->updateBatch([]);
$builder->updateBatch($set, null);
}

public function testUpdateBatchThrowsExceptionWithEmptySetArray()
{
$builder = new BaseBuilder('jobs', $this->db);

$this->expectException(DatabaseException::class);
$this->expectExceptionMessage('updateBatch() called with no data');
$this->expectExceptionMessage('updateBatch() has no data.');

$builder->updateBatch([], 'id');
}
Expand Down
86 changes: 86 additions & 0 deletions tests/system/Database/Live/UpdateTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
namespace CodeIgniter\Database\Live;

use CodeIgniter\Database\Exceptions\DatabaseException;
use CodeIgniter\Database\RawSql;
use CodeIgniter\Test\CIUnitTestCase;
use CodeIgniter\Test\DatabaseTestTrait;
use Config\Database;
Expand Down Expand Up @@ -238,4 +239,89 @@ public function testSetWithBoolean()
'type_boolean' => true,
]);
}

public function testUpdateBatchTwoConstraints()
{
if (version_compare($this->db->getVersion(), '3.33.0') < 0) {
$this->markTestSkipped('This SQLite version does not support this test.');
}

$data = [
[
'id' => 1,
'name' => 'Derek Jones Changes',
'country' => 'US',
],
[
'id' => 2,
'name' => 'Ahmadinejad Does Not Change',
'country' => 'Greece',
],
];

$this->db->table('user')->updateBatch($data, 'id, country');

$this->seeInDatabase('user', [
'name' => 'Derek Jones Changes',
'country' => 'US',
]);
$this->seeInDatabase('user', [
'name' => 'Ahmadinejad',
'country' => 'Iran',
]);
}

public function testUpdateBatchConstraintsRawSqlAndAlias()
{
if (version_compare($this->db->getVersion(), '3.33.0') < 0) {
$this->markTestSkipped('This SQLite version does not support this test.');
}

$data = [
[
'id' => 1,
'name' => 'Derek Jones Changes',
'country' => 'US',
],
[
'id' => 2,
'name' => 'Ahmadinejad Changes',
'country' => 'Uruguay',
Copy link
Member

@kenjis kenjis Sep 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you remove the existing test case instead of adding a test case?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not where I removed any tests. I looked back through the commits and didn't see any. Sometimes I'll rename them if I add to them. In general though I try to build on existing tests where appropriate to try and not create redundant tests. I try and get the maximum code coverage for each test.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I searched // same length but different in UpdateTest.php, but not found.
You had a test case Iraq for same length but different, but it was removed
because Uruguay is not the same length.
Is it okay?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed this test around a few times.. I was having trouble useing RawSql with the % character. Ultimately I fixed the issue and changed the test to do what I was originally wanting to do.

Copy link
Member Author

@sclubricants sclubricants Sep 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed to Uruguay and used LIKE 'U%'. It was a better test because this forced me to solve the escaping of % in RawSql.

],
[
'id' => 3,
'name' => 'Richard A Causey Changes',
'country' => 'US',
],
[
'id' => 4,
'name' => 'Chris Martin Does Not Change',
'country' => 'Greece',
],
];

$this->db->table('user')->setData($data, true, 'd')->updateBatch(
null,
['id', new RawSql($this->db->protectIdentifiers('d')
. '.' . $this->db->protectIdentifiers('country')
. " LIKE 'U%'")]
);

$this->seeInDatabase('user', [
'name' => 'Derek Jones Changes',
'country' => 'US',
]);
$this->seeInDatabase('user', [
'name' => 'Ahmadinejad Changes',
'country' => 'Uruguay',
]);
$this->seeInDatabase('user', [
'name' => 'Richard A Causey Changes',
'country' => 'US',
]);
$this->seeInDatabase('user', [
'name' => 'Chris Martin',
'country' => 'UK',
]);
}
}
Loading