uptrace / bun

SQL-first Golang ORM
https://bun.uptrace.dev
BSD 2-Clause "Simplified" License
3.64k stars 220 forks source link

Issue with BeforeAppendModel when selecting columns to update #976

Closed rohanramaswamy closed 5 months ago

rohanramaswamy commented 5 months ago

Model

type cameraBM struct {
    bun.BaseModel bun:"table:camera"
    ID            uuid.UUID `bun:"type:uuid,default:uuid_generate_v4(),pk"`
    CreatedAt     time.Time `bun:",nullzero,notnull,default:current_timestamp"`
    UpdatedAt     time.Time `bun:",nullzero,notnull,default:current_timestamp"`
    Config        json.RawMessage `bun:"type:jsonb"`
}

Hook

var _ bun.BeforeAppendModelHook = (*CameraBM)(nil)

func (u *CameraBM) BeforeAppendModel(ctx context.Context, query bun.Query) error {
    switch query.(type) {
    case *bun.InsertQuery:
        u.CreatedAt = time.Now()
    case *bun.UpdateQuery:
        u.UpdatedAt = time.Now()
    }
    return nil
}

Update Query which updates updated_at

_, err = db.NewUpdate().
            Model(&cameraBM).
            Column("config", "updated_at").
            WherePK().
            Exec(ctx)

Update Query which I expect to update updated_at

_, err = db.NewUpdate().
            Model(&cameraBM).
            Column("config").
            WherePK().
            Exec(ctx)

I am not sure if this behaviour is expected or not, I spent hours trying to figure out why the hook wasn't working, maybe the hook should add the appropriate column if it doesn't already exist. DB used is Postgres 15

JunNishimura commented 5 months ago

@rohanramaswamy The SQL resulting from the query build in the first example looks like this

UPDATE camera SET config = "...", updated_at = "..." WHERE ID = "...";

The SET clause specifies the updated_at column, so it is updated.

On the other hand, the SQL resulting from the query build in the second example looks like this

UPDATE camera SET config = "..." WHERE ID = "...";

The SET clause does not specify the updated_at column, so it is not updated.

I think the behaviour is as expected because the responsibility of BeforeAppendModel method should only be to update the model field (in this case CreatedAt or UpdatedAt), and the responsibility of the Column method should be to determine which column values are actually updated, and those two responsibilities should be separated.