Skip to content

Improve InsertQuery with IGNORE and flexible ON CONFLICT #148

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 2 commits into from
Closed

Improve InsertQuery with IGNORE and flexible ON CONFLICT #148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
26024f3
feat(rain): improve InsertQuery with IGNORE and flexible ON CONFLICT
google-labs-jules[bot] Jun 29, 2026
3c9f790
feat(rain): improve InsertQuery with IGNORE and flexible ON CONFLICT
google-labs-jules[bot] Jun 29, 2026
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
61 changes: 47 additions & 14 deletions pkg/rain/query_insert.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ type InsertQuery struct {
conflict *insertConflictClause
ctes []cteDefinition
defaultValues bool
ignore bool

// OPTIMIZATION: Internal buffers to avoid heap allocations for common
// query shapes while keeping the struct size reasonable.
Expand All @@ -42,7 +43,7 @@ const (
)

type insertConflictClause struct {
columns []schema.ColumnReference
targets []schema.Expression
constraint string
targetWhere []schema.Predicate
action insertConflictAction
Expand Down Expand Up @@ -184,9 +185,15 @@ func (q *InsertQuery) DefaultValues() *InsertQuery {
return q
}

// Ignore configures the INSERT to use IGNORE for MySQL.
func (q *InsertQuery) Ignore() *InsertQuery {
q.ignore = true
return q
}

// OnConflict starts an upsert clause for PostgreSQL and SQLite dialects.
func (q *InsertQuery) OnConflict(columns ...schema.ColumnReference) *InsertConflictBuilder {
q.conflict = &insertConflictClause{columns: columns}
func (q *InsertQuery) OnConflict(targets ...schema.Expression) *InsertConflictBuilder {
q.conflict = &insertConflictClause{targets: targets}
return &InsertConflictBuilder{query: q}
}

Expand Down Expand Up @@ -261,6 +268,10 @@ func (q *InsertQuery) compile() (compiledQuery, error) {
ctx := newCompileContext(q.dialect)
defer releaseCompileContext(ctx)

if q.dialect.Name() == "mysql" && q.conflict != nil && q.conflict.action == insertConflictActionDoNothing {
q.ignore = true

@greptile-apps greptile-apps Bot Jun 29, 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 Compilation Persists Ignore State

ToSQL, Exec, and Prepare now mutate the builder by setting q.ignore when MySQL DoNothing is compiled. If the same InsertQuery is compiled once for DoNothing and then reused with a different conflict action, the stale flag still emits INSERT IGNORE, so a later DoUpdateSet query can compile as INSERT IGNORE ... ON DUPLICATE KEY UPDATE ... without the caller asking for ignore semantics.

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: 272

Comment:
**Compilation Persists Ignore State**

`ToSQL`, `Exec`, and `Prepare` now mutate the builder by setting `q.ignore` when MySQL `DoNothing` is compiled. If the same `InsertQuery` is compiled once for `DoNothing` and then reused with a different conflict action, the stale flag still emits `INSERT IGNORE`, so a later `DoUpdateSet` query can compile as `INSERT IGNORE ... ON DUPLICATE KEY UPDATE ...` without the caller asking for ignore semantics.

**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.

Fix in Codex

}

if q.selectQuery != nil {
if err := q.writeSelectSQL(ctx); err != nil {
return compiledQuery{}, err
Expand All @@ -287,7 +298,11 @@ func (q *InsertQuery) writeValuesSQL(ctx *compileContext) error {
if err := q.validateSources(); err != nil {
return err
}
ctx.writeString("INSERT INTO ")
ctx.writeString("INSERT ")
if q.ignore && ctx.dialect.Name() == "mysql" {
ctx.writeString("IGNORE ")
}
ctx.writeString("INTO ")
ctx.writeTableName(q.table)
if ctx.dialect.Name() == "mysql" {
ctx.writeString(" () VALUES ()")
Expand All @@ -299,7 +314,11 @@ func (q *InsertQuery) writeValuesSQL(ctx *compileContext) error {
return err
}

ctx.writeString("INSERT INTO ")
ctx.writeString("INSERT ")
if q.ignore && ctx.dialect.Name() == "mysql" {
ctx.writeString("IGNORE ")
}
ctx.writeString("INTO ")
ctx.writeTableName(q.table)

if q.models != nil {
Expand Down Expand Up @@ -574,7 +593,11 @@ func (q *InsertQuery) writeSelectSQL(ctx *compileContext) error {
selectQuery = selectQuery.withSQLiteInsertSelectConflictWhere()
}

ctx.writeString("INSERT INTO ")
ctx.writeString("INSERT ")
if q.ignore && ctx.dialect.Name() == "mysql" {
ctx.writeString("IGNORE ")
}
ctx.writeString("INTO ")
ctx.writeTableName(q.table)

if len(q.columns) > 0 {
Expand Down Expand Up @@ -703,7 +726,11 @@ func (q *InsertQuery) writeConflictClause(ctx *compileContext) error {
}

if q.dialect.Name() == "mysql" {
if len(q.conflict.columns) > 0 || q.conflict.constraint != "" || len(q.conflict.targetWhere) > 0 {
if q.conflict.action == insertConflictActionDoNothing {
return nil
Comment on lines +729 to +730

@greptile-apps greptile-apps Bot Jun 29, 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 Scoped Conflicts Become Global Ignore

When a MySQL caller uses OnConflict(users.Email).Where(...).DoNothing() or OnConstraint(...).DoNothing(), this branch returns before rejecting the unsupported target fields and the compiler emits plain INSERT IGNORE. That drops the requested conflict scope and can silently ignore unrelated duplicate-key, NOT NULL, foreign-key, or data conversion errors instead of returning them.

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: 729-730

Comment:
**Scoped Conflicts Become Global Ignore**

When a MySQL caller uses `OnConflict(users.Email).Where(...).DoNothing()` or `OnConstraint(...).DoNothing()`, this branch returns before rejecting the unsupported target fields and the compiler emits plain `INSERT IGNORE`. That drops the requested conflict scope and can silently ignore unrelated duplicate-key, NOT NULL, foreign-key, or data conversion errors instead of returning them.

**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.

Fix in Codex

}

if len(q.conflict.targets) > 0 || q.conflict.constraint != "" || len(q.conflict.targetWhere) > 0 {
return errors.New("rain: MySQL ON DUPLICATE KEY UPDATE does not support conflict targets (columns, constraints, or WHERE); call OnConflict() without modifiers")
}
if len(q.conflict.updateWhere) > 0 {
Expand Down Expand Up @@ -742,24 +769,30 @@ func (q *InsertQuery) writeConflictClause(ctx *compileContext) error {
return nil
}

if len(q.conflict.columns) == 0 && q.conflict.constraint == "" {
if len(q.conflict.targets) == 0 && q.conflict.constraint == "" && q.conflict.action != insertConflictActionDoNothing {
return errors.New("rain: conflict clause requires at least one target (columns or constraint)")
}

ctx.writeString(" ON CONFLICT")
if q.conflict.constraint != "" {
ctx.writeString(" ON CONSTRAINT ")
ctx.writeQuotedIdentifier(q.conflict.constraint)
} else if len(q.conflict.columns) > 0 {
} else if len(q.conflict.targets) > 0 {
ctx.writeString(" (")
for idx, col := range q.conflict.columns {
if err := validateColumnBelongsToTable(q.table, col.ColumnDef()); err != nil {
return err
}
for idx, target := range q.conflict.targets {
if idx > 0 {
ctx.writeString(", ")
}
ctx.writeColumnName(col)
if col, ok := target.(schema.ColumnReference); ok {
if err := validateColumnBelongsToTable(q.table, col.ColumnDef()); err != nil {
return err
}
ctx.writeColumnName(col)
} else {
if err := ctx.writeExpression(target); err != nil {
Comment on lines +790 to +792

@greptile-apps greptile-apps Bot Jun 29, 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 Conflict Targets Accept Parameters

Non-column conflict targets are rendered with the general expression writer, which can add bind parameters for expressions such as schema.Raw("lower(email COLLATE ?)", locale), placeholders, values, comparisons, or subqueries. PostgreSQL and SQLite conflict targets must name columns or index expressions, so this can return SQL like ON CONFLICT (lower(email COLLATE $2)) with an extra argument that the database rejects instead of failing during query construction.

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: 790-792

Comment:
**Conflict Targets Accept Parameters**

Non-column conflict targets are rendered with the general expression writer, which can add bind parameters for expressions such as `schema.Raw("lower(email COLLATE ?)", locale)`, placeholders, values, comparisons, or subqueries. PostgreSQL and SQLite conflict targets must name columns or index expressions, so this can return SQL like `ON CONFLICT (lower(email COLLATE $2))` with an extra argument that the database rejects instead of failing during query construction.

**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.

Fix in Codex

return err
}
}
}
ctx.writeByte(')')
}
Expand Down
107 changes: 92 additions & 15 deletions pkg/rain/query_insert_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,43 @@ func TestInsertOnConflictPostgres(t *testing.T) {
}
})

t.Run("do nothing without targets", func(t *testing.T) {
sqlText, args, 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)
}
if len(args) != 1 {
t.Fatalf("unexpected args: %#v", args)
}
})

t.Run("functional index target", func(t *testing.T) {
sqlText, _, err := db.Insert().
Table(users).
Set(users.Email, "alice@example.com").
OnConflict(schema.Raw("(lower(email))")).
DoNothing().
ToSQL()
if err != nil {
t.Fatalf("ToSQL returned error: %v", err)
}

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

t.Run("do update set", func(t *testing.T) {
sqlText, args, err := db.Insert().
Table(users).
Expand Down Expand Up @@ -680,7 +717,7 @@ func TestInsertOnConflictMySQL(t *testing.T) {
}
users, _ := defineTables()

t.Run("do nothing (no-op update)", func(t *testing.T) {
t.Run("do nothing (mapped to ignore)", func(t *testing.T) {
sqlText, args, err := db.Insert().
Table(users).
Set(users.Email, "alice@example.com").
Expand All @@ -692,7 +729,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 All @@ -701,19 +738,6 @@ func TestInsertOnConflictMySQL(t *testing.T) {
}
})

t.Run("target columns are rejected for do nothing", func(t *testing.T) {
_, _, err := db.Insert().
Table(users).
Set(users.Email, "alice@example.com").
Set(users.Name, "Alice").
OnConflict(users.Email).
DoNothing().
ToSQL()
if err == nil || !strings.Contains(err.Error(), "does not support conflict targets") {
t.Fatalf("expected mysql conflict target error, got %v", err)
}
})

t.Run("target columns are rejected for do update set", func(t *testing.T) {
_, _, err := db.Insert().
Table(users).
Expand Down Expand Up @@ -748,4 +772,57 @@ func TestInsertOnConflictMySQL(t *testing.T) {
t.Fatalf("unexpected mysql do update args: %#v", args)
}
})

t.Run("ignore", func(t *testing.T) {
sqlText, args, err := db.Insert().
Ignore().
Table(users).
Set(users.Email, "alice@example.com").
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)
}
if len(args) != 1 {
t.Fatalf("unexpected args: %#v", args)
}
})

t.Run("ignore with select", func(t *testing.T) {
subquery := db.Select().Table(users).Column(users.Email)
sqlText, _, err := db.Insert().
Ignore().
Table(users).
Columns(users.Email).
Select(subquery).
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("ignore with default values", func(t *testing.T) {
sqlText, _, err := db.Insert().
Ignore().
Table(users).
DefaultValues().
ToSQL()
if err != nil {
t.Fatalf("ToSQL returned error: %v", err)
}

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