Opened 3 years ago

Last modified 5 months ago

#32398 assigned Bug

Excluding on annotations doesn't apply null handling.

Reported by: Gordon Wrigley Owned by: Simon Charette
Component: Database layer (models, ORM) Version: 3.1
Severity: Normal Keywords:
Cc: Jacob Walls, Simon Charette Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

I would generally expect my_timestamp__year to behave the same as .annotate(my_timestamp_year=ExtractYear("my_timestamp")) however I have noticed that when using exclude the former will account for null's but the later won't.

print(M.objects.exclude(closed_at__year=t.year).values("closed_at__year").query)

SELECT 
    EXTRACT('year' FROM "r_m"."closed_at" AT TIME ZONE 'Europe/London') 
FROM "r_m" 
WHERE NOT (
    "r_m"."closed_at" BETWEEN 2021-01-01 00:00:00+00:00 AND 2021-12-31 23:59:59.999999+00:00 
    AND "r_m"."closed_at" IS NOT NULL
)
print(M.objects.annotate(year=ExtractYear("closed_at")).exclude(year=t.year).values("year").query)

SELECT 
     EXTRACT('year' FROM "r_m"."closed_at" AT TIME ZONE 'Europe/London') AS "year" 
FROM "r_m" 
WHERE NOT (
    "r_m"."closed_at" BETWEEN 2021-01-01 00:00:00+00:00 AND 2021-12-31 23:59:59.999999+00:00
)

Annotated aggregates like min and max also don't account for nulls which leads me to suspect this is a problem with annotations.

I also tried wrapping the ExtractYear in an ExpressionWrapper with output_field=IntegerField(null=True) which didn't change anything.

Change History (20)

comment:1 by Mariusz Felisiak, 3 years ago

Cc: Jacob Walls added
Component: UncategorizedDatabase layer (models, ORM)
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

Thanks for the report. In this case a WHERE clause is returned before handling nullable columns.

comment:2 by Gordon Wrigley, 3 years ago

Whoever looks at it might want to take a quick look at the other early returns in the same function.

comment:3 by Simon Charette, 3 years ago

Cc: Simon Charette added

comment:4 by Jordan Bae, 3 years ago

Owner: changed from nobody to Jordan Bae
Status: newassigned

Could i look into this and fix?

comment:5 by Jacob Walls, 3 years ago

No problem (no need to ask).

comment:6 by Jacob Walls, 3 years ago

Has patch: set
Patch needs improvement: set

comment:7 by Jacob Walls, 3 years ago

Patch needs improvement: unset

comment:8 by Mariusz Felisiak, 3 years ago

Needs tests: set
Patch needs improvement: set

comment:9 by Jordan Bae, 3 years ago

Has patch: unset
Needs tests: unset

comment:10 by Jordan Bae, 3 years ago

Has patch: set
Patch needs improvement: unset

comment:11 by Jordan Bae, 3 years ago

if anyone has time, please check this PR(https://github.com/django/django/pull/14065)

comment:12 by Mariusz Felisiak, 3 years ago

#32684 was a duplicate for a subquery annotation.

comment:13 by Mariusz Felisiak, 3 years ago

Patch needs improvement: set

comment:14 by Gerben Morsink, 3 years ago

I think I have found another case that fails and is related to handling nullable columns:

value = ['1', None]

status_model = StatusModel.objects.filter(field=OuterRef('pk')).order_by('-timestamp')
qs = RelatedModel.annotate(current_status_db=Subquery(status_model.values('status')[:1])).filter(current_status_db__in=value)

qs always returns an empty queryset, while if the filter equals .filter(current_status_db=None) it works correctly.

comment:15 by Jacob Walls, 13 months ago

Owner: changed from Jordan Bae to Jacob Walls

comment:16 by Jacob Walls, 13 months ago

Patch needs improvement: unset

comment:17 by Mariusz Felisiak, 13 months ago

Patch needs improvement: set

Per Simon's comment.

comment:18 by Natalia Bidart, 10 months ago

Owner: changed from Jacob Walls to Simon Charette

Resetting assignee as mentioned here.

comment:19 by Natalia Bidart, 5 months ago

#34959 was also closed as duplicate of this one.

comment:20 by Roman Odaisky, 5 months ago

Or sometimes the null handling is applied in places where it should not be applied:

def isblank(value, true_or_false):
    """Basically the equivalent of `(not value) == true_or_false`"""
    return Exact(Exact(value, 0) | IsNull(value, True), true_or_false)

def lookupify(function):
    class LookupifyLookup(Lookup):
        prepare_rhs = False

        def as_sql(self, compiler, connection):
            return compiler.compile(
                function(self.lhs, self.rhs)
                .resolve_expression(compiler.query)
            )

    return LookupifyLookup

Field.register_lookup(lookupify(isblank), "isblank")

SomeModel.objects.exclude(isblank(F("field"), True))
# correctly excludes zeroes and nulls

SomeModel.objects.exclude(field__isblank=True)
# only excludes zeroes but not nulls!
Note: See TracTickets for help on using tickets.
Back to Top