ovn-org/libovsdb

bindings: non-nil empty maps/slices are considered as zero values and filtered out from updates

jcaamano opened this issue · 4 comments

The idea is to be able to clear out sets or maps in an update. If you provide a non-nil empty slice/map in an update that does not explicitly specify that field, libovsdb will consider it as a default value and filter it out of the update:

libovsdb/ovsdb/bindings.go

Lines 352 to 372 in 586143b

func isDefaultBaseValue(elem interface{}, etype ExtendedType) bool {
value := reflect.ValueOf(elem)
if !value.IsValid() {
return true
}
switch etype {
case TypeUUID:
return elem.(string) == "00000000-0000-0000-0000-000000000000" || elem.(string) == "" || isNamed(elem.(string))
case TypeMap, TypeSet:
return value.IsNil() || value.Len() == 0
case TypeString:
return elem.(string) == ""
case TypeInteger:
return elem.(int) == 0
case TypeReal:
return elem.(float64) == 0
default:
return false
}
}

libovsdb/mapper/mapper.go

Lines 152 to 154 in 586143b

if len(fields) == 0 && ovsdb.IsDefaultValue(column, nativeElem) {
continue
}

The expected behavior was that libovsdb would consider a nil map/slice (the zero value of a slice/map) as a default but not a non-nil empty slice/map and thus that it could be used to clear out the value from DB.

Currently with nbctl you can do something as <set>=[] to clear a value out.

Alternatives in user code are:

  • Check the current values in the map/slice from cache and clear them out with a Mutate. This seems more complex than what should be needed.
  • Calculate your own list of non default values and explicitly specify those in the Update. This might lead to code that needs to be kept current with future updates of the schema.

Is there any specific reason why non-nil empty slice/map is considered default instead of just only a nil slice/map?
Could this be changed as the expected behavior described?
How would this affect arrays? Would they need to be changed to pointers?

Generic user code is not viable unless libovsdb modelgen includes information about mutability. Or silently filters out inmutable fields.

If 'nil' is the only way for libovsdb to distinguish 'no change' from 'update', then everything will become a pointer. Does not seem ideal either. I am thinking that mutate -- as you proposed above -- is a more natural approach to explicitly clearing the column. Just a thought.

related issue: #259

@jcaamano I think you could specify the fields you want to be updated/overided explicity when update a row:

Update(model.Model, ...interface{}) ([]ovsdb.Operation, error)

ovs.ExternalIDs = map[string]string {}
err := c.impl.Where(&ovs).Update(&ovs, %ovs.ExternalIDs) //  The `external_ids` will be replaced by the empty set.