-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Confusing behaviour when ListFields are set to empty list #267
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
Comments
I would much rather lists not be auto serialized as empties into the database. Though, it is a bit of a consistency issue with how mongo handles lists, especially as they apply to indexes. That said I would still rather have a non-existant entry until content is added. |
This issue seesaws alot - once 0.8 is out I will review for 0.9 |
I would also like to see this changed to be more consistent. There are situations where I am removing entries from a list, but require an empty list to be kept in the database. As of right now, I have to detect if I am removing the last element from the list and if so, use the update method to set the field as an empty list. |
@seglberg I'll remove the 0.10 milestone on this one, when you have time, you're very welcome to submit something on the topic |
A similar case is also the issue #938 |
This is not an issue anymore -
|
Hello! Is this fix included in release v0.11.0? I'm still having this issue on the latest release, also with flask-mongoengine 0.9.2. Thanks! I'm using a ListField of ReferenceFields. |
@andrewsosa001 could you provide some code that reliably reproduces your issue? |
@wojcikstefan Here's a sample script I wrote to demonstrate:
I've also visually inspected my MongoDB instance using Robomongo to confirm my findings. Thanks! |
Ah, thanks @andrewsosa001, that makes it clearer. Proof I have shown in #267 (comment) is invalid then - it only worked because I was changing the value from an implicit
|
Now, one last question we have to answer is why we expect an empty list to be stored in the database in the cases above. An argument can be made for auto-unsetting of the field actually being beneficial in terms of storage. However, in such case we should also handle document creation correctly so that it doesn't set On the other hand, always storing an empty list makes for easier querying where you can always be sure that querying by I'm personally leaning towards the latter argument of always storing the empty lists. What do you think @lafrech @thedrow @andrewsosa001 @gukoff ? |
@wojcikstefan I think it actually makes more sense to have empty lists automatically removed (what is currently happening). You made good points in your argument for that behavior. It would be easy to adjust for in terms of querying, instead of the Ultimately, I think as long as the approach is consistent document creation behavior (i.e., right now new documents with empty list fields initialize to |
This is true. However, there's always a tradeoff between simplicity and performance, and for sure one should keep away of the premature optimization. This heuristic seems like one, because we improve the memory issue without being explicitly asked to. And this improvement implies the simplicity/maintainability/transparency degradation, because you have to maintain the introduced complexity.
Is it? Implicitly having the same field in 2 possible states with different behaviour {non-existent/non-empty array} reminds me only of the nullable references aka the Billion Dollar Mistake. I think that transparency is important. Mongoengine isn't a service, which uses the database as its private backend. It doesn't even hide the raw MongoDB queries to the full. It is a tool, which helps a user work with the database. Not the database helps a user work with mongoengine(which might be a rather convenient scenario). Therefore, all the tricks with the storage should be explicit. A hyperbolized example of a similar heuristic can be a compression of the large strings. It would make storage more efficient, but the string indexes useless. |
This is wonderfully put, thank you for taking the time to write it @gukoff ! I agree with all of your points. Clearly current implementation is already causing confusion, as proven by this issue. We could try to tweak it and fix some of its problems, but it will always have certain edge cases. For example, In the future, we should go with the more explicit (and thus more intuitive) solution. The only "magic" I'm a fan of is to default to an empty list in the MongoEngine object if the document we loaded from the database doesn't contain a specific list field or if that field is set to |
Small note wrt consistency among different field types: When you assign a |
Did this ever get resolved? I feel like there should at least be a way to make sure the field is not unset, preferably when defining the document's fields. |
Same opinion here, was that ever resolved? As it turns out, unsetting the fields seems to not cascade on the CachedReferenceFields as well |
I found a nice little work around this issue. You can use this |
I wonder if there's a way to make it so that it can be done on the Document
page... like "if (len(field) == 0): add_to_Set__list_field." I'm not too
versed in mongoengine now to do it, but would someone know? Maybe it's done
on the pre_save hook?
…On Sat, Mar 2, 2019 at 7:00 AM alaminKutumbita ***@***.***> wrote:
I found a nice little work around this issue. You can use this
test_doc.save() test_doc.update(add_to_set__list_field = [])
it will update the reference with the empty list fiel
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#267 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFQjWYjx0ASpbjGlQdHL6le4uMNhztdEks5vSmfLgaJpZM4AjGfp>
.
--
*Harnek Gulati*
*Occupation:* Founder of MakerFleet
*Education: *Harvard University Computer Science Graduate
*w*: ilikebuildingthings.com and makerfleet.com
*e*: [email protected]
|
The problem @alaminKutumbita is that no post_save hooks is triggered on update, moreover it doesnt update the cached reference fields :/ |
Having a similar issue when setting a DictionaryField to an empty object/dict, the field becomes unset. But the field has required=True so now the document fails validation on loading. I'm able to get around it by using pymongo functions, but am worried that later something will save the document and cause mongoengine to unset it again. |
You'll get different results when you query a collection or call .first() at this query. For the whole collection, those unset fields are missing. I rely on this behavior as my workaround, that is I loop the whole results, and for every single iteration the default values are there. (However, the unset fields have been unset and no data in db actually.) |
Hi, In the model definition add default value like this "users = fields.ListField(default=['text/html'])", |
Hi, In the model definition add default value like this "users = fields.ListField(default=['text/html'])", |
FYI I started working on fixing this so that MongoEngine won't unset list/dict fields as soon as the list/dict is empty |
I got the same issue at v0.23.1. |
Hello, any updates ? |
After this operation, in spite of the fact that list_field was not given any value, the BSON document in the collection has a corresponding field initialized to an empty list:
I understand why initializing to empty list could be a good idea - it allows subsequent list operations on that field to succeed. However, in the light of this, the following behaviour seems wrong to me:
This call causes mongoengine to issue an $unset command, removing list_field from the BSON document:
So ListFields, unlike other field types, get initialized to empty lists when no value is given, but setting a ListField to empty list causes it to be removed from the underlying BSON document.
The text was updated successfully, but these errors were encountered: