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

Bug: A wrong escape on BaseBuilder::set() #3127

Closed
smartse0k opened this issue Jun 19, 2020 · 5 comments
Closed

Bug: A wrong escape on BaseBuilder::set() #3127

smartse0k opened this issue Jun 19, 2020 · 5 comments
Labels
bug Verified issues on the current code behavior or pull requests that will fix them

Comments

@smartse0k
Copy link

Describe the bug
I guess it is a bug that the function named BaseBuilder::set() building wrong escape character

CodeIgniter 4 version
4.0.3 on master branch.

Affected module(s)
i don't know

Expected behavior, and steps to reproduce if appropriate
I expected follow compiled SQL with code:

  • source code in my model
    $this->where('ID', $id)->set('VIEW_COUNT', 'VIEW_COUNT + 1', false)->update();

  • expected SQL
    UPDATE TABLENAME SET VIEW_COUNT = VIEW_COUNT + 1, UPDATE_FIELD = 'date' WHERE ID = '1'

  • but, complied SQL (also the escape character in updatedField has been removed.)
    UPDATE TABLENAME SET VIEW_COUNT = VIEW_COUNT + 1, UPDATE_FIELD = date WHERE ID = '1'

Context

  • OS: CentOS 7
  • Apache Daemon (httpd) 2.4
  • PHP 7.4.1
@smartse0k smartse0k added the bug Verified issues on the current code behavior or pull requests that will fix them label Jun 19, 2020
@michalsn
Copy link
Member

Can you show how do you assign the UPDATE_FIELD part to the builder?

@lonnieezell
Copy link
Member

That has been an issue with CI'q query builder for many years, AFAIK. You should use the increment and decrement functions instead.

@smartse0k
Copy link
Author

Note: Sorry my bad english and thank you your reply.

@michalsn Just I wrote only my column name in MyModel::$updatedField

Full code are...

<?php
namespace App\Models;

use App\Entities\BoardPost;
use CodeIgniter\Model;

class BoardPostModel extends Model {
    protected $table = 'BOARD_POST_TB';
    protected $primaryKey = 'ID';

    protected $returnType = 'App\Entities\BoardPost';
    protected $useSoftDeletes = true;

    protected $allowedFields = [
        'BOARD_ID',
        'USER_ID',
        'TITLE',
        'BODY',
        'VIEW_COUNT'
    ];

    protected $useTimestamps = true;
    protected $createdField  = 'CREATE_DT';
    protected $updatedField  = 'UPDATE_DT';
    protected $deletedField  = 'DELETE_DT';

    protected $validationRules    = [];
    protected $validationMessages = [];
    protected $skipValidation     = false;

    public function addViewCountByPostId(int $postId) : bool {
        $updateResult = $this->where('ID', $postId)
            ->set('VIEW_COUNT', 'VIEW_COUNT + 1', false)
            ->update();

        return $updateResult;
    }
}

CI4 built follow SQL:
UPDATE BOARD_POST_TB SET VIEW_COUNT = VIEW_COUNT + 1, UPDATE_DT= 2020-06-19 11:22:33 WHERE ID = 1

but, wrong(i guess) SQL was built when I wrote follow code:
UPDATE BOARD_POST_TB SET VIEW_COUNT = 'VIEW_COUNT + 1', USER_ID = '1', UPDATE_DT= '2020-06-19 11:22:33', WHERE ID = 1

    public function addViewCountByPostId(int $postId) : bool {
        $updateResult = $this->where('ID', $postId)
            ->set('VIEW_COUNT', 'VIEW_COUNT + 1', false) // Third parameter is not working
            ->set('USER_ID', '1', true) // because this escape flag is TRUE (for test line)
            ->update();

        return $updateResult;
    }

@lonnieezell Thank you for point a guide! I replaced my code to increment and decrement function. maybe it is my mistake. but I want to know that this is a bug or the user's mistake. I can not see and find the warning about it. and I confused!

Again, thank you a lot for reply my question!

@michalsn
Copy link
Member

@smartse0k to make the story short - enabling/disabling escaping works as it should when you work directly with BaseBuilder class. That's why I asked the follow-up question.

As for Model class, you should use increment()/decrement() as it was mentioned. It's because of the nature of the "escaping" feature currently implemented in a Model - you can only "escape" all data or nothing. The last status you have set for escaping will be adopted to all your data.

@smartse0k
Copy link
Author

I solved this problem.

First, BaseBuilder is working perfectly.

My mistake is that. CI4's controller and another class object can not get the BaseBuilder of model.
So how I solved it, I created a public function in model. and wrote follow code:

public function test() {
$this->builder() // THE builder() is KEY.
    ->set('view_count', 'view_count + 1', false)
    ->set('name', 'hello ci4')
    ->update();
// Last Query: UPDATE `TEST` SET view_count = view_count + 1, `name` = 'hello ci4';
}

When I coded without builder():

public function test() {
$this->set('view_count', 'view_count + 1', false)
    ->set('name', 'hello ci4')
    ->update();
// Last Query: UPDATE `TEST` SET `view_count` = 'view_count + 1', `name` = 'hello ci4';
}

Thank you again for CI4's developer!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
Development

No branches or pull requests

3 participants