bcit-ci / CodeIgniter

Open Source PHP Framework (originally from EllisLab)
https://codeigniter.com/
MIT License
18.27k stars 7.6k forks source link

Bug: inconsistent behavior of Query Builder limit 0 #6253

Open kenjis opened 11 months ago

kenjis commented 11 months ago

$this->db->limit(0)->get('table') returns no records. But $this->db->get('table', 0) returns all records.

Steps to reproduce:

        $config['database'] = './test.db';
        $config['dbdriver'] = 'sqlite3';
        $this->load->database($config);

        $this->load->dbforge();
        $fields = array(
            'name' => array(
                'type' => 'VARCHAR',
                'constraint' => '100',
            ),
        );
        $this->dbforge->add_field('id');
        $this->dbforge->add_field($fields);
        $this->dbforge->create_table('test', TRUE);

        $data = array(
            array(
                'name' => 'My Name',
            ),
            array(
                'name' => 'Another Name',
            ),
            array(
                'name' => 'Other Name',
            ),
        );
        $this->db->insert_batch('test', $data);

        echo '$this->db->limit(0)->get(\'test\')' . PHP_EOL;
        $query = $this->db->limit(0)->get('test');
        foreach ($query->result() as $row)
        {
            echo $row->name . PHP_EOL;
        }

        echo '$this->db->get(\'test\', 0)' . PHP_EOL;
        $query = $this->db->get('test', 0);
        foreach ($query->result() as $row)
        {
            echo $row->name . PHP_EOL;
        }

Result:

$this->db->limit(0)->get('test')
$this->db->get('test', 0)
My Name
Another Name
Other Name
poodle123 commented 11 months ago

This is no a bug but design. $this->db->get('table', 0) means no limit not zero records.

kenjis commented 11 months ago

No, it is not documented like that.

$query = $this->db->get('mytable', 10, 20); // Executes: SELECT * FROM mytable LIMIT 20, 10 https://www.codeigniter.com/userguide3/database/query_builder.html#selecting-data

https://github.com/bcit-ci/CodeIgniter/blob/63d037565dd782795021dcbd3b1ca6f8c8a509e4/system/database/DB_query_builder.php#L1438-L1443

poodle123 commented 11 months ago

interesting. seems i misunderstood it then. I looked into DB_query_builder.php and it seems you are right. 0 should indeed be passed as a limit. Do you have the generated SQL query from your code? Just to be sure if the code is generated correctly or not?

kenjis commented 11 months ago
$this->db->limit(0)->get('test')
SELECT *
FROM "test"
 LIMIT 0
$this->db->get('test', 0)
SELECT *
FROM "test"
poodle123 commented 11 months ago

This is what I mentioned originally: 0 is not passed along due to this in the DB_query_builder.php:

    public function limit($value, $offset = 0)
    {
        is_null($value) OR $this->qb_limit = (int) $value;
        empty($offset) OR $this->qb_offset = (int) $offset;

        return $this;
    }

giving you all records when you set limit to zero.

If you change it to

public function limit($value, $offset = 0)
{
    $this->qb_limit = ($value === 0) ? 0 : (int) $value;
    empty($offset) OR $this->qb_offset = (int) $offset;

    return $this;
}

It should return what you expect. But again my understanding was that 0 was supposed to be interpreted als "do not set a limit" but I might be wrong.

kenjis commented 11 months ago

No, no. limit(0) returns 0 records. It is a correct behavior. The issue is that $this->db->get('table', 0) returns all records.

php > $value = 0;
php > is_null($value) OR $qb_limit = (int) $value;
php > var_dump($qb_limit);
php shell code:1:
int(0)

But again my understanding was that 0 was supposed to be interpreted als "do not set a limit" but I might be wrong.

LIMIT 0 returns 0 records. Try MySQL or SQLite3 or PostgreSQL.

poodle123 commented 11 months ago

I know that limit 0 returns 0 records in MySQL. But I understood the CI way differently: namely limit 0 = no limit,

I have no other ideas why limit is dropped. Sorry.

kenjis commented 11 months ago

It seems CI3 distinguishes between $limit = 0 and $limit = NULL. So it is natural to interpret $limit = NULL means no limit, $limit = 0 means zero records. In fact, limit() behaves like that. Therefore I think the behavior of get() is a bug.