[GODRIVER-2137] empty/nil slices create null field, which can't be used by various functions Created: 22/Aug/21  Updated: 26/Jul/23

Status: Backlog
Project: Go Driver
Component/s: BSON
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Minor - P4
Reporter: Liam Stanley Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: File emptyarraymarshal_test.go    
Issue Links:
Related
related to GODRIVER-2765 Add an "omitnil" BSON struct tag Backlog
related to GODRIVER-2425 use nil slice in `$in` will cause "(B... Closed
related to GODRIVER-2920 Should empty bson.Raw follow the json... Backlog
is related to GODRIVER-971 ability to marshal empty slices as bs... Closed
is related to GODRIVER-1362 Add SliceCodec for mgocompat Closed
Backwards Compatibility: Major Change

 Description   

As similarly expressed in GODRIVER-971 (and added support for via mgocompat in GODRIVER-1362), the default behavior of the default registry used by the Go library is that an empty array (initialized but no elements) or nil array (uninitialized) becomes null when encoded to BSON, and stored in Mongo.

 

The primary issue with this, is that things like $push, $in, $addToSet, etc do not work as expected when the value is set to null.

 

The two current solutions are:

  1. Use mgocompat registry – a feature that's not very obvious, and it further strays away from the "recommended" approach to using the library. This however works as expected, writing an empty array into mongo.
  2. Use "omitempty" on the field, which doesn't write the field at all
    1. Most methods (e.g. $push, $addToSet, etc) work when the field doesn't exist, and those methods will create the field.
    2. It's not obvious that this would be needed to prevent the issue, so users would likely deploy and start writing data before realizing that this may not do what they expect. One would have to first do a database migration to remove all fields that are supposed to be arrays (which were written as null, as this isn't what they were expecting), and then add the necessary tag field for future writes.
    3. There are still some edge-cases with this approach I think in terms of validation (i.e. a field that is required, but can be empty, now can't be required in schema, among others), which would have to be written around in code.

 

Given the field being written as an empty array via bson, can be converted back to nil on the Go side, I don't believe there is a downside in taking the mgocompat approach, and bringing this in as the default. This would also ensure standard methods are prone to work as expected, play better with folks who are using validation, and seems more obvious.

 

At a minimum, I wish this would be re-visited, given the nature of how Go arrays have a nil zero value, which isn't necessarily common with some of the other languages which are supported, and thus I think may warrant an exception to the standard "take exactly what we're given" bson encoding semantics.



 Comments   
Comment by Matt Dale [ 02/Mar/23 ]

Here's what we're currently planning to do for various use cases.

Go structs as MongoDB documents

To support the use case of encoding Go structs as MongoDB documents, we will add an omitnil BSON struct tag that only omits nil values but includes empty values. The $push and $addToSet update operators work the same when modifying document fields that are empty arrays and document fields that do not exist (the only problem is document fields that are BSON null), so the behavior of omitnil maps well to that use case. Additionally, other MongoDB drivers (e.g. Java and Rust) offer a similar behaviors. See GODRIVER-2765 for more details.

Make configuring the MongoDB Client BSON marshaling behavior much easier

To support the use case of changing Client-wide BSON marshaling behavior, we will move all of the current configuration options that are buried in the bsoncodec package onto the bson.Encoder and bson.Decoder types. For example, instead of having to create a SliceCodec and set EncodeNilAsEmpty, you only have to create a bson.Encoder and then call SetNilAsEmpty. See GODRIVER-2716 for more details.

me@liamstanley.io Does that sound like it will resolve some of your original concerns?

Comment by Benji Rewis (Inactive) [ 02/Dec/22 ]

karthickcseapitam@gmail.com I've responded to your community forum post; that's probably the best place to continue this conversation. Thanks!

Comment by karthick d [ 02/Dec/22 ]

benji.rewis@mongodb.com  , How can I use mgocompact with my client options ? currently my code looks like below , I don't see the mongocompact registry file in my vendor directory not sure why , 

tM := reflect.TypeOf(bson.M{})
registry := bson.NewRegistryBuilder().RegisterTypeMapEntry(bsontype.EmbeddedDocument, tM).Build()
clientOpts := options.Client().ApplyURI(URI).SetAuth(info).SetRegistry(registry) client, err := mongo.Connect(ctx, clientOpts)
 

 

how can I add also NewRespectNilValuesRegistryBuilder().Build() in the above code ,? 

Also us emptyarray BSON tag implemented by any chance ? 

 

Comment by Benji Rewis (Inactive) [ 29/Sep/21 ]

All good, me@liamstanley.io! I'll reopen this ticket for now and mark it with fix version 2.0. We can reconsider changing our default behavior at that point in time. For now, it seems like using the mgocompat registry is a viable workaround if you want both empty-initialized and nil arrays to marshal and unmarshal to and from empty BSON arrays.

I'll bring up the idea of a new emptyarray BSON tag with the rest of the team, too.

Comment by Liam Stanley [ 27/Sep/21 ]

I'm sorry for the delay. You are correct, I thought I tested that specific scenario previously, but I guess I didn't test properly.

 

I guess that does change things a bit, though I do think the default for nil arrays being changed should be something that is discussed. I'm not confident in if it should be the standard, but it does seem to make sense to me.

I wonder if a tag value (similar to omitempty), would be better (not as much of a potential breaking change), that when present, tells bson that it:

  • Should output an empty initialized array when encoding
  • Should return an empty initialized array when decoding, if the db has nil rather than an empty array.

Something like `bson:",emptyarray"` or similar, on the field.

 

I still think the default changing makes sense for most users, since nil is harder to work with (e.g. $addToSet and $push fail, so you'd have to convert first, then use those methods). If we were to change the default, we could use tag fields to keep the original behavior if for some reason someone wanted to continue to write nil's.

Comment by Backlog - Core Eng Program Management Team [ 15/Sep/21 ]

There hasn't been any recent activity on this ticket, so we're resolving it. Thanks for reaching out! Please feel free to comment on this if you're able to provide more information.

Comment by Benji Rewis (Inactive) [ 31/Aug/21 ]

Thanks again for your improvement suggestion me@liamstanley.io!

If I’m not mistaken, Go arrays that are empty but initialized will Marshal to an empty BSON array. Go arrays that are nil will become null when encoded to BSON. Please see the emptyarraymarshal_test.go file I’ve attached. Were you referring to a different form of “empty” slice?

In any case, modifying the default behavior of the default registry used by the Go BSON library in order to encode nil Go arrays as empty BSON arrays would be a backwards-breaking API change. Users are likely relying on the existing behavior.

I do see an argument for encoding nil Go arrays to empty BSON arrays, but that change would have to be done in a major release (like the upcoming Go driver 2.0). I also agree that neither the mgocompat registry nor the omitempty struct tag are immediately obvious solutions for this use-case. So, we could certainly Open this ticket for consideration in 2.0.

Comment by Benji Rewis (Inactive) [ 30/Aug/21 ]

Hello me@liamstanley.io and apologies for the delayed response. Thanks for your improvement suggestions; we're investigating now!

Generated at Thu Feb 08 08:37:54 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.