Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
60 changes: 56 additions & 4 deletions pkg/rain/query_insert.go
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 Down Expand Up @@ -184,6 +185,15 @@ func (q *InsertQuery) DefaultValues() *InsertQuery {
return q
}

// Ignore configures the INSERT to ignore conflicting rows.
// For MySQL, this renders "INSERT IGNORE".
// For SQLite, this renders "INSERT OR IGNORE".
// For PostgreSQL, this renders "ON CONFLICT DO NOTHING".
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}
Expand Down Expand Up @@ -275,6 +285,31 @@ func (q *InsertQuery) compile() (compiledQuery, error) {
return ctx.compiledQuery(), ctx.err
}

func (q *InsertQuery) useIgnore() bool {
if q.ignore {
return true
}
Comment on lines +289 to +291

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 Overrides Explicit Conflict

When a caller chains Ignore() with an explicit conflict rule, useIgnore() returns true from q.ignore before looking at the configured conflict action. On MySQL and SQLite this makes writeConflictClause skip the conflict clause, so .Ignore().OnConflict(users.Email).DoUpdateSet(users.Name) emits only INSERT IGNORE or INSERT OR IGNORE and silently drops the requested update.

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

Comment:
**Ignore Overrides Explicit Conflict**

When a caller chains `Ignore()` with an explicit conflict rule, `useIgnore()` returns true from `q.ignore` before looking at the configured conflict action. On MySQL and SQLite this makes `writeConflictClause` skip the conflict clause, so `.Ignore().OnConflict(users.Email).DoUpdateSet(users.Name)` emits only `INSERT IGNORE` or `INSERT OR IGNORE` and silently drops the requested update.

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

Fix in Codex

if q.conflict != nil && q.conflict.action == insertConflictActionDoNothing &&
len(q.conflict.columns) == 0 && q.conflict.constraint == "" {
return true
}
return false
}

func (q *InsertQuery) writeInsertInto(ctx *compileContext) {
if q.useIgnore() {
switch ctx.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) writeValuesSQL(ctx *compileContext) error {
if err := writeCTEs(ctx, q.ctes, "insert"); err != nil {
return err
Expand All @@ -287,7 +322,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 +334,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 Down Expand Up @@ -648,7 +683,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,9 +800,15 @@ func (q *InsertQuery) validateSources() error {
}

func (q *InsertQuery) writeConflictClause(ctx *compileContext) error {
useIgnore := q.useIgnore()

if q.conflict == nil {
if useIgnore && ctx.dialect.Name() == "postgres" {
ctx.writeString(" ON CONFLICT DO NOTHING")
}
return nil
}

if q.conflict.action == insertConflictActionNone {
return errors.New("rain: conflict action is required; call DoNothing() or DoUpdateSet(...)")
}
Expand All @@ -777,6 +818,9 @@ func (q *InsertQuery) writeConflictClause(ctx *compileContext) error {
}

if q.dialect.Name() == "mysql" {
if useIgnore {
return nil
}
if len(q.conflict.columns) > 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")
}
Expand Down Expand Up @@ -816,11 +860,19 @@ func (q *InsertQuery) writeConflictClause(ctx *compileContext) error {
return nil
}

if len(q.conflict.columns) == 0 && q.conflict.constraint == "" {
if q.dialect.Name() == "sqlite" && useIgnore {
return nil
}

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

ctx.writeString(" ON CONFLICT")
if q.useIgnore() && len(q.conflict.columns) == 0 && q.conflict.constraint == "" {
ctx.writeString(" DO NOTHING")
return nil
}
Comment on lines +872 to +875

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 Targetless Update Becomes DoNothing

With PostgreSQL, .Ignore().OnConflict().DoUpdateSet(...) reaches this block with useIgnore() true from q.ignore and no conflict target. The code emits ON CONFLICT DO NOTHING, so the requested update assignments are discarded instead of returning the existing target-required error.

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

Comment:
**Targetless Update Becomes DoNothing**

With PostgreSQL, `.Ignore().OnConflict().DoUpdateSet(...)` reaches this block with `useIgnore()` true from `q.ignore` and no conflict target. The code emits `ON CONFLICT DO NOTHING`, so the requested update assignments are discarded instead of returning the existing target-required error.

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

Fix in Codex

if q.conflict.constraint != "" {
ctx.writeString(" ON CONSTRAINT ")
ctx.writeQuotedIdentifier(q.conflict.constraint)
Expand Down
113 changes: 113 additions & 0 deletions pkg/rain/query_insert_ignore_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
package rain_test

import (
"testing"

"github.com/hyperlocalise/rain-orm/pkg/rain"
"github.com/hyperlocalise/rain-orm/pkg/schema"
)

func TestInsertIgnore(t *testing.T) {
type UsersTable struct {
schema.TableModel
ID *schema.Column[int64]
Name *schema.Column[string]
}
users := schema.Define("users", func(t *UsersTable) {
t.ID = t.BigInt("id").PrimaryKey()
t.Name = t.Text("name").NotNull()
})

tests := []struct {
name string
dialect string
builder func(db *rain.DB) *rain.InsertQuery
wantSQL string
}{
{
name: "PostgreSQL Ignore",
dialect: "postgres",
builder: func(db *rain.DB) *rain.InsertQuery {
return db.Insert().Table(users).Set(users.ID, 1).Set(users.Name, "Alice").Ignore()
},
wantSQL: `INSERT INTO "users" ("id", "name") VALUES ($1, $2) ON CONFLICT DO NOTHING`,
},
{
name: "MySQL Ignore",
dialect: "mysql",
builder: func(db *rain.DB) *rain.InsertQuery {
return db.Insert().Table(users).Set(users.ID, 1).Set(users.Name, "Alice").Ignore()
},
wantSQL: "INSERT IGNORE INTO `users` (`id`, `name`) VALUES (?, ?)",
},
{
name: "SQLite Ignore",
dialect: "sqlite",
builder: func(db *rain.DB) *rain.InsertQuery {
return db.Insert().Table(users).Set(users.ID, 1).Set(users.Name, "Alice").Ignore()
},
wantSQL: `INSERT OR IGNORE INTO "users" ("id", "name") VALUES (?, ?)`,
},
{
name: "PostgreSQL Targetless DoNothing",
dialect: "postgres",
builder: func(db *rain.DB) *rain.InsertQuery {
return db.Insert().Table(users).Set(users.ID, 1).Set(users.Name, "Alice").OnConflict().DoNothing()
},
wantSQL: `INSERT INTO "users" ("id", "name") VALUES ($1, $2) ON CONFLICT DO NOTHING`,
},
{
name: "MySQL Targetless DoNothing",
dialect: "mysql",
builder: func(db *rain.DB) *rain.InsertQuery {
return db.Insert().Table(users).Set(users.ID, 1).Set(users.Name, "Alice").OnConflict().DoNothing()
},
wantSQL: "INSERT IGNORE INTO `users` (`id`, `name`) VALUES (?, ?)",
},
{
name: "SQLite Targetless DoNothing",
dialect: "sqlite",
builder: func(db *rain.DB) *rain.InsertQuery {
return db.Insert().Table(users).Set(users.ID, 1).Set(users.Name, "Alice").OnConflict().DoNothing()
},
wantSQL: `INSERT OR IGNORE INTO "users" ("id", "name") VALUES (?, ?)`,
},
{
name: "PostgreSQL Select Ignore",
dialect: "postgres",
builder: func(db *rain.DB) *rain.InsertQuery {
return db.Insert().Table(users).Columns(users.ID, users.Name).Select(db.Select(users.ID, users.Name).Table(users)).Ignore()
},
wantSQL: `INSERT INTO "users" ("id", "name") SELECT "users"."id", "users"."name" FROM "users" ON CONFLICT DO NOTHING`,
},
{
name: "MySQL Select Ignore",
dialect: "mysql",
builder: func(db *rain.DB) *rain.InsertQuery {
return db.Insert().Table(users).Columns(users.ID, users.Name).Select(db.Select(users.ID, users.Name).Table(users)).Ignore()
},
wantSQL: "INSERT IGNORE INTO `users` (`id`, `name`) SELECT `users`.`id`, `users`.`name` FROM `users`",
},
{
name: "SQLite Select Ignore",
dialect: "sqlite",
builder: func(db *rain.DB) *rain.InsertQuery {
return db.Insert().Table(users).Columns(users.ID, users.Name).Select(db.Select(users.ID, users.Name).Table(users)).Ignore()
},
wantSQL: `INSERT OR IGNORE INTO "users" ("id", "name") SELECT "users"."id", "users"."name" FROM "users"`,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
db, _ := rain.OpenDialect(tt.dialect)
gotSQL, _, err := tt.builder(db).ToSQL()
if err != nil {
t.Fatalf("ToSQL() error: %v", err)
}
if gotSQL != tt.wantSQL {
t.Errorf("ToSQL() got = %q, want %q", gotSQL, tt.wantSQL)
}
})
}
}
4 changes: 2 additions & 2 deletions pkg/rain/query_insert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,7 @@ func TestInsertOnConflictMySQL(t *testing.T) {
}
users, _ := defineTables()

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