test(contrib/drivers/mysql): add pagination and error handling tests - #4703
Merged
gqcn merged 2 commits intoFeb 27, 2026
Merged
Conversation
This was referenced Feb 13, 2026
lingcoder
added a commit
to lingcoder/gf
that referenced
this pull request
Feb 13, 2026
## Summary Fix bug where AllAndCount(true) with multiple fields generates invalid SQL COUNT(field1, field2, ...) causing syntax error. ## Root Cause When useFieldForCount=true, the COUNT query inherits the fields configuration from the model, generating COUNT(multiple fields) which is invalid SQL syntax. ## Fix Always use COUNT(1) regardless of useFieldForCount parameter since COUNT() accepts only one argument. Applied to both AllAndCount() and ScanAndCount() methods in database/gdb/gdb_model_select.go. ## Tests Added Test_Issue4698 in contrib/drivers/mysql/mysql_z_unit_issue_test.go with 5 test cases covering all scenarios. Updated SQLiteCGo tests to expect correct behavior after framework fix. Fixes gogf#4698 Ref gogf#4703
lingcoder
added a commit
to lingcoder/gf
that referenced
this pull request
Feb 13, 2026
## Summary Fix bug where AllAndCount(true) with multiple fields generates invalid SQL COUNT(field1, field2, ...) causing syntax error. ## Root Cause When useFieldForCount=true, the COUNT query inherits the fields configuration from the model, generating COUNT(multiple fields) which is invalid SQL syntax. ## Fix Always use COUNT(1) regardless of useFieldForCount parameter since COUNT() accepts only one argument. Applied to both AllAndCount() and ScanAndCount() methods in database/gdb/gdb_model_select.go. ## Tests Added Test_Issue4698 in contrib/drivers/mysql/mysql_z_unit_issue_test.go with 5 test cases covering all scenarios. Updated SQLiteCGo tests to expect correct behavior after framework fix. Fixes gogf#4698 Ref gogf#4703
lingcoder
force-pushed
the
test/mysql-pagination-error-handling
branch
from
February 13, 2026 13:48
7f29c65 to
2433f49
Compare
This was referenced Feb 13, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR expands the MySQL driver unit test suite to cover pagination APIs and a variety of error/edge-case behaviors, aligned with the broader driver test coverage plan in #4689.
Changes:
- Add pagination-focused tests for
AllAndCount,ScanAndCount,Chunk, and boundary behavior forPage/Limit. - Add error-handling/edge-case tests for invalid inputs, invalid SQL fragments, empty results, context cancellation, and transaction rollback.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| contrib/drivers/mysql/mysql_z_unit_feature_pagination_test.go | New test suite covering pagination helpers and boundary conditions. |
| contrib/drivers/mysql/mysql_z_unit_feature_error_handling_test.go | New test suite covering invalid operations, empty-result behavior, SQL-injection safety checks, and transaction rollback. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
gqcn
pushed a commit
that referenced
this pull request
Feb 26, 2026
## Summary
Fix bug where `Fields("")` with empty string generates invalid SQL
`SELECT FROM table`.
## Root Cause
`mappingAndFilterToTableFields` method doesn't skip empty strings when
processing fields:
- `gstr.SplitAndTrim("", ",")` returns empty array
- No fields added to query
- Results in invalid SQL: `SELECT FROM table`
## Fix
Skip empty string fields in `mappingAndFilterToTableFields` (line
97-100):
```go
// Skip empty string fields
if fieldStr == "" {
continue
}
```
## Behavior Changes
- `Fields("")` → SELECT * FROM table (uses default)
- `Fields("", "id")` → SELECT id FROM table (ignores empty string)
- `Fields("id", "", "nickname")` → SELECT id, nickname FROM table
## Tests
Added `Test_Issue4697` with 3 scenarios covering all cases above.
## Related
Fixes #4697
Ref #4703 (discovered during pagination test development)
lingcoder
added a commit
to lingcoder/gf
that referenced
this pull request
Feb 26, 2026
Fix bug where AllAndCount(true) with multiple fields generates invalid SQL COUNT(field1, field2, ...) causing syntax error. When useFieldForCount=true, the COUNT query inherits the fields configuration from the model, generating COUNT(multiple fields) which is invalid SQL syntax. Always use COUNT(1) regardless of useFieldForCount parameter since COUNT() accepts only one argument. Applied to both AllAndCount() and ScanAndCount() methods in database/gdb/gdb_model_select.go. Added Test_Issue4698 in contrib/drivers/mysql/mysql_z_unit_issue_test.go with 5 test cases covering all scenarios. Updated SQLiteCGo tests to expect correct behavior after framework fix. Fixes gogf#4698 Ref gogf#4703
lingcoder
force-pushed
the
test/mysql-pagination-error-handling
branch
from
February 26, 2026 15:15
2433f49 to
4f632c7
Compare
gqcn
pushed a commit
that referenced
this pull request
Feb 27, 2026
…4702) ## Summary Fix bug where negative values in `Limit()`, `Page()`, and `Offset()` methods generate invalid SQL causing database errors. ## Root Cause The methods don't validate negative input: - `Limit(-1)` generates `LIMIT -1` → SQL error - `Page(1, -10)` generates `LIMIT -10` → SQL error - `Offset(-5)` generates `OFFSET -5` → SQL error ## Fix Treat all negative values as zero (safe default): **Limit() method**: ```go case 1: if limit[0] < 0 { limit[0] = 0 } case 2: if limit[0] < 0 { limit[0] = 0 } if limit[1] < 0 { limit[1] = 0 } ``` **Page() method**: ```go if limit < 0 { limit = 0 } ``` **Offset() method**: ```go if offset < 0 { offset = 0 } ``` ## Behavior Changes - `Limit(-1)` → `Limit(0)` (no limit) - `Limit(-10, -5)` → `Limit(0, 0)` (no offset, no limit) - `Page(1, -10)` → `Page(1, 0)` (no results) - `Offset(-5)` → `Offset(0)` (no offset) ## Documentation Added "Note: Negative values are treated as zero" to all three methods. ## Tests Added `Test_Issue4699` in `database/gdb/gdb_z_unit_issue_test.go` with 7 test cases: 1. Limit with single negative parameter 2. Limit with two negative parameters 3. Limit with mixed parameters (negative start, positive limit) 4. Page with negative limit 5. Page with negative limit on page 2 6. Offset with negative value 7. Offset with positive value (sanity check) ## Related Fixes #4699 Ref #4703 (discovered during pagination test development)
gqcn
pushed a commit
that referenced
this pull request
Feb 27, 2026
…n when parameter `useFieldForCount` is true in AllAndCount/ScanAndCount (#4701) ## Summary Fix bug where `AllAndCount(true)` with multiple fields generates invalid SQL `COUNT(field1, field2, ...)` causing syntax error. ## Root Cause When `useFieldForCount=true`, the COUNT query inherits the fields configuration from the model: ```go // Before (buggy code) if !useFieldForCount { countModel.fields = []any{Raw("1")} } // When useFieldForCount=true, fields remain as ["id", "nickname"] // Generates: SELECT COUNT(id, nickname) FROM table ❌ ``` ## Fix Always use `COUNT(1)` regardless of `useFieldForCount` parameter since COUNT() accepts only one argument: ```go // After (fixed code) // Always use COUNT(1) for counting, regardless of useFieldForCount. // COUNT() accepts only one argument, so we can't use multiple fields. countModel.fields = []any{Raw("1")} ``` Applied to both `AllAndCount()` and `ScanAndCount()` methods. ## Tests Added `Test_Issue4698` with 5 test cases: 1. AllAndCount(true) with multiple fields 2. AllAndCount(false) with multiple fields (baseline) 3. ScanAndCount with multiple fields 4. AllAndCount with single field 5. AllAndCount with WHERE condition All tests verify that COUNT generates valid SQL and returns correct count. ## Related Fixes #4698 Ref #4703 (discovered during pagination test development)
Add 55 new test functions (1,011 lines) to improve MySQL driver test coverage. ## New Test Files ### Pagination Tests (22 functions, 531 lines) - AllAndCount: 7 tests covering basic usage, WHERE conditions, pagination, field selection, empty results, cache support, DISTINCT queries - ScanAndCount: 7 tests covering basic usage, WHERE conditions, pagination, single record, empty results, field selection, cache support - Chunk: 5 tests covering basic iteration, early stop, WHERE conditions, error handling, empty results - Page/Limit: 3 tests covering boundary values (0, negative, beyond data), combination usage ### Error Handling Tests (33 functions, 480 lines) - Nil/empty data handling: Insert/Update with nil, empty map, empty slice - Missing WHERE clause: Update/Delete without WHERE - Scan errors: nil pointer, invalid pointer type, empty result - Invalid SQL: invalid operators, non-existent fields/tables - SQL injection prevention: WHERE clause, Insert, Update (parameterized queries) - Aggregate functions: Max/Min/Avg/Sum with empty results - Context: cancelled context handling - Transaction: rollback after error - Duplicate key errors - Invalid connections ## Framework Bugs Discovered During test development, discovered 3 framework edge case bugs: - gogf#4697: Fields("") should handle empty string gracefully - Regression test: Test_Model_Fields_Empty - gogf#4697 - gogf#4698: AllAndCount(true) with multiple fields should work correctly - Regression test: Test_Model_AllAndCount_WithFields - gogf#4698 - gogf#4699: Negative Limit/Page values should be sanitized - Regression tests: Test_Model_Page_Boundary, Test_Model_Limit_Boundary - gogf#4699 These tests define correct expected behavior and will pass once bugs are fixed. ## Test Quality - All tests use gtest.C() wrapper pattern - Independent test execution (no dependencies) - Comprehensive boundary/edge case coverage - Proper error validation - Code formatted with gofmt and gci ref gogf#4689
lingcoder
force-pushed
the
test/mysql-pagination-error-handling
branch
from
February 27, 2026 08:46
4f632c7 to
696ffc2
Compare
Replace t.Assert(len(result) <= 1) with t.AssertLE(len(result), 1). t.Assert requires two arguments (actual, expected) but was given a boolean expression.
lingcoder
force-pushed
the
test/mysql-pagination-error-handling
branch
from
February 27, 2026 09:36
696ffc2 to
e0a9b29
Compare
gqcn
approved these changes
Feb 27, 2026
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.
Summary
Test coverage added:
Ref #4689
Test plan