From 892f42999bf95fd22d48d689b5901797e315d478 Mon Sep 17 00:00:00 2001 From: Emanuel Lupi Date: Mon, 19 Aug 2024 13:51:45 -0400 Subject: [PATCH 1/2] add aggregation_regress test skips --- .github/workflows/test-python.yml | 1 + django_mongodb/features.py | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/.github/workflows/test-python.yml b/.github/workflows/test-python.yml index ec2817965..1d38102f2 100644 --- a/.github/workflows/test-python.yml +++ b/.github/workflows/test-python.yml @@ -68,6 +68,7 @@ jobs: run: > python3 django_repo/tests/runtests.py --settings mongodb_settings -v 2 aggregation + aggregation_regress annotations auth_tests.test_models.UserManagerTestCase backends.base.test_base.DatabaseWrapperTests diff --git a/django_mongodb/features.py b/django_mongodb/features.py index 41b64a5d9..927a88124 100644 --- a/django_mongodb/features.py +++ b/django_mongodb/features.py @@ -59,6 +59,18 @@ class DatabaseFeatures(BaseDatabaseFeatures): "aggregation.tests.AggregateTestCase.test_aggregation_default_passed_another_aggregate", "aggregation.tests.AggregateTestCase.test_annotation_expressions", "aggregation.tests.AggregateTestCase.test_reverse_fkey_annotate", + "aggregation_regress.tests.AggregationTests.test_annotation_disjunction", + "aggregation_regress.tests.AggregationTests.test_decimal_aggregate_annotation_filter", + # Invalid $project :: caused by :: Path collision at aggregation_regress_publisher.name + "aggregation_regress.tests.AggregationTests.test_values_list_annotation_args_ordering", + # QuerySet.extra(select=...) should raise NotSupportedError instead of: + # 'RawSQL' object has no attribute 'as_mql'. + "aggregation_regress.tests.AggregationTests.test_annotate_with_extra", + "aggregation_regress.tests.AggregationTests.test_annotation", + "aggregation_regress.tests.AggregationTests.test_more_more3", + "aggregation_regress.tests.AggregationTests.test_more_more_more3", + # Incorrect JOIN with GenericRelation gives incorrect results. + "aggregation_regress.tests.AggregationTests.test_aggregation_with_generic_reverse_relation", } # $bitAnd, #bitOr, and $bitXor are new in MongoDB 6.3. _django_test_expected_failures_bitwise = { @@ -208,6 +220,7 @@ def django_test_expected_failures(self): "aggregation.tests.AggregateTestCase.test_exists_extra_where_with_aggregate", "annotations.tests.NonAggregateAnnotationTestCase.test_annotation_exists_aggregate_values_chaining", "annotations.tests.NonAggregateAnnotationTestCase.test_annotation_exists_none_query", + "aggregation_regress.tests.AggregationTests.test_annotate_and_join", "delete_regress.tests.DeleteTests.test_self_reference_with_through_m2m_at_second_level", "expressions.tests.BasicExpressionsTests.test_annotation_with_deeply_nested_outerref", "expressions.tests.BasicExpressionsTests.test_boolean_expression_combined", @@ -297,6 +310,13 @@ def django_test_expected_failures(self): "annotations.tests.NonAggregateAnnotationTestCase.test_annotation_and_alias_filter_in_subquery", "annotations.tests.NonAggregateAnnotationTestCase.test_annotation_and_alias_filter_related_in_subquery", "annotations.tests.NonAggregateAnnotationTestCase.test_empty_expression_annotation", + "aggregation_regress.tests.AggregationTests.test_aggregates_in_where_clause", + "aggregation_regress.tests.AggregationTests.test_aggregates_in_where_clause_pre_eval", + "aggregation_regress.tests.AggregationTests.test_f_expression_annotation", + "aggregation_regress.tests.AggregationTests.test_having_subquery_select", + "aggregation_regress.tests.AggregationTests.test_more_more4", + "aggregation_regress.tests.AggregationTests.test_more_more_more5", + "aggregation_regress.tests.AggregationTests.test_negated_aggregation", "db_functions.comparison.test_coalesce.CoalesceTests.test_empty_queryset", "expressions_case.tests.CaseExpressionTests.test_annotate_with_in_clause", "expressions.tests.FTimeDeltaTests.test_date_subquery_subtraction", @@ -351,6 +371,7 @@ def django_test_expected_failures(self): "QuerySet.dates() is not supported on MongoDB.": { "aggregation.tests.AggregateTestCase.test_dates_with_aggregation", "annotations.tests.AliasTests.test_dates_alias", + "aggregation_regress.tests.AggregationTests.test_more_more_more2", "dates.tests.DatesTests.test_dates_trunc_datetime_fields", "dates.tests.DatesTests.test_related_model_traverse", }, @@ -368,6 +389,9 @@ def django_test_expected_failures(self): }, "QuerySet.distinct() is not supported.": { "aggregation.tests.AggregateTestCase.test_sum_distinct_aggregate", + "aggregation_regress.tests.AggregationTests.test_annotate_distinct_aggregate", + "aggregation_regress.tests.AggregationTests.test_conditional_aggregate_on_complex_condition", + "aggregation_regress.tests.AggregationTests.test_distinct_conditional_aggregate", "lookup.tests.LookupTests.test_lookup_collision_distinct", "ordering.tests.OrderingTests.test_orders_nulls_first_on_filtered_subquery", "queries.tests.ExcludeTest17600.test_exclude_plain_distinct", From 9dd5f08a830082d8b392f09ee6bf35e6e54c29d4 Mon Sep 17 00:00:00 2001 From: Emanuel Lupi Date: Sun, 18 Aug 2024 15:28:12 -0300 Subject: [PATCH 2/2] fix collisions with projected columns and order by columns --- django_mongodb/compiler.py | 27 ++++++++++++--------------- django_mongodb/features.py | 2 -- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/django_mongodb/compiler.py b/django_mongodb/compiler.py index d84790bf4..3a6efbe3e 100644 --- a/django_mongodb/compiler.py +++ b/django_mongodb/compiler.py @@ -358,9 +358,7 @@ def build_query(self, columns=None): if columns is None: extra_fields += ordering_fields if extra_fields: - query.extra_fields = { - field_name: expr.as_mql(self, self.connection) for field_name, expr in extra_fields - } + query.extra_fields = self.get_project_fields(extra_fields, force_expression=True) where = self.get_where() try: expr = where.as_mql(self, self.connection) if where else {} @@ -431,16 +429,18 @@ def _get_aggregate_expressions(self, expr): elif hasattr(expr, "get_source_expressions"): stack.extend(expr.get_source_expressions()) - def get_project_fields(self, columns=None, ordering=None): + def get_project_fields(self, columns=None, ordering=None, force_expression=False): + if not columns: + return {} fields = defaultdict(dict) - for name, expr in columns or []: + for name, expr in columns + (ordering or ()): collection = expr.alias if isinstance(expr, Col) else None try: fields[collection][name] = ( 1 # For brevity/simplicity, project {"field_name": 1} # instead of {"field_name": "$field_name"}. - if isinstance(expr, Col) and name == expr.target.column + if isinstance(expr, Col) and name == expr.target.column and not force_expression else expr.as_mql(self, self.connection) ) except EmptyResultSet: @@ -451,9 +451,6 @@ def get_project_fields(self, columns=None, ordering=None): # should appear in the top-level of the fields dict. fields.update(fields.pop(None, {})) fields.update(fields.pop(self.collection_name, {})) - # Add order_by() fields. - if fields and ordering: - fields.update({alias: expr.as_mql(self, self.connection) for alias, expr in ordering}) # Convert defaultdict to dict so it doesn't appear as # "defaultdict(, ..." in query logging. return dict(fields) @@ -466,28 +463,28 @@ def _get_ordering(self): - A tuple of ('field_name': Expression, ...) for expressions that need to be added to extra_fields. """ - fields = {} + fields = [] sort_ordering = SON() - extra_fields = {} + extra_fields = [] idx = itertools.count(start=1) for order in self.order_by_objs or []: if isinstance(order.expression, Col): field_name = order.expression.as_mql(self, self.connection).removeprefix("$") - fields[field_name] = order.expression + fields.append((order.expression.target.column, order.expression)) elif isinstance(order.expression, Ref): field_name = order.expression.as_mql(self, self.connection).removeprefix("$") else: field_name = f"__order{next(idx)}" - fields[field_name] = order.expression + fields.append((field_name, order.expression)) # If the expression is ordered by NULLS FIRST or NULLS LAST, # add a field for sorting that's 1 if null or 0 if not. if order.nulls_first or order.nulls_last: null_fieldname = f"__order{next(idx)}" condition = When(IsNull(order.expression, True), then=Value(1)) - extra_fields[null_fieldname] = Case(condition, default=Value(0)) + extra_fields.append((null_fieldname, Case(condition, default=Value(0)))) sort_ordering[null_fieldname] = DESCENDING if order.nulls_first else ASCENDING sort_ordering[field_name] = DESCENDING if order.descending else ASCENDING - return tuple(fields.items()), sort_ordering, tuple(extra_fields.items()) + return tuple(fields), sort_ordering, tuple(extra_fields) def get_where(self): return self.where diff --git a/django_mongodb/features.py b/django_mongodb/features.py index 927a88124..822f190a3 100644 --- a/django_mongodb/features.py +++ b/django_mongodb/features.py @@ -61,8 +61,6 @@ class DatabaseFeatures(BaseDatabaseFeatures): "aggregation.tests.AggregateTestCase.test_reverse_fkey_annotate", "aggregation_regress.tests.AggregationTests.test_annotation_disjunction", "aggregation_regress.tests.AggregationTests.test_decimal_aggregate_annotation_filter", - # Invalid $project :: caused by :: Path collision at aggregation_regress_publisher.name - "aggregation_regress.tests.AggregationTests.test_values_list_annotation_args_ordering", # QuerySet.extra(select=...) should raise NotSupportedError instead of: # 'RawSQL' object has no attribute 'as_mql'. "aggregation_regress.tests.AggregationTests.test_annotate_with_extra",