Skip to content
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.

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

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

Open
jcaamano opened this issue Sep 16, 2021 · 4 comments

Comments

@jcaamano
Copy link
Collaborator

jcaamano commented Sep 16, 2021

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?

@jcaamano
Copy link
Collaborator Author

jcaamano commented Sep 16, 2021

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

@flavio-fernandes
Copy link
Contributor

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.

@amorenoz
Copy link
Collaborator

amorenoz commented Nov 4, 2021

related issue: #259

@ruicao93
Copy link

ruicao93 commented Nov 6, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants