Model::paginate() behavior when $page exceeds the last page

[crustamet]

PHP Version

8.3

CodeIgniter4 Version

4.1.5

CodeIgniter4 Installation Method

Manual (zip or tar.gz)

Which operating systems have you tested for this bug?

Linux

Which server did you use?

apache

Database

MySQL 5.6

What happened?

When i paginate the model like this $UsersMode->orderBy('id_user', 'ASC')->paginate(8, 'group', $page_number);

returns "SELECT * FROM users ORDER BY id_user ASC LIMIT 8, 8" based on offset and per page number

But my total count rows in the table are 24, so when i reach the end 16, 8 it returns me the same query as the last existing page in my database even with $page_number = 100

Steps to Reproduce

	public function load_more($page_number = 2)
	{
		$UsersModel 	= new UsersModel();
		
		$this->ViewData['Page'] = $page_number;
		
		$this->ViewData['Users'] = $UsersMode->orderBy('id_user', 'ASC')->paginate(8, 'group', $page_number);

		echo $UsersModel->db->getLastQuery();
	}

returns "SELECT * FROM users ORDER BY id_user ASC LIMIT 8, 8"

But my total count rows in the table are 24, so when i reach the end 16, 8 $page_number = 10 and go to a page number more than 3 it returns the same query "SELECT * FROM users ORDER BY id_user ASC LIMIT 16, 8"

Expected Output

Going to a page number more than the total count rows of my table it should return the propper offset even if i have the total rows less than the per page number or less than the total count

$page_number = 10 it should returns the query "SELECT * FROM users ORDER BY id_user ASC LIMIT 80, 8" even if i don't have enough records in my database table

Anything else?

No response

[crustamet]

one this BaseModel
https://github.com/codeigniter4/CodeIgniter4/blob/develop/system/BaseModel.php#L1273

	public function paginate(?int $perPage = null, string $group = 'default', ?int $page = null, int $segment = 0)
	{
		// Since multiple models may use the Pager, the Pager must be shared.
		$pager = service('pager');

		if ($segment !== 0) {
			$pager->setSegment($segment, $group);
		}

		$page = $page >= 1 ? $page : $pager->getCurrentPage($group);
		// Store it in the Pager library, so it can be paginated in the views.
		$this->pager = $pager->store($group, $page, $perPage, $this->countAllResults(false), $segment);
		$perPage     = $this->pager->getPerPage($group);
		$offset      = ($pager->getCurrentPage($group) - 1) * $perPage;

		return $this->findAll($perPage, $offset);
	}

should be converted to something like this

	public function paginate(?int $perPage = null, string $group = 'default', ?int $page = null, int $segment = 0)
	{
		// Since multiple models may use the Pager, the Pager must be shared.
		$pager = service('pager');

		if ($segment !== 0) {
			$pager->setSegment($segment, $group);
		}

		$page = $page >= 1 ? $page : $pager->getCurrentPage($group);
		// Store it in the Pager library, so it can be paginated in the views.
		$this->pager = $pager->store($group, $page, $perPage, $this->countAllResults(false), $segment);
		$offset      = ($page) * $perPage;
		$perPage     = $this->pager->getPerPage($group);

		return $this->findAll($perPage, $offset);
	}

[kenjis]

@crustamet I don't understand the difference. Please show the diff by diff command or git diff.

[kenjis]

$page_number = 10 it should returns the query "SELECT * FROM users ORDER BY id_user ASC LIMIT 80, 8" even if i don't have enough records in my database table

Why?

[crustamet]

it doesn't matter the difference because how i did it is no good it is just to point out that if the total records in the database are 10

when you go to page 1000

the query remains as of the last page so the limit whould be {a long number}, {8}

I dont have pagination only a load more button

"Why?" - because i cannot remove the button when i have no records, it always gives me records even when i don't have so it always shows load more

[crustamet]

$page_number = 10 it should returns the query "SELECT * FROM users ORDER BY id_user ASC LIMIT 80, 8" even if i don't have enough records in my database table

Why?

First of all it should return that query if you use

$Model->paginate(8, 'group', 10);
it should create the query above ? right or wrong ?

While the current implemented code is dependent on how many rows you actually have in the table

If you have 10 users and you are at page 10

$Model->paginate(8, 'group', 10);
This returns "SELECT * FROM users ORDER BY id_user ASC LIMIT 16, 8"

I cannot return a 404 page if i have no more users on page 10 LOL

[kenjis]

It seems this behavior is intentional.

CodeIgniter4/tests/system/Models/PaginateModelTest.php

Lines 78 to 85 in 8fc8985

	public function testPaginatePageOutOfRange(): void
	{
	$this->createModel(ValidModel::class);
	$this->model->paginate(1, 'default', -500);
	$this->assertSame(1, $this->model->pager->getCurrentPage());
	$this->model->paginate(1, 'default', 500);
	$this->assertSame($this->model->pager->getPageCount(), $this->model->pager->getCurrentPage());
	}

[kenjis]

You can check if it has any more pages with $pager->hasMore().

[crustamet]

Ok i get what you are saying, I dig this $pager->hasMore() method it does what I need a kind of a checker, but i have millions in my database, i just can't wrap my head around this one why it is an intentional behavior. while the query speed is slower when you select from millions at the last page than if you select from millions from a page that does not exists.
Furthermore you don't actually need to add additional code to check this as pagination model will return an empty array if you go out of range

Anyways up to you guys, $pager->hasMore() is missing from the documentation

[kenjis]

I don't know why the pagination behaves like that.
But test code is document that describes how the system works,
so as you see the test code, the current behavior is intentional.

If we change the behavior, it will be an breaking change.

If you insist that this behavior is something that needs to be corrected,
please send a PR to 4.6 branch and explain reasons that many people would agree.
If it is to meet the needs of just one of you, you can simply customize the pagination.

In my opinion, this pagination was not originally designed to handle very large amounts of data at high speed.
If you want to process large amounts of data at high speed,
I recommend a different implementation without using OFFSET.

[crustamet]

I agree with everything you said, actually i will extend the current BaseModel paginate() maybe in time i will understand why it returns data even if it is out of range.

I hope some other guy read this and maybe help us understand better

[neznaika0]

I've known about this behavior for a long time.
At first, this caused a misunderstanding of "Why?" Such a feature, I think, was popular in 2009.
This is not critical at this stage. Problems may appear if you use paginate() for the API (strict result is important).
As a fix, it is possible to create a new strictPaginate() method and remove this behavior

[kenjis]

The method name pagenateStrict() is better, but I agree with adding the new method.

[crustamet]

i made it and tested it
The old one

    public function paginate(?int $perPage = null, string $group = 'default', ?int $page = null, int $segment = 0)
    {
        // Since multiple models may use the Pager, the Pager must be shared.
        $pager = Services::pager();

        if ($segment) {
            $pager->setSegment($segment, $group);
        }

        $page = $page >= 1 ? $page : $pager->getCurrentPage($group);
        // Store it in the Pager library, so it can be paginated in the views.
        $this->pager = $pager->store($group, $page, $perPage, $this->countAllResults(false), $segment);
        $perPage     = $this->pager->getPerPage($group);
        $offset      = ($pager->getCurrentPage($group) - 1) * $perPage;

        return $this->findAll($perPage, $offset);
    }

The new one

	public function paginateStrict(?int $perPage = null, string $group = 'default', ?int $page = null, int $segment = 0)
	{
		$pager = Services::pager();
		
		if ($segment) {
			$pager->setSegment($segment, $group);
		}
	
		$page = $page >= 1 ? $page : $pager->getCurrentPage($group);
		
		// Store it in the Pager library, so it can be paginated in the views.
		$this->pager = $pager->store($group, $page, $perPage, $this->countAllResults(false), $segment);
		
		$perPage     = $this->pager->getPerPage($group);
		$offset      = ($page - 1) * $perPage;
	
		return $this->findAll($perPage, $offset);
	}

[neznaika0]

I've looked at the code.

In a good way, you need to change Pager->store(), $page is illogically set in it, $page has a "raw" value in line 148, and then it in line 154 is changed to limit.

CodeIgniter4/system/Pager/Pager.php

Lines 147 to 154 in 4151c12

	if ($segment > 0 && $this->groups[$group]['currentPage'] > 0) {
	$page = $this->groups[$group]['currentPage'];
	}
	$perPage ??= $this->config->perPage;
	$pageCount = (int) ceil($total / $perPage);
	$this->groups[$group]['currentPage'] = $page > $pageCount ? $pageCount : $page;

Similar actions in BaseModel->paginate(): On line 1282 it has one value, then it is not used for $offset

CodeIgniter4/system/BaseModel.php

Lines 1282 to 1286 in 4151c12

	$page = $page >= 1 ? $page : $pager->getCurrentPage($group);
	// Store it in the Pager library, so it can be paginated in the views.
	$this->pager = $pager->store($group, $page, $perPage, $this->countAllResults(false), $segment);
	$perPage = $this->pager->getPerPage($group);
	$offset = ($pager->getCurrentPage($group) - 1) * $perPage;

So the main reason is to change the currentPage.
Will we find more options instead of editing the BaseModel?