Closes #18980: Optimize update of object data when adding/removing custom fields #18983
Conversation
… adding/removing custom fields
jeremystretch
changed the title
jeremystretch
requested review from
a team and
jnovinger
and removed request for
a team
| value, | ||
| function='jsonb_set' | ||
| ) | ||
| ) |
There was a problem hiding this comment.
Previously, we weren't updating instances that already had a value for the self.name path in the custom field data field (I think) and only setting it on those that did not.
But now we're just flat out setting the default value on all records (either adding or updating, per jsonb_set docs), is that right? I think this is safe to do since this is only running as a result of adding new custom field. Is that the reasoning or is there something else I'm missing?
There was a problem hiding this comment.
IIRC the exclusion filter was left over from an earlier implementation of custom fields and is no longer needed. An argument could be made for retaining it, however it's cleaner IMO to overwrite any data that happens to exist (somehow) when adding a new custom field on the model, because that's what I'd expect to happen as a user.
netbox/extras/models/customfields.py
Outdated
| model.objects.bulk_update(instances, ['custom_field_data'], batch_size=100) | ||
| ct.model_class().objects.update( | ||
| custom_field_data=F('custom_field_data') - self.name | ||
| ) |
There was a problem hiding this comment.
Previously there was a check here (but oddly not in the other modified methods) to make sure the result of ct.model_class() wasn't falsy (I assume None, which I've seen locally once or twice).
Is the feeling that it's safe to assume that we'll never try to call None.objects?
There was a problem hiding this comment.
Good catch. I suspect it might be there to elegantly handle the case where a plugin model has been removed. I'm going to restore the check.
Fixes: #18980
populate_initial_data()now uses the PostgreSQL-nativejsonb_set()function to set initial values on objects when creating a custom fieldremove_stale_data()now uses native JSON mutation to remove the keys for deleted custom fieldsrename_object_data()has likewise been optimized to rename custom field keys in object data usingjsonb_set()A quick test using a REST API call adding a custom field to ~11,000 interfaces took 0.543s with this change versus 5.556s in
main.