Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Major bugfix for multiple annotations in API querysets #965

Merged
merged 7 commits into from Sep 5, 2020

Conversation

SchrodingersGat
Copy link
Member

@SchrodingersGat SchrodingersGat commented Sep 5, 2020

Problem

When performing multiple aggregation annotations on a queryset, the returned results are sometimes not correct. There is a very long discussion here

I thought I had fixed this in a previous PR by adding distinct=True to the annotations. However, this was not the correct solution. The distinct operator only aggregates values which are distinct (unique) and while this resulted in correct behavior in my test cases at the time, it was not sufficient.

Reproduce

This problem only appears in the REST API which uses queryset annotations to improve database performance.

To reproduce, create multiple stock items for a Part with the same quantity. For example, a Part has 10x stock items, each with quantity 5.

Instead of reporting quantity=50 the API data will report quantity=5 (because it ignores rows which do not have "unique" quantity values)...

Solution

The "correct" solution appears to be to use the Django Subquery function.

However this is really unwieldy and difficult to understand.

Luckily, the django-sql-utils module makes this very easy!

Aggregating stock quantity against parts is now as simple as:

        # Filter to limit stock items to "available"
        stock_filter = Q(
            status__in=StockStatus.AVAILABLE_CODES,
            sales_order=None,
            build_order=None,
            belongs_to=None,
            customer=None,
        )

        # Annotate with the total 'in stock' quantity
        queryset = queryset.annotate(
            in_stock=Coalesce(
                SubquerySum('stock_items__quantity', filter=stock_filter),
                Decimal(0)
            ),
        )

Fixes #954

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Part API is reporting incorrect stock count for each part
1 participant