INTPYTHON-751 Make query generation omit $expr unless required #396

WaVEV · 2025-09-09T17:15:54Z

In this PR a unified approach for generating MQL from Django expressions was implemented. The core idea is to centralize the control flow in a base_expression method, which decides whether the expression can be translated into a direct field: value match (index-friendly) or must fall back to $expr. This keeps the logic for wrapping and dispatching in one place, while each lookup/function only defines its own expression-building logic.

This approach also allows mixing direct field: value matches with $expr clauses within the same $match. As a result, multiple $expr entries may coexist alongside index-optimized conditions, depending on the shape of the query.

Most lookups now follow this pattern by simply implementing as_mql_expr (and optionally as_mql_path when a match-based translation is possible). Only a few special cases like Col, Func operators (except the KeyTransform) , and many more, override the base behavior directly. This structure also leaves room for future optimizations (e.g. constant folding) without having to change the overall flow.

Additionally, since MongoDB 6 does not allow nesting $expr inside another $expr, the flow in base_expression ensures that such cases are flattened. In practice, expressions are generated without redundant wrapping, so the final MQL never contains $expr within $expr.

NOTE: Some polish will be made, but the main idea and the majority of the code is already rendered.

WaVEV · 2025-09-26T03:40:00Z

django_mongodb_backend/functions.py

-    Substr.as_mql = substr
-    Trim.as_mql = trim("trim")
-    TruncBase.as_mql = trunc
+    Cast.as_mql_expr = cast


All the function does not support as_mql_path. It could be added latter if we try to simplify constants expressions

WaVEV · 2025-09-26T04:33:56Z

django_mongodb_backend/expressions/builtins.py

    return value


+def base_expression(self, compiler, connection, as_path=False, **extra):


This is the common handler for all expressions. It defines if an expr is needed or not.

timgraham · 2025-09-26T23:34:24Z

django_mongodb_backend/fields/json.py

+    KeyTransformExact.as_mql_expr = key_transform_exact_expr
+    KeyTransformExact.as_mql_path = key_transform_exact_path


Please check alphabetization of the classes and functions (not only in this file).

timgraham · 2025-09-26T23:47:43Z

django_mongodb_backend/base.py

+    }
+
+    def range_match(a, b):
+        ## TODO: MAKE A TEST TO TEST WHEN BOTH ENDS ARE NONE. WHAT SHALL I RETURN?


AI says, "If either the start or end value provided to the BETWEEN operator is NULL, the entire BETWEEN condition will typically evaluate to UNKNOWN (and thus FALSE in a WHERE clause), unless explicitly handled." (I confirmed this for SQLite and PostgreSQL)

However, to match the semantics implemented here where None is treated as min/max date, I would expect __range=[None, None] not to filter any values.

timgraham · 2025-09-26T23:51:50Z

django_mongodb_backend/aggregates.py

-    connection,
-    operator=None,
-    resolve_inner_expression=False,
-    **extra_context,  # noqa: ARG001


Is the removal of extra_context strictly related to this patch? (Mainly wondering, though perhaps it could be a separate trivial PR).

Yes, It was imported from django. I copied the extra_context thing but I realize that the codes never uses those extra_context. So I went to remove it. But I agree, it could be in a separate PR.

timgraham · 2025-09-27T00:03:27Z

django_mongodb_backend/aggregates.py

+    Aggregate.as_mql_expr = aggregate
+    Count.as_mql_expr = count
+    StdDev.as_mql_expr = stddev_variance
+    Variance.as_mql_expr = stddev_variance


I see we have as_mql_expr(), as_mql_path(), and as_mql(..., as_path=...). If this is the way we keep it, it would be good to explain in the design document which objects (aggregate, func, expression, etc.) get which.

I wonder about renaming as_mql_expr() or as_mql_path() to as_mql() (i.e. treating one of paths as the default). Do you think it would be more or less confusing?

Yes, that was the idea. I’ll explain it in the docs, and we might also consider renaming some methods. The core concept is:

Every expression has an as_mql method.

In some cases, it’s simpler to implement as_mql directly, so those methods don’t follow the common expression flow.

For other expressions, as_mql is a composite function that delegates to as_path or as_expr when applied.

The base_expression.as_mql method controls when these are called and performs boilerplate checks to prevent nesting an expr inside another expr (a MongoDB 6 restriction).

In short: every object has as_mql. Some also define as_path and as_expr. The base_expression coordinates how these methods are used, except for cases where as_mql is defined directly.

Doc here: link

timgraham · 2025-09-27T00:04:37Z

django_mongodb_backend/base.py

+            return {"$or": [{a: {"$exists": False}}, {a: None}]}
+        return {"$and": [{a: {"$exists": True}}, {a: {"$ne": None}}]}
+
+    mongo_operators_expr = {


mongo_expr_operators might be a more natural word order.

timgraham · 2025-09-27T00:21:40Z

django_mongodb_backend/functions.py

        lhs_mql = {"$convert": {"input": lhs_mql, "to": output_type}}
    if decimal_places := getattr(self.output_field, "decimal_places", None):
        lhs_mql = {"$trunc": [lhs_mql, decimal_places]}
+


django_mongodb_backend/functions.py

timgraham · 2025-09-27T00:24:14Z

django_mongodb_backend/query.py

 from django.db.models.sql.where import AND, OR, XOR, ExtraWhere, NothingNode, WhereNode
 from pymongo.errors import BulkWriteError, DuplicateKeyError, PyMongoError

-from .query_conversion.query_optimizer import convert_expr_to_match


You can delete all that code too. :-)

timgraham · 2025-09-27T00:25:03Z

django_mongodb_backend/query_utils.py

+
+def regex_match(field, regex, insensitive=False):
+    options = "i" if insensitive else ""
+    # return {"$regexMatch": {"input": field, "regex": regex, "options": options}}


timgraham · 2025-09-27T00:25:53Z

tests/model_fields_/test_embedded_model.py

+    def test_annotate(self):
+        obj = Book.objects.create(
+            author=Author(name="Shakespeare", age=55, address=Address(city="NYC", state="NY"))
+        )
+        book_from_ny = (
+            Book.objects.annotate(city=F("author__address__city")).filter(city="NYC").first()
+        )
+        self.assertCountEqual(book_from_ny.city, obj.author.address.city)


If it passes in the current code, it could be added separately.

mmh. maybe I have to delete it, I create It to validate something. But the check is contained in others test

Co-authored-by: Tim Graham <[email protected]>

WaVEV · 2025-09-27T01:22:58Z

tests/model_fields_/test_embedded_model_array.py

        qs = Tour.objects.filter(exhibit__sections__number=1)
        self.assertCountEqual(qs, [self.egypt_tour, self.wonders_tour])

+    def test_foreign_field_exact_expr(self):


Do I have to do more test like this? just make a query and the check the generated sql?

Yes, an assertion could be added to existing tests where possible.

WaVEV force-pushed the lookup-refactor branch 3 times, most recently from 529e0ff to a78f26b Compare September 15, 2025 21:21

timgraham changed the title ~~WIP lookup refactor~~ INTPYTHON-751 Make query generation omit $expr unless required Sep 20, 2025

Functional approach solution

59c8faf

WaVEV force-pushed the lookup-refactor branch from 8b0c247 to 141f1cf Compare September 23, 2025 03:52

Jibola mentioned this pull request Sep 23, 2025

INTPYTHON-750: Support converting $expr with $getField paths #392

Closed

Object-oriented approach solution

2c48d11

WaVEV force-pushed the lookup-refactor branch from d11378a to 2c48d11 Compare September 26, 2025 02:26

WaVEV added 2 commits September 26, 2025 00:16

Edits.

7f450a7

Add example generated query test.

e819f29

WaVEV commented Sep 26, 2025

View reviewed changes

Refactor.

ebb78e5

WaVEV commented Sep 26, 2025

View reviewed changes

WaVEV marked this pull request as ready for review September 26, 2025 04:52

WaVEV requested review from Jibola and timgraham and removed request for Jibola September 26, 2025 04:52

timgraham reviewed Sep 27, 2025

View reviewed changes

Update django_mongodb_backend/functions.py

dbbd7db

Co-authored-by: Tim Graham <[email protected]>

WaVEV commented Sep 27, 2025

View reviewed changes

WaVEV added 5 commits September 26, 2025 23:04

Edits.

7372300

Fix.

529f451

Remove query converter

17e9517

Refactor.

e5beb83

Simplify if.

75d53f8

		return value


		def base_expression(self, compiler, connection, as_path=False, **extra):

		KeyTransformExact.as_mql_expr = key_transform_exact_expr
		KeyTransformExact.as_mql_path = key_transform_exact_path

INTPYTHON-751 Make query generation omit $expr unless required #396

Are you sure you want to change the base?

INTPYTHON-751 Make query generation omit $expr unless required #396

Conversation

WaVEV commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WaVEV Sep 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

WaVEV commented Sep 9, 2025 •

edited

Loading

WaVEV Sep 27, 2025 •

edited

Loading