django-nonrel / djangotoolbox

Django tools for building nonrel backends
BSD 3-Clause "New" or "Revised" License
200 stars 125 forks source link

Fix issue when reading NULL from non-NULLable field #44

Closed nealedj closed 8 months ago

nealedj commented 10 years ago

Fall back to default if we read a NULL from a non-NULLable field (this is more consistent with Django+mysql behaviour if you do the same)

aburgel commented 10 years ago

Seems reasonable. I wonder if we could make this a little more flexible so instead of returning default values for all missing fields, just return the ones that were requested and let the higher layers deal. I think this would allow for projection queries to work.

ghost commented 10 years ago

I'm not sure I understand, isn't "fields" only the ones requested, or is it all fields for the model?

aburgel commented 10 years ago

You're right, I misread it.

I'm not sure what the correct behavior should be. On SQL backends, it looks like they just pass whatever was received from the DB to the model, so if there's a null in the DB, then the model gets a None.

The equivalent of that for a non-rel backend would be that if the field is not returned from the backend, then the model should also get a None. Does that make sense?

The projection query thing needs to be handled by the appengine compiler. But that's a separate issue.

ghost commented 10 years ago

The problem with that is ListFields... because saving an empty list to the datastore saves nothing, you could save an empty list, and then get back a None.

This is the reason we chose to return the default, rather than None.