Merged
Conversation
Ensure wrapWithBackticks has shared test cases for correctness and performance coverage.
Use consistent English comment for wrapWithBackticks to match surrounding style.
Why: - Enable automated setup commands for git worktrees - Standardize worktree initialization process What: - Add worktrees.json with npm install setup command Files: - .cursor/worktrees.json
Why: - Standardize commit workflow with Conventional Commits - Enable automatic splitting of independent changes - Improve commit message quality and consistency What: - Add commit.md command with structured workflow - Define atomic commit rules and message format - Include execution loop and verification steps Files: - .cursor/commands/commit.md
Why: - Improve formatting consistency in documentation - Align emoji formatting with common conventions What: - Remove spaces between emojis and type names in commit types list Files: - .cursor/commands/commit.md
Why: - Improve documentation clarity for commit message format - Explicitly indicate that emojis are part of the type format What: - Update format specification to show "emoji type" instead of "type" Files: - .cursor/commands/commit.md
Why: - Provide alternative commit command with more detailed instructions and explicit commit ordering rules - Enable structured commit workflow with comprehensive safety constraints and verification steps What: - Add commit1.md command file with full workflow documentation - Include detailed repository inspection procedures - Define mandatory multi-commit splitting rules - Specify commit message format and creation process Files: - .cursor/commands/commit1.md
Why: - Consolidate commit commands into single file - Remove redundant alternative command implementation What: - Delete commit1.md command file Files: - .cursor/commands/commit1.md
Why: - Clarify that type format doesn't require emoji prefix - Add spacing between emojis and type names for readability - Include reminder to preserve emojis in commit messages What: - Change format from 'emoji type' to 'type' - Add spaces in types list between emojis and names - Add note to preserve emojis in commit messages Files: - .cursor/commands/commit.md
Why: - Make the commit helper docs shorter and more AI-focused - Emphasize atomic grouping and Conventional Commits while dropping redundant detail What: - Rewrite commit.md as a concise AI Git Automator guide - Keep execution loop, format, and safety rules in a compressed form Files: - .cursor/commands/commit.md
Why: - Standardize commit messages with visual emoji prefixes - Improve commit message readability and categorization What: - Update commit command to mandate emoji in title format - Add emoji map and breaking change notation - Clarify format specification in standards section Files: - .cursor/commands/commit.md
Why: - Make the AI commit helper more precise and concise - Emphasize mandatory emojis and clear grouping rules What: - Rewrite sections for analysis, execution loop, and standards - Add final output summary requirement for commits Files: - .cursor/commands/commit.md
Why: - Standardize code review process with consistent checklist What: - Add SKILL.md with Go-specific review guidelines - Include project-specific controller pattern checks Files: - .cursor/skills/code-review/SKILL.md
Why: - Improve readability of LIKE pattern selection - Make operator handling more explicit and easier to extend What: - Replace if/else chain with switch over contains operators - Preserve existing valueFormat behavior for starts/ends/contains Files: - internal/queryset/queryset.go
Why: - Test names must be unique for proper identification - Error comparisons were incorrect (comparing nil to error) What: - Rename duplicate test case names to unique identifiers - Fix error assertion logic in TestLimit, TestOrderByError, TestGroupByError - Refactor TestLimit to use error message strings Files: - internal/queryset/queryset_test.go
Why: - Typo in variable name reduces code readability - Redundant variable assignment adds unnecessary complexity What: - Rename filedValue to fieldValue in filterHandler - Remove orderByList variable in SliceOrderByToSQL Files: - internal/queryset/queryset.go
Why: - Reduce unnecessary code complexity - Improve readability by avoiding nested if-else blocks What: - Simplify haveError() return statement - Use var declaration for unknownColumns slice - Fix Delete() preCheck params and use update() directly - Flatten nested if-else in CreateOrUpdate() - Update test for new Delete preCheck order Files: - handler.go - handler_test.go
Why: - Previous guidance was too specific and prescriptive - Focus on practical performance considerations What: - Replace strings.Builder advice with general guidance Files: - .cursor/skills/code-review/SKILL.md
Why: - Previous logic checked slice length when err != nil, which is unreliable What: - Simplify error handling to return nil for both nil error and ErrNotFound Files: - handler.go
Why: - return nil prevented transaction rollback on CreateOrUpdate error - && instead of || in assertions required both conditions to fail What: - Change return nil to return err for proper rollback - Change && to || for correct assertion logic Files: - handler_test.go
Why: - Chinese comment reduced codebase consistency - XOR toggle logic was obscure without explanation What: - Translate Chinese comment to English in exposed.go - Add clarifying comment for notFlag^1 toggle in utils.go Files: - exposed.go - internal/queryset/utils.go
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Why: - Skill no longer needed in project What: - Delete code-review SKILL.md Files: - .cursor/skills/code-review/SKILL.md
Why: - Original error assertions had bug: `if err != nil && err.Error() != expected` would pass silently when err is nil - Tests lacked structure for maintainability What: - Fix error assertions: check err == nil first, then validate message - Add table-driven tests for error cases - Add refactored versions: TestGoZeroMysqlTransaction_Refactored, TestGoZeroMysqlMethods_Refactored, TestGoZeroMysqlHandlerError_Refactored - Add getMysqlAddress() helper for env-based config Files: - handler_test.go
Why: - Test coverage incomplete for Select and Having unsupported operations What: - Add Select and Having test cases for FindOne unsupported - Add Select, Having, GroupBy+Having test cases for FindAll unsupported Files: - handler_test.go
Why: - Old test functions TestGoZeroMysqlTransaction and TestGoZeroMysqlMethods are redundant - Refactored versions already exist and provide better coverage What: - Remove TestGoZeroMysqlTransaction and TestGoZeroMysqlMethods - Remove mysqlAddress constant, use getMysqlAddress() helper instead - Consolidate test code to use refactored versions Files: - handler_test.go
Why: - Update with empty map should return error - Error messages should use constants for consistency What: - Add ModelTypeNotStructError and ModelTypeNotSliceError constants - Validate empty update data in update() method - Use constants for model type error messages - Add tests for List unsupported operations - Add tests for model type validation - Add test for empty update validation Files: - handler.go - handler_test.go
Why: - Tests should use constants instead of hardcoded strings for maintainability What: - Replace hardcoded error strings with DataEmptyError constant - Add missing test case for Create with empty map Files: - handler_test.go
Why: - Ensure model helpers correctly handle pointer slices, invalid types, and non-pointer inputs. What: - Add test for pointer slice input in modelStructSlice2MapSlice. - Add test for pointer struct input in createModelPointerAndSlice. - Add test for non-pointer input in deepCopyModelPtrStructure. Files: - utils_test.go
Why: - Strengthen behavioural guarantees and unsupported-path coverage for the MySQL handler API. What: - Add tests for empty FindOne/FindAllModel results. - Add CreateIfNotExist existing-row behaviour test. - Add batch Create tests for model slices and slice pointers. - Add unsupported-operation tests for Create/Update/Remove/Delete and helper methods. - Add additional data-empty, nil-filter, and prior-error propagation tests. Files: - handler_test.go
Why: - Where clause arguments were not variadically forwarded, causing incorrect SQL parameter binding. - Additional behaviour and unsupported-path coverage is needed for transactions and Having-based helpers. What: - Fix Impl.Where to call WhereToSQL with variadic args. - Add transaction rollback visibility test with session. - Add Where success path test in methods suite. - Add Having unsupported tests for GetOrCreate, CreateOrUpdate and CreateIfNotExist. Files: - handler.go - handler_test.go
Why: - Ensure transactional updates and Reset semantics behave correctly within a session and roll back cleanly. What: - Add test verifying update visibility inside a transaction and rollback removes rows. - Add test ensuring Reset keeps session binding while clearing query state. Files: - handler_test.go
Why: - Where conditions allowed mismatched argument usage and some queryset branches lacked regression coverage. What: - Enforced strict alignment between SQL placeholders and args and expanded queryset/filter utilities and conflict handling tests. Files: - internal/queryset/queryset.go internal/queryset/queryset_test.go internal/queryset/utils_test.go
Why: - Handler and go-zero mysql operator had branched NotFound handling and missing coverage for various DB error scenarios. What: - Simplified FindAll/FindAllModel error paths and aligned go-zero operator behaviour while adding tests to exercise bad schema/table and CRUD error branches. Files: - handler.go handler_test.go operator/mysql/go-zero/methods.go
Why: - Align with handler/mysql behaviour: empty result is success, not not-found. What: - Remove ErrNotFound when FindAll returns zero rows in clickhouse-go operator. Files: - operator/clickhouse/clickhouse-go/methods.go Made-with: Cursor
Why: - SetLevel could run before any Get(), leaving c uninitialized; init logic was duplicated. What: - Extract ensureInit(), call from Get and SetLevel; single place for init. Files: - internal/config/hadler.go Made-with: Cursor
Why: - Compilation failed because logx.LogConf.Level does not exist in current go-zero API. What: - Drop level-mapping logic from NewMysql and rely on default go-zero logging configuration. Files: - operator/mysql/go-zero/methods.go Made-with: Cursor
| if len(orderByVal) == 0 { | ||
| return m | ||
| } | ||
| unknownColumns := []string{} |
There was a problem hiding this comment.
Having args not spread like fixed Where method
High Severity
The Having method calls m.qs.HavingToSQL(having, args) without the spread operator ..., while the identical pattern in Where was fixed in this commit to use args.... Since HavingToSQL accepts variadic ...any, passing args (a []any) without spreading wraps it in an extra layer, resulting in []any{[]any{actual_args}} instead of []any{actual_args}. This causes SQL parameter binding to fail at runtime when Having is used with arguments.
Additional Locations (1)
Why: - Replace sqlx BulkInserter with single multi-row INSERT for predictable behaviour and duplicate-key handling. What: - Build bulk INSERT by repeating VALUES placeholder; add buildBulkInsertQuery and isDuplicateKeyError. - Use go-sql-driver/mysql for 1062 duplicate key detection in Insert and BulkInsert. - Alias internal/operator/mysql to mysqlOp. Add unit tests for buildBulkInsertQuery. Files: - operator/mysql/go-zero/methods.go - operator/mysql/go-zero/methods_test.go Made-with: Cursor
Why: - Ensure batch create preserves datetime fields (CreateTime, UpdateTime) for struct slices. What: - New subtest creates two rows via Create([]test.Source), asserts length and datetime drift within 1s. Files: - handler_test.go Made-with: Cursor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
operator/mysql/go-zero/methods.go
Outdated
|
|
||
| blk.Flush() | ||
| lower := strings.ToLower(query) | ||
| pos := strings.Index(lower, "values") |
There was a problem hiding this comment.
buildBulkInsertQuery matches "values" in column or table names
Low Severity
buildBulkInsertQuery uses strings.Index(lower, "values") to locate the SQL VALUES keyword, but this naively matches the first occurrence of the substring "values" anywhere in the query — including inside backtick-quoted column names or unquoted table names. If a column or table name contains "values" (e.g., values_log, default_values), the function either produces garbled SQL or returns a spurious error, causing bulk inserts to fail.
Why: - reflect.Ptr is deprecated since Go 1.18, linter flags it What: - Replace all reflect.Ptr usages with reflect.Pointer Files: - handler.go Made-with: Cursor
Why: - Map-based FindOne/FindAll previously could not safely use Select or express intent on returned columns. What: - Allow Select with FindOne/FindAll/List while rejecting aliases and qualified wildcards in map APIs. - Add robust SQL select parsing helpers and map filtering by selected columns. - Extend unit tests and benchmarks to cover select strings, aliases, and wildcard validation paths. Files: - handler.go, handler_bench_test.go, handler_test.go - utils.go, utils_test.go Made-with: Cursor
Why: - Prevent IDE-specific .cursor files from being tracked and committed What: - Add .cursor directory to .gitignore so Cursor metadata stays local Files: - .gitignore Made-with: Cursor
Why: - Remove redundant switch on error that added no behavioral value. What: - Replace switch-based error handling in FindAllModel with a direct return of the underlying error. Files: - handler.go Made-with: Cursor
Why: - Having wrapped args slice one level too deep, breaking SQL binding when using placeholders What: - Pass variadic args to HavingToSQL using args... Files: - handler.go Made-with: Cursor
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on April 20. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
Why: - VALUES keyword could be matched inside ON DUPLICATE KEY UPDATE ... VALUES() or in identifiers. What: - Restrict search to text before " on duplicate key" and use LastIndex for VALUES. - Range over valueTemplates in loop instead of rows. Files: - operator/mysql/go-zero/methods.go Made-with: Cursor
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on April 20. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
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
1 participant
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.
Note
High Risk
Touches core query building and DB operator behavior (Select validation/filtering, bulk insert execution, and
ErrNotFoundsemantics), which can change runtime results for existing consumers despite expanded test coverage.Overview
Query/controller behavior changes:
FindOne/FindAll(map-returning) now allowSelectbut reject select aliases and qualified wildcards (e.g.table.*), and will filter returned maps to the selected columns.Updatenow errors on empty data,Deleteroutes directly through the internal soft-delete update path, and some type/error messages were standardized.Operator/queryset changes: MySQL go-zero
BulkInsertwas reworked to build a multi-rowVALUESquery and execute it directly (with typed duplicate-key detection), andFindAll/ClickHouseFindAllno longer translate empty results toErrNotFound.WhereToSQLnow errors when args are provided without placeholders, plus small refactors/benchmarks inquerysetutilities.Tests/tooling: Large refactor and expansion of handler/queryset tests (including new
Selectedge cases, transaction coverage, and env-configurable MySQL DSN), plus new benchmarks for handler/queryset;.cursorworkflow files were added and.cursorwas added to.gitignore.Written by Cursor Bugbot for commit 446383b. This will update automatically on new commits. Configure here.