Add BaseBuilder::upsert() and BaseBuilder::upsertBatch()

[sclubricants]

This PR adds BaseBuilder::upsert() and BaseBuilder::upsertBatch() methods.

There is some more functionality to add but I'll hold off on that for now so we can hopefully get this into 4.3.

Related: #6407

Checklist:

• Securely signed commits
• Component(s) with PHPDoc blocks, only if necessary or adds value
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[sclubricants]

@codeigniter4/database-team Have at it!

[kenjis]

The code coverage is down. Can you add tests for uncovered code?

[sclubricants]

There is a few lines of code uncovered because its there for a feature not yet fully implemented.

I never really get the code coverage tests.. they don't make a lot of sense at least on the database. From my experience it seems anytime you change anything in the database these coverage tests complain. I think its because what changes in one driver affects the other coverage tests. Its odd because you look at the mysql test and it shows coverage for say sqlsrv.

Ill try and run some coverage tests locally and make sure we are covered.

[kenjis]

Yeah, something wrong with coverall results.
So if the results are wrong, I don't mind the coverage down.

[sclubricants]

The coverage was really good. Just a couple of lines missed. I added a test and changed one to cover the missing lines. All the upsert and related methods had > 95% coverage.

I think what happens in a case like this is that when the coverage test for MySql runs, all the code that was added to the other drivers is not visible. So basically you increase the denominator and the coverage drops. One thing that would help would be to exclude the other drivers for each coverage test. This isn't perfect either though because sometimes a driver's code is implemented in BaseBuilder.

[kenjis]

The docs have conflicts now. Please resolve.

[kenjis]

The following tests fails on my local.

$ vendor/bin/phpunit tests/system/Database/Live/UpdateTest.php
PHPUnit 9.5.25 #StandWithUkraine

Runtime:       PHP 8.1.10
Configuration: /Users/kenji/work/codeigniter/official/CodeIgniter4/phpunit.xml

...........EEFEE.                                                 17 / 17 (100%)

Time: 00:00.532, Memory: 16.00 MB

There were 4 errors:

1) CodeIgniter\Database\Live\UpdateTest::testUpdateBatchTwoConstraints
CodeIgniter\Database\Exceptions\DatabaseException: You are trying to use a feature which requires SQLite version 3.33 or higher.

/Users/kenji/work/codeigniter/official/CodeIgniter4/system/Database/SQLite3/Builder.php:96
/Users/kenji/work/codeigniter/official/CodeIgniter4/system/Database/BaseBuilder.php:1771
/Users/kenji/work/codeigniter/official/CodeIgniter4/system/Database/BaseBuilder.php:2411
/Users/kenji/work/codeigniter/official/CodeIgniter4/tests/system/Database/Live/UpdateTest.php:262

2) CodeIgniter\Database\Live\UpdateTest::testUpdateBatchConstraintsRawSqlAndAlias
CodeIgniter\Database\Exceptions\DatabaseException: You are trying to use a feature which requires SQLite version 3.33 or higher.

/Users/kenji/work/codeigniter/official/CodeIgniter4/system/Database/SQLite3/Builder.php:96
/Users/kenji/work/codeigniter/official/CodeIgniter4/system/Database/BaseBuilder.php:1771
/Users/kenji/work/codeigniter/official/CodeIgniter4/system/Database/BaseBuilder.php:2411
/Users/kenji/work/codeigniter/official/CodeIgniter4/tests/system/Database/Live/UpdateTest.php:307

3) CodeIgniter\Database\Live\UpdateTest::testUpdateBatchWithoutOnConstraint
CodeIgniter\Database\Exceptions\DatabaseException: You are trying to use a feature which requires SQLite version 3.33 or higher.

/Users/kenji/work/codeigniter/official/CodeIgniter4/system/Database/SQLite3/Builder.php:96
/Users/kenji/work/codeigniter/official/CodeIgniter4/system/Database/BaseBuilder.php:1771
/Users/kenji/work/codeigniter/official/CodeIgniter4/system/Database/BaseBuilder.php:2411
/Users/kenji/work/codeigniter/official/CodeIgniter4/tests/system/Database/Live/UpdateTest.php:449

4) CodeIgniter\Database\Live\UpdateTest::testRawSqlConstraint
CodeIgniter\Database\Exceptions\DatabaseException: You are trying to use a feature which requires SQLite version 3.33 or higher.

/Users/kenji/work/codeigniter/official/CodeIgniter4/system/Database/SQLite3/Builder.php:96
/Users/kenji/work/codeigniter/official/CodeIgniter4/system/Database/BaseBuilder.php:1771
/Users/kenji/work/codeigniter/official/CodeIgniter4/system/Database/BaseBuilder.php:2411
/Users/kenji/work/codeigniter/official/CodeIgniter4/tests/system/Database/Live/UpdateTest.php:481

--

There was 1 failure:

1) CodeIgniter\Database\Live\UpdateTest::testUpdateBatchUpdateFieldsAndAlias
Failed asserting that null is not null.

/Users/kenji/work/codeigniter/official/CodeIgniter4/tests/system/Database/Live/UpdateTest.php:361

ERRORS!
Tests: 17, Assertions: 19, Errors: 4, Failures: 1.

[sclubricants]

The following tests fails on my local.

This failed because I forgot to remove the comment in SQLite Builder to return parent. The github actions runs an old version of SQLite so it didn't cause an error when uploaded here. This should be fixed now.

[michalsn]

I don't see any problems... Really nice feature. Even the user guide part is polished. No complains from me this time 😅

Thanks, @sclubricants!

[kenjis]

Please rebase to fix PHPStan errors.

[kenjis]

The following test causes an error. Is this misuse?

--- a/tests/system/Database/Live/UpsertTest.php
+++ b/tests/system/Database/Live/UpsertTest.php
@@ -234,11 +234,11 @@ final class UpsertTest extends CIUnitTestCase
         $userData = [
             'email'   => 'ahmadinejad@world.com',
             'name'    => 'Ahmadinejad',
-            'country' => 'Iran',
+            'country' => 'now()',
         ];
 
         $this->assertSame($expected, $this->db->table('user')
-            ->set($userData)
+            ->set($userData, '', false)
             ->getCompiledUpsert());
     }
 

CodeIgniter\Database\Exceptions\DatabaseException : setData() has no data.
 /Users/kenji/work/codeigniter/official/CodeIgniter4/system/Database/BaseBuilder.php:1805
 /Users/kenji/work/codeigniter/official/CodeIgniter4/system/Database/BaseBuilder.php:1900
 /Users/kenji/work/codeigniter/official/CodeIgniter4/system/Database/BaseBuilder.php:1869
 /Users/kenji/work/codeigniter/official/CodeIgniter4/tests/system/Database/Live/UpsertTest.php:242

[sclubricants]

I have fixed your issue. It appears that when calling set() without escape binds is not used. I made some adjustments that allow for this.

I also added the ability to use setData() with upsert(). I'd like to eventually (maybe version 5) move all the calls to use setData(). set() might be more useful of a command for the SQL command SET. But by adding setData() this allows calling getCompiledUpsert() on a batch of data. I added a test for this too.

[kenjis]

set() works fine:

        $data = [
            'email'   => 'ahmadinejad@example.com',
            'name'    => 'Ahmadinejad',
            'country' => 'Iran',
        ];
        $sql = $this->db->table('user')->set($data)->getCompiledUpsert();
        echo $sql;

INSERT INTO `db_user` (`country`, `email`, `name`)
VALUES ('Iran','ahmadinejad@example.com','Ahmadinejad')
ON DUPLICATE KEY UPDATE
`db_user`.`country` = VALUES(`country`),
`db_user`.`email` = VALUES(`email`),
`db_user`.`name` = VALUES(`name`)

setData() does not work:

        $data = [
            'email'   => 'ahmadinejad@example.com',
            'name'    => 'Ahmadinejad',
            'country' => 'Iran',
        ];
        $sql = $this->db->table('user')->setData($data)->getCompiledUpsert();
        echo $sql;

INSERT INTO `db_user` (0, 1, 2)
VALUES ('\'Iran\'','\'ahmadinejad@example.com\'','\'Ahmadinejad\'')
ON DUPLICATE KEY UPDATE
`db_user`.0 = VALUES(0),
`db_user`.1 = VALUES(1),
`db_user`.2 = VALUES(2)

[kenjis]

        $this->db->table('user')->truncate();
        $builder = $this->db->table('user');

        $builder->set('email', 'ahmadinejad@example.com');
        $builder->set('name', 'Ahmadinejad');
        $builder->set('country', 'now()', false);
        $builder->upsert();

        echo $this->db->getLastQuery();

INSERT INTO `db_user` (`email`, `name`)
VALUES ('ahmadinejad@example.com','Ahmadinejad')
ON DUPLICATE KEY UPDATE
`db_user`.`email` = VALUES(`email`),
`db_user`.`name` = VALUES(`name`)

The country is gone.

[sclubricants]

The country is gone.

This helped me figure out what was going on. set() without escape saves data to QBSet. set() with escape saves data to binds. So I changed the code to merge the two sources. setData() doesn't allow using escape with one item and then not with the next. So I merged the two from set() and used RawSql for the items to remain unescaped and then call setData().

[michalsn]

Can we add the examples that kenjis provided to our test cases? I think that would be very helpful.

[sclubricants]

Can we add the examples that kenjis provided to our test cases? I think that would be very helpful.

I previously adjusted a test to cover one scenario and also added a test for the use of multiple set() calls.

[kenjis]

set() without escape saves data to QBSet. set() with escape saves data to binds. So I changed the code to merge the two sources. setData() doesn't allow using escape with one item and then not with the next. So I merged the two from set() and used RawSql for the items to remain unescaped and then call setData().

I am not a fan of this process of retrieving and changing data once it has been set,
but it seems to work.

[sclubricants]

I am not a fan of this process of retrieving and changing data once it has been set, but it seems to work.

Id like to maybe refactor the set() method sometime and the methods that use it and make the data structure the same as with setData(). Then maybe rework binds a bit perhaps having it as an optional setting. It would just be nice to have all setting of data funnel though the same method. But this is for another day.

[kenjis]

Thank you for this big contribution!

[kenjis]

After I've merged, I created two issues.

[kenjis]

UpsertTest::testUpsertWithMultipleSet failed.
Created #6691

[MGatner]

🥳🕺👍