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: [QueryBuilder] if set limit to 0, it will return all data. Perhaps it should return 0 data. #8278

Closed
RikhzanAmir opened this issue Nov 30, 2023 · 11 comments · Fixed by #8280
Labels
bug Verified issues on the current code behavior or pull requests that will fix them

Comments

@RikhzanAmir
Copy link

RikhzanAmir commented Nov 30, 2023

PHP Version

8.2

CodeIgniter4 Version

4

CodeIgniter4 Installation Method

Composer (using codeigniter4/appstarter)

Which operating systems have you tested for this bug?

Linux

Which server did you use?

fpm-fcgi

Database

MariaDB 10.2.44

What happened?

$builder->limit(0, 0) , set the limit => 0 and the query will shows all the table rows. It should return no rows because limit is 0.

Steps to Reproduce

$builder->limit(0,0)

Expected Output

it should return no rows if it set limit to 0.

Anything else?

No response

@RikhzanAmir RikhzanAmir added the bug Verified issues on the current code behavior or pull requests that will fix them label Nov 30, 2023
@kenjis
Copy link
Member

kenjis commented Nov 30, 2023

Thank you for reporting. This is very interesting.

$builder->limit(10, 20);
// Produces: LIMIT 20, 10 (in MySQL. Other databases have slightly different syntax)
https://codeigniter4.github.io/CodeIgniter4/database/query_builder.html#limit

As I read it, I expect limit(0, 0) produces LIMIT 0, 0.
In reality, however, no LIMIT clause is generated.

@lonnieezell
Copy link
Member

What is the practical purpose of doing a query where you don't want any results? I'm not sure that I can think of a reason to do that.

Think of it as "there's 0 limit on the number of results I'd like returned."

@kenjis
Copy link
Member

kenjis commented Nov 30, 2023

This behavior has not changed since the beginning of 4.0.

// LIMIT
if ($this->QBLimit)
{
return $this->_limit($sql."\n");
}

@kenjis
Copy link
Member

kenjis commented Nov 30, 2023

In CI3, the following code generates SELECT * FROM "job" LIMIT 0 (sqlite3):

		$jobs = $this->db->limit(0, 0)
		                      ->get('job')
		                      ->result_array();
		// LIMIT
		if ($this->qb_limit !== FALSE OR $this->qb_offset)
		{
			return $this->_limit($sql."\n");
		}

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

@kenjis kenjis changed the title Bug: Query limit, if set limit => 0 , it will return all data. Perhaps it should return 0 data. Bug: [QueryBuilder] if set limit to 0, it will return all data. Perhaps it should return 0 data. Nov 30, 2023
@kenjis
Copy link
Member

kenjis commented Nov 30, 2023

I don't know if this is a bug or not.

Since no one has reported it before, there are probably few use cases where 0 is specified.

@RikhzanAmir
Copy link
Author

I am writing this query in MariaDB: "SELECT * FROM payment limit 0 offset 0" , and it shows no result.

So on Ci3, and Ci4 I am expecting $builder->limit(0,0) or $builder->limit($set, $offset) do the same.

Maybe, it is not a bug, some query do different depending on database we use.

https://dev.mysql.com/doc/refman/8.0/en/limit-optimization.html

"LIMIT 0 quickly returns an empty set. This can be useful for checking the validity of a query. It can also be employed to obtain the types of the result columns within applications that use a MySQL API that makes result set metadata available."

@kenjis
Copy link
Member

kenjis commented Nov 30, 2023

Do you have a use case for limit 0 with Query Builder?

LIMIT is not in SQL standards. So we don't need to implement like MySQL's.
But since "limit 1" returns 1 row, it is logical that "limit 0" would return 0 rows.
However, the implementation will be more complicated than it is now.

@RikhzanAmir
Copy link
Author

If that so, maybe do not change and stick to it. Because on my side it can be fix by adding 1 line of code.

$query = $this->db->table('order')->limit($quantity)->offset($offset)->get()->getResultArray();
return empty($quantity) ? [] : $query;

@kenjis
Copy link
Member

kenjis commented Nov 30, 2023

if ($quantity === 0) {
    return [];
}
$query = $this->db->table('order')->limit($quantity)->offset($offset)->get()->getResultArray();

@kenjis
Copy link
Member

kenjis commented Dec 1, 2023

I sent PR #8280 to fix this.

@kenjis kenjis reopened this Dec 1, 2023
@kenjis
Copy link
Member

kenjis commented Dec 20, 2023

You can fix this behavior in v4.5.0. See #8280

@kenjis kenjis closed this as completed Dec 20, 2023
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

Successfully merging a pull request may close this issue.

3 participants