upper / db

Data Access Layer (DAL) for PostgreSQL, CockroachDB, MySQL, SQLite and MongoDB with ORM-like features.
https://upper.io/
MIT License
3.53k stars 234 forks source link

omitempty on a UUID value #624

Open sbernierlacapitale opened 3 years ago

sbernierlacapitale commented 3 years ago

I use UUID primary keys in my database and throughout the application. I am hoping for these UUIDs to work with upper.db as if they were normal integer values but I can't make the omitempty to work.

I would assume that UUID primary keys are a frequent behavior to desire, so it's possible that I am doing something wrong. I didn't find any documentation related to this, though.

Simplified example:

type Resource struct {
    ID uuid.UUID `db:"id,omitempty"`
    Value string `db:"value"`
}

resource := Resource{
    Value: "test",
}

sess.Collection("resources").Insert(&resource)

This will work the first time, but will create a new resource with a "zero" UUID - 00000000-0000-0000-0000-000000000000. It will then fail the second time when breaking the primary key's unique constraint.

The related code in /internal/sqlbuilder/builder.go is:

isZero := false
if t, ok := fld.Interface().(hasIsZero); ok {
    if t.IsZero() {
        isZero = true
    }
} else if fld.Kind() == reflect.Array || fld.Kind() == reflect.Slice {
    if fld.Len() == 0 {
        isZero = true
    }
} else if reflect.DeepEqual(fi.Zero.Interface(), value) {
    isZero = true
}

UUIDs, as defined in https://github.com/google/uuid and https://github.com/satori/go.uuid / https://github.com/gofrs/uuid, are an array [16]byte. Arrays don't satisfy the hasIsZero interface, but satisfy the second if, which then fail the fld.Len() == 0 check because the UUID has a length of 16 bytes.

reflect.Value's definition of an array's zero-value is the following:

for i := 0; i < v.Len(); i++ {
    if !v.Index(i).IsZero() {
        return false
    }
}
return true

while a slice's zero-value is return v.IsNil(), so I guess there's a fundamental difference between the two.

Alternatively, using isZero := fld.IsZero() (instead of isZero := false) would actually work.

I did not investigate the repercussions on other types of values. Maybe there's a huge problem with this change, in which case I'd like to know how UUID primary keys are expected to work.

I can help with a PR if needed.

Thank you very much!

mnabialek commented 9 months ago

I assume you already solved this long time ago, but I had same issue today and it seems instead of:

ID uuid.UUID `db:"id,omitempty"`

you should define it as string like so:

ID string `db:"id,omitempty"`

to make it work