Join style queries for dict, set, list, and embedded fields

[Molanda]

This change adds the ability to query (get, filter, exclude) on DictField, SetField, ListField, and EmbeddedModelField using join style syntax.

For example...

MyModel.objects.filter(myfield__one__two__exact='something').exclude(myfield__three='something')

Which would filter on myfield.one.two and exclude myfield.three in MongoDB.

[aburgel]

None of the code in this file seems to be used anywhere. Am I missing something?

[Molanda]

I copied utils.py from the other tests. It's in aggregations, contrib, embedded, etc... It appears that it sets up a custom version of TestCase with some extra debugging support (settings.DEBUG = True). I didn't go deeply into what it does, but wanted to model my tests after the existing ones.

[aburgel]

So I looked over the code and made some comments above, but as I looked more closely this reminded me of the A queries that are marked as deprecated. Its in django_mongodb_engine/query.py. It seems to do a similar thing. Have you looked at those?

[Molanda]

Thanks for the comments, Alex. I pushed up your recommended changes.

Regarding the A() query, I did look at that originally. It unfortunately has quite a few issues.

The syntax mixes Django and MongoDB styles. The Django part uses "__" and "=" while the MongoDB part uses "." and "," The Django part resolves db_columns from the models while the MongoDB part requires you to know the column names.

You also run into issues when specifying a field more than once. For example, the following will return results with letters "abc" because the second exclude overwrites the first on f_dict...

qs = DotQueryTestModel.objects.exclude(f_dict=A('letters','abc')).exclude(f_dict=A('letters','bc'))

With the changes I’ve proposed, you can write your query in standard Django format, taking into account all of the db_columns and field types, and using the normal Django query methods (exact, contains, in, etc...). For example...

qs = DotQueryTestModel.objects.filter(f_dict__letters__contains='b')
qs = qs.exclude(f_embedded__f_int__in=[10, 12])
qs = qs.filter(Q(f_dict__numbers=1) | Q(f_dict__numbers=4))

[aburgel]

Ok, I see the difference. My concern with your approach, which otherwise seems well-written, is that it adds extra fake fields to the model. I don't have a specific case where this breaks things but it seems like it likely would cause problems, for example if there are name conflicts or if some other code is inspecting the model and finding all these extra fields.

I think you can create a two-part solution which addresses these issues:

1. A better A which fixes the complaints you have, namely using field names and not db_columns, fixing the filter overwriting, etc.
2. Some syntactic sugar in MongoDBQuerySet which allows you to write these queries as more naturally django-ish and which just converts the query into the new style A query.

Does that make sense?

[Molanda]

Let me think a bit about your concerns to make sure they are addressed.

I'm not a fan of the A approach, since it would require a lot of duplication of Django's code to get the database-abstraction API features working correctly.

One thing to note is that the extra fields are only added when a query is made on a child field of a MongoDB Toolbox field types. Since a period is not a valid character for a normal field name, and a period is always present in a child field name, there isn't any chance of field name collision. Also, this code is a no-op if the caller never makes a child field query.

But one area I think could be improved is hiding the extra field a bit better. They need to be part of the query, since Django's code expects to be able to resolve a query key into field information. But I agree with you that it would be nice if the app didn't know about this.

I'll think about that last point a bit more. Thank you again for taking the time to look this over and comment.

On a side note, I was able to fix the embedded model foreign key lookup in 1.6.

[Molanda]

So I did some research and experiments this weekend. What I found was Django itself does a similar technique for internal fields, specifically with the ManyToManyField. I tweaked my code a bit so it matches the method Django uses to add internal fields. Here is a summary...

1. Internal fields (as Django calls them) for MongoDB subfield queries are added using model.add_to_class(), which is the same method that Django uses for fields of this type.
2. Only queries that reference a subfield of a DictField, SetField, ListField, or EmbeddedModelField are processed.
3. The added field names always start with the name of the parent field and always contain one or more periods. So there is no chance of a collision with a normal field.
4. The base model class is not modified. Django adds these fields to the model's options (_meta).

Hopefully this addresses your concerns. If there is a more specific case where you see an issue, please let me know.

[manuthema]

There is a bug in _maybe_add_dot_field function.

If you use "icontains" operator, when the code is checking if exist a operator in the query, the operator "contains" pass in if name.endswith(op) but then, in re.sub function, the string is not replace. Then, the break command finish de for cycle.

You need to delete the break line.

Example:

If you use the query "alumn__name__icontains"
With break line you get
alumn.name.icontains

Without break line
alumn.name__icontains

def _maybe_add_dot_field(self, name):

        if LOOKUP_SEP in name and name.split(LOOKUP_SEP)[0] in self._get_mongo_field_names():
            for op in ALL_OPERATORS:
                if name.endswith(op):
                    name = re.sub(LOOKUP_SEP + op + '$', '#' + op, name)
                    #break 

            name = name.replace(LOOKUP_SEP, '.').replace('#', LOOKUP_SEP)

[Molanda]

Hi Manuel,Thanks for the message.  I fixed the code so it matches endswith(LOOKUP_SEP + op), so it doesn't confuse icontains and contains.Brian

-------- Original Message --------
Subject: Re: [mongodb-engine] Join style queries for dict, set, list,
and embedded fields (#178)
From: Manuel Rodriguez [email protected]
Date: Wed, April 30, 2014 3:09 am
To: django-nonrel/mongodb-engine [email protected]
Cc: Molanda [email protected] is a bug in _maybe_add_dot_field function. If you use "icontains" operator, when the code is checking if exist a operator in the query, the operator "contains" pass in if name.endswith(op) but then, in re.sub function, the string is not replace. Then, the break command finish de for cycle. You need to delete the break line. Example: If you use the query "alumn__name__icontains" With break line you getalumn.name.icontains Without break line alumn.name__icontains def _maybe_add_dot_field(self, name): if LOOKUP_SEP in name and name.split(LOOKUP_SEP)[0] in self._get_mongo_field_names():
for op in ALL_OPERATORS:
if name.endswith(op):
name = re.sub(LOOKUP_SEP + op + '$', '#' + op, name)
#break

    name = name.replace(LOOKUP_SEP, '.').replace('#', LOOKUP_SEP)

—Reply to this email directly or view it on GitHub.

[aburgel]

Can you update the pull request with the fix?

[Molanda]

Hi Alex. The pull request should include the fix now. Let me know if you see any issues.

[Alir3z4]

This is great, I try to avoid using dict, set, list, and embedded fields mostly because of executing/using raw_query against the models.

Why it's not merged into master yet ?