add support for subqueries (Subquery, Exists, and QuerySet as a lookup value) #149
Conversation
timgraham
changed the title
WaVEV
force-pushed
the
add-support-to-subquery-expressions
branch
from
c848f5b to
101fe23
Compare
WaVEV
requested review from
timgraham and
Jibola
and removed request for
timgraham
django_mongodb/expressions.py
Outdated
| from_table = next( | ||
| e.table_name for alias, e in self.alias_map.items() if self.alias_refcount[alias] | ||
| ) | ||
| subquery.lookup_data = { |
There was a problem hiding this comment.
The name lookup_data does not readily indicate its purpose and I don't see any explanatory comments about it.
| @@ -581,7 +613,7 @@ def _get_ordering(self): | |||
| return tuple(fields), sort_ordering, tuple(extra_fields) | |||
|
|
|||
| def get_where(self): | |||
| return self.where | |||
| return getattr(self, "where", self.query.where) | |||
There was a problem hiding this comment.
So we are back to getattr() for this... maybe we should drop the get_where() hook we previously added to try to avoid it. Those could be done in a follow up though. For now, perhaps a comment is warranted.
WaVEV
force-pushed
the
add-support-to-subquery-expressions
branch
from
bf5230b to
d97a3d4
Compare
django_mongodb/compiler.py
Outdated
| for expr in having_group_by: | ||
| group_expressions.add(expr) | ||
| if isinstance(self.query.group_by, tuple | list): | ||
| group_expressions |= set(self.query.group_by) | ||
| elif self.query.group_by is None: | ||
| group_expressions = set() | ||
| expressions.add(expr) | ||
| return self.collapse_group_by(expressions, having_group_by) |
There was a problem hiding this comment.
What is the purpose of collapse_group_by if we already have having_group_by getting added to the expressions set?
There was a problem hiding this comment.
The logic is taken from here and the idea is to avoid repeating expressions and merge both aggregators.
There was a problem hiding this comment.
Re-reading I figured out that we haven't defined allows_group_by_selected_pks so there is not need to call this function (actually this function does nothing)
| @@ -269,7 +296,7 @@ def where_node(self, compiler, connection): | |||
| raise FullResultSet | |||
|
|
|||
| if self.negated and mql: | |||
| mql = {"$eq": [mql, {"$literal": False}]} | |||
| mql = {"$not": mql} | |||
There was a problem hiding this comment.
should this be treated as a separate change?
There was a problem hiding this comment.
Yes, maybe. But It was hard to debug when I got those {eq: [big_expression, False]}
WaVEV
force-pushed
the
add-support-to-subquery-expressions
branch
from
96115d6 to
4654f21
Compare
Subquery, Exists, and QuerySet as a lookup value.
timgraham
force-pushed
the
add-support-to-subquery-expressions
branch
from
7e6f435 to
e971120
Compare
Add support for subqueries