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.

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

RawSql not working in UpdateBatch #6554

Closed
pixobit opened this issue Sep 19, 2022 · 4 comments
Closed

RawSql not working in UpdateBatch #6554

pixobit opened this issue Sep 19, 2022 · 4 comments
Labels
database Issues or pull requests that affect the database layer enhancement PRs that improve existing functionalities

Comments

@pixobit
Copy link
Contributor

pixobit commented Sep 19, 2022

PHP Version

8.0

CodeIgniter4 Version

4.2.6

CodeIgniter4 Installation Method

Manual (zip or tar.gz)

Which operating systems have you tested for this bug?

Windows

Which server did you use?

apache

Database

MySQL 5.6

What happened?

I'm trying to implement a decrement with UpdateBatch, and previously used the escape, but was recommended to use the RawSql class instead. However, when I checked the query, it seems to be escaped, and when I digged deeper, I didn't see RawSql being implemented in the UpdateBatch method.

Steps to Reproduce

$this->table('table')->updateBatch([
    'id' => 1,
    'int_column' => new RawSql('(CASE WHEN int_column > 1 THEN int_column - 1 ELSE 0  END)')
], 'id')

Expected Output

UPDATE `table` SET `int_column` = CASE WHEN `id` = '1' THEN (CASE WHEN int_column > 1 THEN int_column - 1 ELSE 0 END) ELSE `int_column` END WHERE `id` IN('1')

Anything else?

One way to solve this, is to update this line (in Database/BaseBuilder.php)
$clean[$this->db->protectIdentifiers($k2, false)] = $escape ? $this->db->escape($v2) : $v2;
to
$clean[$this->db->protectIdentifiers($k2, false)] = $escape && !($v2 instanceof RawSql) ? $this->db->escape($v2) : (string) $v2;

@pixobit pixobit added the bug Verified issues on the current code behavior or pull requests that will fix them label Sep 19, 2022
@kenjis kenjis removed the bug Verified issues on the current code behavior or pull requests that will fix them label Sep 20, 2022
@kenjis
Copy link
Member

kenjis commented Sep 20, 2022

Your Steps to Reproduce does not use RawSql at all.

It seems v4.2.6 does not support RawSql in updateBatch().
https://codeigniter4.github.io/CodeIgniter4/changelogs/v4.2.0.html#database

That's all.

@pixobit
Copy link
Contributor Author

pixobit commented Sep 20, 2022

Sorry about that. I edited the issue to include it.
You were the one suggesting RawSql before (here: #6153 (comment)), and the use of RawSql should be consistent... This would feel like a bug to anybody

@kenjis kenjis added the enhancement PRs that improve existing functionalities label Sep 20, 2022
@kenjis kenjis changed the title Bug: RawSql not working in UpdateBatch RawSql not working in UpdateBatch Sep 20, 2022
@sclubricants sclubricants added the database Issues or pull requests that affect the database layer label Oct 6, 2022
@sclubricants
Copy link
Member

sclubricants commented Oct 6, 2022

This should be resolved with 4.3.

RawSql was excluded from escape() #6332

This should have been fixed with the setData() method and refactor #6536

Also the updateBatch() method was updated. #6373

@sclubricants
Copy link
Member

I have tested this in 4.3 without issue:

    public function testUpdateBatchRawSql()
    {
        $sql = $this->db->table('table')->testMode()->updateBatch([
            'id'         => 1,
            'int_column' => new RawSql('(CASE WHEN int_column > 1 THEN int_column - 1 ELSE 0  END)'),
        ], 'id');

        $expectedSql = <<<'SQL'
            UPDATE `db_table`
            SET
            `int_column` = _u.`int_column`
            FROM (
            SELECT 1 `id`, (CASE WHEN int_column > 1 THEN int_column - 1 ELSE 0  END) `int_column`
            ) _u
            WHERE `db_table`.`id` = _u.`id`
            SQL;
        $this->assertStringContainsString($expectedSql, $sql[0]);
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
database Issues or pull requests that affect the database layer enhancement PRs that improve existing functionalities
Projects
None yet
Development

No branches or pull requests

3 participants