fix: [#842] The paginate method will report an error when selecting and renaming only one column#1324
Merged
fix: [#842] The paginate method will report an error when selecting and renaming only one column#1324
Conversation
…nd renaming only one column
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.
| Benchmark suite | Current: 7b80a9b | Previous: 6018934 | Ratio |
|---|---|---|---|
Benchmark_Debug |
152665 ns/op 43436 B/op 674 allocs/op |
86363 ns/op 25770 B/op 438 allocs/op |
1.77 |
Benchmark_Debug - ns/op |
152665 ns/op |
86363 ns/op |
1.77 |
Benchmark_Debug - B/op |
43436 B/op |
25770 B/op |
1.69 |
Benchmark_Debug - allocs/op |
674 allocs/op |
438 allocs/op |
1.54 |
Benchmark_Info |
151540 ns/op 43400 B/op 672 allocs/op |
91501 ns/op 25839 B/op 438 allocs/op |
1.66 |
Benchmark_Info - ns/op |
151540 ns/op |
91501 ns/op |
1.66 |
Benchmark_Info - B/op |
43400 B/op |
25839 B/op |
1.68 |
Benchmark_Info - allocs/op |
672 allocs/op |
438 allocs/op |
1.53 |
Benchmark_Warning |
159803 ns/op 44836 B/op 688 allocs/op |
88741 ns/op 25826 B/op 438 allocs/op |
1.80 |
Benchmark_Warning - ns/op |
159803 ns/op |
88741 ns/op |
1.80 |
Benchmark_Warning - B/op |
44836 B/op |
25826 B/op |
1.74 |
Benchmark_Warning - allocs/op |
688 allocs/op |
438 allocs/op |
1.57 |
Benchmark_Error |
189722 ns/op 56459 B/op 831 allocs/op |
113751 ns/op 35086 B/op 570 allocs/op |
1.67 |
Benchmark_Error - ns/op |
189722 ns/op |
113751 ns/op |
1.67 |
Benchmark_Error - B/op |
56459 B/op |
35086 B/op |
1.61 |
Benchmark_Fatal |
5e-7 ns/op 0 B/op 0 allocs/op |
2e-7 ns/op 0 B/op 0 allocs/op |
2.50 |
Benchmark_Fatal - ns/op |
5e-7 ns/op |
2e-7 ns/op |
2.50 |
Benchmark_Panic |
208475 ns/op 62055 B/op 869 allocs/op |
120432 ns/op 38831 B/op 601 allocs/op |
1.73 |
Benchmark_Panic - ns/op |
208475 ns/op |
120432 ns/op |
1.73 |
Benchmark_Panic - B/op |
62055 B/op |
38831 B/op |
1.60 |
This comment was automatically generated by workflow using github-action-benchmark.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v1.16.x #1324 +/- ##
==========================================
Coverage ? 67.97%
==========================================
Files ? 216
Lines ? 11613
Branches ? 0
==========================================
Hits ? 7894
Misses ? 3339
Partials ? 380 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request fixes an error in the Paginate method when selecting and renaming columns, specifically addressing issue #842 where custom select columns interfered with count operations during pagination.
Key Changes:
- Added
resetSelect()method to clear select columns before performing count operations, ensuring accurate pagination counts - Modified
Select()methods to deduplicate columns usingcollect.Unique()to prevent duplicate column selection - Added regression tests to verify that custom select clauses work correctly with pagination
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| database/gorm/query.go | Implemented resetSelect() helper method, updated Count() to reset select before counting, refined Paginate() to use receiver's Count method directly, and added deduplication to Select() method |
| database/db/query.go | Added deduplication logic to Select() method using collect.Unique() |
| tests/query_test.go | Added regression test for issue #842 with select and rename in pagination, updated Where clause syntax to use modern parameter style |
| tests/db_test.go | Added regression test for issue #842 verifying select with alias works correctly in pagination |
| tests/go.mod | Added indirect dependency github.com/urfave/cli/v3 |
| tests/go.sum | Updated checksums for new indirect dependency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📑 Description
Closes goravel/goravel#842
This pull request addresses issues with the
Selectclause 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:
resetSelectmethod to the GORMQueryimplementation to clear select columns before performing a count, preventing incorrect results when custom select columns are used in paginated queries. [1] [2]Selectmethod in bothdatabase/db/query.goanddatabase/gorm/query.goto remove duplicate select columns using the newcollect.Uniqueutility. [1] [2]Paginatemethod to avoid reusing query objects that may have lingering select columns, ensuring accurate pagination and counts.Testing and Regression Coverage:
TestPaginatein bothdb_test.goandquery_test.goto verify that custom select columns do not break pagination and counting, specifically addressing The orm paginate method will report an error when query contains Select goravel#842. [1] [2]Dependency Updates:
github.com/urfave/cli/v3as an indirect dependency intests/go.mod.✅ Checks