Skip to content

[13.x] Expose new countColumns parameter to control the columns of getCountForPagination#60288

Closed
pfabri-ebrand wants to merge 1 commit into
laravel:13.xfrom
pfabri-ebrand:expose-columns-property-for-count-pagination-query
Closed

[13.x] Expose new countColumns parameter to control the columns of getCountForPagination#60288
pfabri-ebrand wants to merge 1 commit into
laravel:13.xfrom
pfabri-ebrand:expose-columns-property-for-count-pagination-query

Conversation

@pfabri-ebrand
Copy link
Copy Markdown

@pfabri-ebrand pfabri-ebrand commented May 27, 2026

Summary

The idea behind this PR is to be able to control the fields used by the getCountForPagination/runPaginationCountQuery and have slightly better performant queries when the count is done on table with many fields

@pfabri-ebrand pfabri-ebrand marked this pull request as draft May 27, 2026 09:15
@pfabri-ebrand pfabri-ebrand marked this pull request as ready for review May 27, 2026 09:18
Comment on lines +992 to +993
* @param int|null|\Closure $total
* @param array $countColumns
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may want to narrow this further down

Suggested change
* @param int|null|\Closure $total
* @param array $countColumns
* @param int|null|\Closure(): int $total
* @param array<int, string> $countColumns

or

Suggested change
* @param int|null|\Closure $total
* @param array $countColumns
* @param int|null|\Closure(): int $total
* @param string[] $countColumns

or

Suggested change
* @param int|null|\Closure $total
* @param array $countColumns
* @param int|null|\Closure(): int $total
* @param list<string> $countColumns

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just matching the existing phpdoc style

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's always a chance to improve, but not if one always takes the worst example as their guiding principle 😉

@taylorotwell
Copy link
Copy Markdown
Member

Thanks for your pull request to Laravel!

Unfortunately, I'm going to delay merging this code for now. To preserve our ability to adequately maintain the framework, we need to be very careful regarding the amount of code we include.

If applicable, please consider releasing your code as a package so that the community can still take advantage of your contributions!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants