mongoengine icon indicating copy to clipboard operation
mongoengine copied to clipboard

Querying dynamic field on DynamicEmbeddedDocument throws LookUpError

Open tjhall13 opened this issue 5 years ago • 1 comments

An example of a failing DynamicEmbeddedDocument:

class DynamicSettings(DynamicEmbeddedDocument):
    known_field = StringField()

class Person(Document):
    name = StringField()
    settings = EmbeddedDocumentField(DynamicSettings)

p = Person(settings=DynamicSettings(known_field="abc", dynamic_field1="123"), name="John").save()

# The following raises a LookUpError
Person.objects(settings__dynamic_field1="123").first()

tjhall13 avatar Jan 23 '20 19:01 tjhall13

I think I'm having the same issue while querying dynamic documents filtering by a field nested in a dynamic embedded document. While debugging this library I've found in base/document.py the _lookup_field method is as follow:

     # extracted from: https://github.com/MongoEngine/mongoengine/blob/master/mongoengine/base/document.py#L1136
     else:
                ReferenceField = _import_class("ReferenceField")
                GenericReferenceField = _import_class("GenericReferenceField")

                # If previous field was a reference, throw an error (we
                # cannot look up fields that are on references).
                if isinstance(field, (ReferenceField, GenericReferenceField)):
                    raise LookUpError(
                        "Cannot perform join in mongoDB: %s" % "__".join(parts)
                    )

                # If the parent field has a "field" attribute which has a
                # lookup_member method, call it to find the field
                # corresponding to this iteration.
                if hasattr(getattr(field, "field", None), "lookup_member"):
                    new_field = field.field.lookup_member(field_name)

                #### NEXT ELIF IS GIVING PRIORITY TO DYNAMIC DOCUMENTS OVER EMBEDDED DOCUMENTS ####
                # If the parent field is a DynamicField or if it's part of
                # a DynamicDocument, mark current field as a DynamicField
                # with db_name equal to the field name.
                elif cls._dynamic and (
                    isinstance(field, DynamicField)
                    or getattr(getattr(field, "document_type", None), "_dynamic", None)
                ):
                    new_field = DynamicField(db_field=field_name)

                # Else, try to use the parent field's lookup_member method
                # to find the subfield.
                elif hasattr(field, "lookup_member"):
                    new_field = field.lookup_member(field_name)

                # Raise a LookUpError if all the other conditions failed.
                else:
                    raise LookUpError(
                        "Cannot resolve subfield or operator {} "
                        "on the field {}".format(field_name, field.name)
                    )

It seems to me the issue is because the code is first checking if the document is dynamic rather than checking if the field refers to an embedded document. Flipping the code as follow seems to solve this specific issue:

     else:
                ReferenceField = _import_class("ReferenceField")
                GenericReferenceField = _import_class("GenericReferenceField")

                # If previous field was a reference, throw an error (we
                # cannot look up fields that are on references).
                if isinstance(field, (ReferenceField, GenericReferenceField)):
                    raise LookUpError(
                        "Cannot perform join in mongoDB: %s" % "__".join(parts)
                    )

                #### THIS CHANGE GIVES PRIORITY TO EMBEDDED DOCUMENTS OVER DYNAMIC DOCUMENTS ####
                # If the parent field has a "field" attribute which has a
                # lookup_member method, call it to find the field
                # corresponding to this iteration.
                if hasattr(getattr(field, "field", None), "lookup_member"):
                    new_field = field.field.lookup_member(field_name)

                # Else, try to use the parent field's lookup_member method
                # to find the subfield.
                elif hasattr(field, "lookup_member"):
                    new_field = field.lookup_member(field_name)

                # If the parent field is a DynamicField or if it's part of
                # a DynamicDocument, mark current field as a DynamicField
                # with db_name equal to the field name.
                elif cls._dynamic and (
                    isinstance(field, DynamicField)
                    or getattr(getattr(field, "document_type", None), "_dynamic", None)
                ):
                    new_field = DynamicField(db_field=field_name)

                # Raise a LookUpError if all the other conditions failed.
                else:
                    raise LookUpError(
                        "Cannot resolve subfield or operator {} "
                        "on the field {}".format(field_name, field.name)
                    )

What's the reason of prioritizing dynamic documents to resolve field names over embedded documents?

m-revetria avatar Apr 21 '22 14:04 m-revetria