add support for JSONField lookups in an embedded model #230
Conversation
| @@ -7,6 +7,26 @@ def is_direct_value(node): | |||
| return not hasattr(node, "as_sql") | |||
|
|
|||
|
|
|||
| def key_transform_build_path(key_trasnforms, lhs): | |||
There was a problem hiding this comment.
possible rename / reorder of args? def build_json_mql_path(lhs, key_transforms)
And I think it's okay to keep this function in json.py since it's field-specific logic that EMF has to borrow to support json.
| transforms = ".".join(key_transforms) | ||
| return f"{mql}.{transforms}" | ||
| result = f"{mql}.{transforms}" | ||
| return key_transform_build_path(json_key_transforms, result) |
There was a problem hiding this comment.
Perhaps make it clear that "key_transform_build_path" is only necessary for json lookups:
lhs = f"{mql}.{transforms}"
if json_key_transforms:
lhs = key_transform_build_path(...)
return lhs
| @@ -139,6 +139,31 @@ def test_order_and_group_by_embedded_field(self): | |||
| ], | |||
| ) | |||
|
|
|||
| def test_embedded_with_json_field(self): | |||
| models = [] | |||
| for i in range(4): | |||
There was a problem hiding this comment.
Did you consider using the style of other tests?
objs = [
Holder.objects.create(
data=Data(json_value={"field1": i * 5, "field2": {"0": {"value": list(range(i))}}})
) for i in range(4)
]
For clarity, use "key" rather than "field" in json data.
There was a problem hiding this comment.
I didn't, it is better option
tests/model_fields_/models.py
Outdated
| @@ -103,6 +103,7 @@ class Data(EmbeddedModel): | |||
| integer = models.IntegerField(db_column="custom_column") | |||
| auto_now = models.DateTimeField(auto_now=True) | |||
| auto_now_add = models.DateTimeField(auto_now_add=True) | |||
| json_value = models.JSONField(default=None) | |||
There was a problem hiding this comment.
As far as I could tell, default=None isn't needed.
There was a problem hiding this comment.
Perfect.
I want to discuss something here—not about the test, but about a MongoDB 5 restriction that prohibits setting a default dict. This restriction does not exist in other versions.
I haven’t checked whether this applies only to embedded fields or also to common model fields.
The error is:
pymongo.errors.WriteError: Invalid $set :: caused by :: an empty object is not a valid value. Found empty object at path data.json_value, full error: {'index': 0, 'code': 40180, 'errmsg': 'Invalid $set :: caused by :: an empty object is not a valid value. Found empty object at path data.json_value'}
It seems that MongoDB 5 does not allow setting empty subdocuments as {}.
What do you think about raising an exception if the default is dict (and the Mongo version is 5)?
There was a problem hiding this comment.
It's probably not worth spending the time to write code for such an old version (released in 2021). We could document the limitation, but it seems quite unlikely that someone is using a very old version AND this field combination.
And now that I look, MongoDB 5.0 was end of life in October 2024, so probably we should bump the minimum version to 6.0 anyway.
WaVEV
force-pushed
the
EMF-support-JSONField
branch
from
0c1cbac to
3976103
Compare
timgraham
changed the title
timgraham
force-pushed
the
EMF-support-JSONField
branch
from
3976103 to
6a87d84
Compare
No description provided.