Add INSERT IGNORE and INSERT OR IGNORE support#150
Conversation
Implement support for ignoring insertion conflicts using dialect-native syntax: INSERT IGNORE for MySQL and INSERT OR IGNORE for SQLite. Added a new .Ignore() method to the InsertQuery builder. mapped .OnConflict().DoNothing() to these native keywords for MySQL and SQLite when no conflict targets are provided, matching Drizzle ORM's behavior. This also removes a previous no-op update hack for MySQL DoNothing. Tests added for explicit .Ignore() and implicit conflict-to-ignore translations across targeted dialects. Co-authored-by: cungminh2710 <8063319+cungminh2710@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Greptile SummaryThis PR adds ignore support for insert queries. The main changes are:
Confidence Score: 4/5The insert ignore path needs fixes before merging.
pkg/rain/query_insert.go
|
| Filename | Overview |
|---|---|
| pkg/rain/query_insert.go | Adds ignore rendering and conflict suppression, with edge cases that can silently change conflict behavior. |
| pkg/rain/query_insert_test.go | Adds coverage for MySQL and SQLite ignore output, but misses unsupported dialect and mixed conflict-target cases. |
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
pkg/rain/query_insert.go:546-548
**Postgres Ignore Becomes Plain Insert**
When a Postgres caller uses `.Ignore()` without also adding `.OnConflict()`, this branch marks ignore as active but `writeInsertInto()` has no Postgres rendering case and `writeConflictClause()` returns because no conflict clause exists. The query compiles as a plain `INSERT`, so duplicate-key conflicts still fail instead of being ignored or rejected as unsupported.
### Issue 2 of 3
pkg/rain/query_insert.go:552-553
**MySQL Ignore Broadens Errors**
Existing MySQL callers of `.OnConflict().DoNothing()` now get `INSERT IGNORE` instead of the old no-op `ON DUPLICATE KEY UPDATE` clause. That ignores more than duplicate keys: invalid values, truncation, or NOT NULL problems can be downgraded into warnings and coerced/default data, where the old query would still return an error.
### Issue 3 of 3
pkg/rain/query_insert.go:786-789
**Target Predicate Gets Dropped**
For SQLite, `.OnConflict().Where(predicate).DoNothing()` has no columns or constraint but does have a target predicate. This shortcut treats it as untargeted ignore and emits `INSERT OR IGNORE`, silently discarding the predicate and ignoring all constraint failures instead of reporting the incomplete conflict target.
Reviews (1): Last reviewed commit: "feat(rain): add INSERT IGNORE and INSERT..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if q.ignore { | ||
| return true | ||
| } |
There was a problem hiding this comment.
When a Postgres caller uses .Ignore() without also adding .OnConflict(), this branch marks ignore as active but writeInsertInto() has no Postgres rendering case and writeConflictClause() returns because no conflict clause exists. The query compiles as a plain INSERT, so duplicate-key conflicts still fail instead of being ignored or rejected as unsupported.
Context Used: AGENTS.md (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: pkg/rain/query_insert.go
Line: 546-548
Comment:
**Postgres Ignore Becomes Plain Insert**
When a Postgres caller uses `.Ignore()` without also adding `.OnConflict()`, this branch marks ignore as active but `writeInsertInto()` has no Postgres rendering case and `writeConflictClause()` returns because no conflict clause exists. The query compiles as a plain `INSERT`, so duplicate-key conflicts still fail instead of being ignored or rejected as unsupported.
**Context Used:** AGENTS.md ([source](https://app.greptile.com/hyperlocalise/github/hyperlocalise/rain-orm/-/custom-context?memory=3b24bd19-4fb7-4328-8fc3-52795c121ec6))
How can I resolve this? If you propose a fix, please make it concise.
| if ctx.dialect.Name() == "mysql" { | ||
| return true |
There was a problem hiding this comment.
Existing MySQL callers of .OnConflict().DoNothing() now get INSERT IGNORE instead of the old no-op ON DUPLICATE KEY UPDATE clause. That ignores more than duplicate keys: invalid values, truncation, or NOT NULL problems can be downgraded into warnings and coerced/default data, where the old query would still return an error.
Context Used: AGENTS.md (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: pkg/rain/query_insert.go
Line: 552-553
Comment:
**MySQL Ignore Broadens Errors**
Existing MySQL callers of `.OnConflict().DoNothing()` now get `INSERT IGNORE` instead of the old no-op `ON DUPLICATE KEY UPDATE` clause. That ignores more than duplicate keys: invalid values, truncation, or NOT NULL problems can be downgraded into warnings and coerced/default data, where the old query would still return an error.
**Context Used:** AGENTS.md ([source](https://app.greptile.com/hyperlocalise/github/hyperlocalise/rain-orm/-/custom-context?memory=3b24bd19-4fb7-4328-8fc3-52795c121ec6))
How can I resolve this? If you propose a fix, please make it concise.| if len(q.conflict.columns) == 0 && q.conflict.constraint == "" { | ||
| if q.useIgnore(ctx) { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
For SQLite, .OnConflict().Where(predicate).DoNothing() has no columns or constraint but does have a target predicate. This shortcut treats it as untargeted ignore and emits INSERT OR IGNORE, silently discarding the predicate and ignoring all constraint failures instead of reporting the incomplete conflict target.
Context Used: AGENTS.md (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: pkg/rain/query_insert.go
Line: 786-789
Comment:
**Target Predicate Gets Dropped**
For SQLite, `.OnConflict().Where(predicate).DoNothing()` has no columns or constraint but does have a target predicate. This shortcut treats it as untargeted ignore and emits `INSERT OR IGNORE`, silently discarding the predicate and ignoring all constraint failures instead of reporting the incomplete conflict target.
**Context Used:** AGENTS.md ([source](https://app.greptile.com/hyperlocalise/github/hyperlocalise/rain-orm/-/custom-context?memory=3b24bd19-4fb7-4328-8fc3-52795c121ec6))
How can I resolve this? If you propose a fix, please make it concise.
1 participant
Implement
INSERT IGNORE(MySQL) andINSERT OR IGNORE(SQLite) support in Rain ORM.Feature parity target
Matches Drizzle ORM's behavior for ignoring conflicts during insertion.
What was implemented
.Ignore()method toInsertQuerybuilder.ignorefield toInsertQuerystruct.writeInsertIntohelper inpkg/rain/query_insert.goto render dialect-native ignore syntax (INSERT IGNOREfor MySQL,INSERT OR IGNOREfor SQLite).useIgnorehelper to determine when to use the ignore syntax, including automatic mapping from.OnConflict().DoNothing()when no targets are specified for MySQL/SQLite.writeValuesSQLandwriteSelectSQLto usewriteInsertInto.writeConflictClauseto suppress redundantON DUPLICATE KEY UPDATE(MySQL) orON CONFLICT(SQLite) when ignore is active.writeInsertIntoto be more robust based on code review.How it maps to Drizzle ORM behavior
.ignore()on MySQL and SQLite to render these keywords..onConflictDoNothing()without targets asINSERT IGNORE/INSERT OR IGNOREon these dialects.Tests added or updated
TestInsertIgnoreToSQLinpkg/rain/query_insert_test.gocovering various ignore scenarios.TestInsertOnConflictMySQLto reflect the new, more idiomatic ignore behavior.Commands run
make testgo test -race ./pkg/rainmake fmtmake lintRemaining gaps or follow-up work
PR created automatically by Jules for task 16120343557627706943 started by @cungminh2710