Skip to content

fix: [#842] The paginate method will report an error when selecting and renaming only one column#1325

Merged
hwbrzzl merged 1 commit intomasterfrom
bowen/#842-1
Dec 30, 2025
Merged

fix: [#842] The paginate method will report an error when selecting and renaming only one column#1325
hwbrzzl merged 1 commit intomasterfrom
bowen/#842-1

Conversation

@hwbrzzl
Copy link
Contributor

@hwbrzzl hwbrzzl commented Dec 29, 2025

📑 Description

Closes goravel/goravel#842

This pull request addresses issues with the Select clause in pagination and counting queries, ensuring that select columns do not interfere with count operations and that duplicate columns are removed. It also adds regression tests to prevent similar issues in the future and refines the test queries for clarity.

Database Query Improvements:

  • Added resetSelect method to the GORM Query implementation to clear select columns before performing a count, preventing incorrect results when custom select columns are used in paginated queries. [1] [2]
  • Updated the Select method in both database/db/query.go and database/gorm/query.go to remove duplicate select columns using the new collect.Unique utility. [1] [2]
  • Refactored the Paginate method to avoid reusing query objects that may have lingering select columns, ensuring accurate pagination and counts.

Testing and Regression Coverage:

Dependency Updates:

  • Added github.com/urfave/cli/v3 as an indirect dependency in tests/go.mod.

✅ Checks

  • Added test cases for my code

@hwbrzzl hwbrzzl requested a review from a team as a code owner December 29, 2025 08:26
Copilot AI review requested due to automatic review settings December 29, 2025 08:26
@codecov
Copy link

codecov bot commented Dec 29, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.07%. Comparing base (9bad29e) to head (a74c6dc).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1325   +/-   ##
=======================================
  Coverage   70.07%   70.07%           
=======================================
  Files         282      282           
  Lines       16752    16752           
=======================================
  Hits        11739    11739           
  Misses       4522     4522           
  Partials      491      491           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request fixes an issue where the Paginate method would fail when using a Select clause with column renaming (e.g., Select("name as name")). The fix involves:

  1. Adding a resetSelect() method to clear select columns before performing count operations in pagination
  2. Refactoring the Paginate method to avoid building conditions prematurely, allowing the count to properly exclude select columns
  3. Updating the Select method to append columns instead of replacing them and to remove duplicates
  4. Adding regression tests for the reported issue

Key Changes

  • The Count() method now calls resetSelect() before counting to ensure select columns don't interfere with count queries
  • The Select() method now accumulates columns across multiple calls instead of replacing them, making it consistent with database/db/query.go
  • Duplicate columns are automatically removed using collect.Unique()
  • Test queries updated from Where("name = ?", value) to Where("name", value) for consistency

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
tests/query_test.go Updated test query syntax for consistency and added regression test for issue #842 with select + rename + paginate
tests/db_test.go Added regression test for issue #842 in the DB query builder
database/gorm/query.go Added resetSelect() method, updated Count() to use it, refactored Paginate() to avoid premature condition building, and updated Select() to append columns and remove duplicates
database/db/query.go Updated Select() method to remove duplicate columns and filter out "*" when mixed with other columns

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@hwbrzzl hwbrzzl merged commit 6355289 into master Dec 30, 2025
22 of 23 checks passed
@hwbrzzl hwbrzzl deleted the bowen/#842-1 branch December 30, 2025 03:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The orm paginate method will report an error when query contains Select

3 participants