Skip to content

feat: implement InsertQuery.Ignore() #152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
cungminh2710 wants to merge 1 commit into from
Closed

feat: implement InsertQuery.Ignore() #152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 51 additions & 5 deletions pkg/rain/query_insert.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ type InsertQuery struct {
columns []schema.ColumnReference
returning []schema.Expression
conflict *insertConflictClause
ignore bool
ctes []cteDefinition
defaultValues bool

Expand Down Expand Up @@ -210,6 +211,14 @@ func (b *InsertConflictBuilder) DoUpdateSet(columns ...schema.ColumnReference) *
return &InsertConflictUpdateBuilder{query: b.query}
}

// Ignore configures the INSERT to ignore conflict errors.
// PostgreSQL renders "ON CONFLICT DO NOTHING", while MySQL renders "INSERT IGNORE"
// and SQLite renders "INSERT OR IGNORE".
func (q *InsertQuery) Ignore() *InsertQuery {
q.ignore = true
return q
}

// Returning adds RETURNING expressions when supported by the dialect.
func (q *InsertQuery) Returning(exprs ...schema.Expression) *InsertQuery {
q.returning = append(q.returning, exprs...)
Expand Down Expand Up @@ -287,7 +296,7 @@ func (q *InsertQuery) writeValuesSQL(ctx *compileContext) error {
if err := q.validateSources(); err != nil {
return err
}
ctx.writeString("INSERT INTO ")
q.writeInsertInto(ctx)
ctx.writeTableName(q.table)
if ctx.dialect.Name() == "mysql" {
ctx.writeString(" () VALUES ()")
Expand All @@ -299,7 +308,7 @@ func (q *InsertQuery) writeValuesSQL(ctx *compileContext) error {
return err
}

ctx.writeString("INSERT INTO ")
q.writeInsertInto(ctx)
ctx.writeTableName(q.table)

if q.models != nil {
Expand All @@ -325,6 +334,33 @@ func (q *InsertQuery) writeValuesSQL(ctx *compileContext) error {
return ctx.writeReturning(q.returning, q.returningClause())
}

func (q *InsertQuery) useIgnore() bool {
if q.ignore {
return true
}
if q.conflict == nil || q.conflict.action != insertConflictActionDoNothing {
return false
}
// For MySQL and SQLite, OnConflict().DoNothing() with no targets is
// equivalent to INSERT [OR] IGNORE.
return (q.dialect.Name() == "mysql" || q.dialect.Name() == "sqlite") &&
len(q.conflict.columns) == 0 && q.conflict.constraint == ""
}
Comment on lines +337 to +348

@greptile-apps greptile-apps Bot Jul 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 MySQL OnConflict().DoNothing() now suppresses more errors than before

The previous ON DUPLICATE KEY UPDATE id = id pattern was precise: it only handled duplicate-key violations and surfaced all other errors (data truncation, foreign-key failures, invalid column values, etc.) normally. INSERT IGNORE INTO is broader — MySQL converts all those error classes to warnings and continues. Existing callers relying on MySQL error propagation for non-duplicate-key failures on targetless OnConflict().DoNothing() will silently swallow errors they were previously seeing. This is a semantic breaking change even though the test expectation was updated to match.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: pkg/rain/query_insert.go
Line: 337-348

Comment:
**MySQL `OnConflict().DoNothing()` now suppresses more errors than before**

The previous `ON DUPLICATE KEY UPDATE id = id` pattern was precise: it only handled duplicate-key violations and surfaced all other errors (data truncation, foreign-key failures, invalid column values, etc.) normally. `INSERT IGNORE INTO` is broader — MySQL converts all those error classes to warnings and continues. Existing callers relying on MySQL error propagation for non-duplicate-key failures on targetless `OnConflict().DoNothing()` will silently swallow errors they were previously seeing. This is a semantic breaking change even though the test expectation was updated to match.

How can I resolve this? If you propose a fix, please make it concise.

Fix in Codex


func (q *InsertQuery) writeInsertInto(ctx *compileContext) {
if q.useIgnore() {
switch q.dialect.Name() {
case "mysql":
ctx.writeString("INSERT IGNORE INTO ")
return
case "sqlite":
ctx.writeString("INSERT OR IGNORE INTO ")
return
}
}
ctx.writeString("INSERT INTO ")
}

func (q *InsertQuery) writeModelsSQL(ctx *compileContext) error {
value := reflect.ValueOf(q.models)
for value.Kind() == reflect.Pointer {
Expand Down Expand Up @@ -648,7 +684,7 @@ func (q *InsertQuery) writeSelectSQL(ctx *compileContext) error {
selectQuery = selectQuery.withSQLiteInsertSelectConflictWhere()
}

ctx.writeString("INSERT INTO ")
q.writeInsertInto(ctx)
ctx.writeTableName(q.table)

if len(q.columns) > 0 {
Expand Down Expand Up @@ -765,6 +801,11 @@ func (q *InsertQuery) validateSources() error {
}

func (q *InsertQuery) writeConflictClause(ctx *compileContext) error {
if q.ignore && q.dialect.Name() == "postgres" {
ctx.writeString(" ON CONFLICT DO NOTHING")
return nil
}

if q.conflict == nil {
return nil
}
Expand All @@ -777,6 +818,9 @@ func (q *InsertQuery) writeConflictClause(ctx *compileContext) error {
}

if q.dialect.Name() == "mysql" {
if q.useIgnore() {
return nil
}
if len(q.conflict.columns) > 0 || q.conflict.constraint != "" || len(q.conflict.targetWhere) > 0 {
Comment on lines 803 to 824

@greptile-apps greptile-apps Bot Jul 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Ignore() silently discards an explicit OnConflict(...).DoUpdateSet(...) clause

When q.ignore = true, writeConflictClause returns early for every dialect — Postgres at line 804, MySQL via useIgnore() at line 821, SQLite via useIgnore() at line 863 — before any DoUpdateSet logic is reached. A user who calls .Ignore().OnConflict(col).DoUpdateSet(other) (or the reverse order) gets INSERT IGNORE / ON CONFLICT DO NOTHING with no update applied and no error raised. This silently drops an explicit upsert intent and will produce wrong data in any caller expecting the update to fire.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: pkg/rain/query_insert.go
Line: 803-824

Comment:
**`Ignore()` silently discards an explicit `OnConflict(...).DoUpdateSet(...)` clause**

When `q.ignore = true`, `writeConflictClause` returns early for every dialect — Postgres at line 804, MySQL via `useIgnore()` at line 821, SQLite via `useIgnore()` at line 863 — before any DoUpdateSet logic is reached. A user who calls `.Ignore().OnConflict(col).DoUpdateSet(other)` (or the reverse order) gets `INSERT IGNORE / ON CONFLICT DO NOTHING` with no update applied and no error raised. This silently drops an explicit upsert intent and will produce wrong data in any caller expecting the update to fire.

How can I resolve this? If you propose a fix, please make it concise.

Fix in Codex

return errors.New("rain: MySQL ON DUPLICATE KEY UPDATE does not support conflict targets (columns, constraints, or WHERE); call OnConflict() without modifiers")
}
Expand Down Expand Up @@ -816,8 +860,8 @@ func (q *InsertQuery) writeConflictClause(ctx *compileContext) error {
return nil
}

if len(q.conflict.columns) == 0 && q.conflict.constraint == "" {
return errors.New("rain: conflict clause requires at least one target (columns or constraint)")
if q.dialect.Name() == "sqlite" && q.useIgnore() {
return nil
}

ctx.writeString(" ON CONFLICT")
Expand All @@ -836,6 +880,8 @@ func (q *InsertQuery) writeConflictClause(ctx *compileContext) error {
ctx.writeColumnName(col)
}
ctx.writeByte(')')
} else if q.conflict.action != insertConflictActionDoNothing {
return errors.New("rain: conflict clause requires at least one target (columns or constraint)")
}

if len(q.conflict.targetWhere) > 0 {
Expand Down
165 changes: 164 additions & 1 deletion pkg/rain/query_insert_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,169 @@ func TestInsertOnConflictPostgres(t *testing.T) {
})
}

func TestInsertIgnoreToSQL(t *testing.T) {
t.Parallel()

users, _ := defineTables()

t.Run("mysql insert ignore", func(t *testing.T) {
db, _ := rain.OpenDialect("mysql")
sqlText, _, err := db.Insert().
Table(users).
Set(users.Email, "alice@example.com").
Ignore().
ToSQL()
if err != nil {
t.Fatalf("ToSQL returned error: %v", err)
}

wantSQL := "INSERT IGNORE INTO `users` (`email`) VALUES (?)"
if sqlText != wantSQL {
t.Fatalf("unexpected SQL:\nwant: %s\ngot: %s", wantSQL, sqlText)
}
})

t.Run("sqlite insert or ignore", func(t *testing.T) {
db, _ := rain.OpenDialect("sqlite")
sqlText, _, err := db.Insert().
Table(users).
Set(users.Email, "alice@example.com").
Ignore().
ToSQL()
if err != nil {
t.Fatalf("ToSQL returned error: %v", err)
}

wantSQL := `INSERT OR IGNORE INTO "users" ("email") VALUES (?)`
if sqlText != wantSQL {
t.Fatalf("unexpected SQL:\nwant: %s\ngot: %s", wantSQL, sqlText)
}
})

t.Run("postgres ignore renders on conflict do nothing", func(t *testing.T) {
db, _ := rain.OpenDialect("postgres")
sqlText, _, err := db.Insert().
Table(users).
Set(users.Email, "alice@example.com").
Ignore().
ToSQL()
if err != nil {
t.Fatalf("ToSQL returned error: %v", err)
}

wantSQL := `INSERT INTO "users" ("email") VALUES ($1) ON CONFLICT DO NOTHING`
if sqlText != wantSQL {
t.Fatalf("unexpected SQL:\nwant: %s\ngot: %s", wantSQL, sqlText)
}
})
}

func TestInsertOnConflictDoNothingTargetless(t *testing.T) {
t.Parallel()

users, _ := defineTables()

t.Run("mysql renders insert ignore", func(t *testing.T) {
db, _ := rain.OpenDialect("mysql")
sqlText, _, err := db.Insert().
Table(users).
Set(users.Email, "alice@example.com").
OnConflict().
DoNothing().
ToSQL()
if err != nil {
t.Fatalf("ToSQL returned error: %v", err)
}

wantSQL := "INSERT IGNORE INTO `users` (`email`) VALUES (?)"
if sqlText != wantSQL {
t.Fatalf("unexpected SQL:\nwant: %s\ngot: %s", wantSQL, sqlText)
}
})

t.Run("sqlite renders insert or ignore", func(t *testing.T) {
db, _ := rain.OpenDialect("sqlite")
sqlText, _, err := db.Insert().
Table(users).
Set(users.Email, "alice@example.com").
OnConflict().
DoNothing().
ToSQL()
if err != nil {
t.Fatalf("ToSQL returned error: %v", err)
}

wantSQL := `INSERT OR IGNORE INTO "users" ("email") VALUES (?)`
if sqlText != wantSQL {
t.Fatalf("unexpected SQL:\nwant: %s\ngot: %s", wantSQL, sqlText)
}
})

t.Run("postgres renders targetless on conflict do nothing", func(t *testing.T) {
db, _ := rain.OpenDialect("postgres")
sqlText, _, err := db.Insert().
Table(users).
Set(users.Email, "alice@example.com").
OnConflict().
DoNothing().
ToSQL()
if err != nil {
t.Fatalf("ToSQL returned error: %v", err)
}

wantSQL := `INSERT INTO "users" ("email") VALUES ($1) ON CONFLICT DO NOTHING`
if sqlText != wantSQL {
t.Fatalf("unexpected SQL:\nwant: %s\ngot: %s", wantSQL, sqlText)
}
})
}

func TestInsertSelectIgnoreToSQL(t *testing.T) {
t.Parallel()

users, _ := defineTables()

t.Run("mysql insert ignore select", func(t *testing.T) {
db, _ := rain.OpenDialect("mysql")
subquery := db.Select().Table(users).Column(users.Email)

sqlText, _, err := db.Insert().
Table(users).
Columns(users.Email).
Select(subquery).
Ignore().
ToSQL()
if err != nil {
t.Fatalf("ToSQL returned error: %v", err)
}

wantSQL := "INSERT IGNORE INTO `users` (`email`) SELECT `users`.`email` FROM `users`"
if sqlText != wantSQL {
t.Fatalf("unexpected SQL:\nwant: %s\ngot: %s", wantSQL, sqlText)
}
})

t.Run("postgres insert ignore select", func(t *testing.T) {
db, _ := rain.OpenDialect("postgres")
subquery := db.Select().Table(users).Column(users.Email)
Comment on lines +390 to +416

@greptile-apps greptile-apps Bot Jul 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 TestInsertSelectIgnoreToSQL is missing a SQLite sub-case

TestInsertSelectIgnoreToSQL covers MySQL and Postgres INSERT...SELECT with Ignore() but has no SQLite sub-test. For SQLite with q.ignore = true and a SELECT source, q.conflict is nil so withSQLiteInsertSelectConflictWhere() is correctly skipped, but this path is unverified. Adding a test that confirms INSERT OR IGNORE INTO "users" ("email") SELECT ... for SQLite would lock in the expected output and prevent regressions.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: pkg/rain/query_insert_test.go
Line: 390-416

Comment:
**`TestInsertSelectIgnoreToSQL` is missing a SQLite sub-case**

`TestInsertSelectIgnoreToSQL` covers MySQL and Postgres INSERT...SELECT with `Ignore()` but has no SQLite sub-test. For SQLite with `q.ignore = true` and a SELECT source, `q.conflict` is nil so `withSQLiteInsertSelectConflictWhere()` is correctly skipped, but this path is unverified. Adding a test that confirms `INSERT OR IGNORE INTO "users" ("email") SELECT ...` for SQLite would lock in the expected output and prevent regressions.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Codex


sqlText, _, err := db.Insert().
Table(users).
Columns(users.Email).
Select(subquery).
Ignore().
ToSQL()
if err != nil {
t.Fatalf("ToSQL returned error: %v", err)
}

wantSQL := `INSERT INTO "users" ("email") SELECT "users"."email" FROM "users" ON CONFLICT DO NOTHING`
if sqlText != wantSQL {
t.Fatalf("unexpected SQL:\nwant: %s\ngot: %s", wantSQL, sqlText)
}
})
}

func TestInsertWithCTEToSQL(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -692,7 +855,7 @@ func TestInsertOnConflictMySQL(t *testing.T) {
t.Fatalf("insert on conflict mysql do nothing ToSQL returned error: %v", err)
}

wantSQL := "INSERT INTO `users` (`email`, `name`) VALUES (?, ?) ON DUPLICATE KEY UPDATE `id` = `id`"
wantSQL := "INSERT IGNORE INTO `users` (`email`, `name`) VALUES (?, ?)"
if sqlText != wantSQL {
t.Fatalf("unexpected mysql do nothing SQL:\nwant: %s\ngot: %s", wantSQL, sqlText)
}
Expand Down