Opened 5 years ago
Last modified 2 years 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 , 5 years ago
| Cc: | added |
|---|---|
| Component: | Uncategorized → Database layer (models, ORM) |
| Triage Stage: | Unreviewed → Accepted |
| Type: | Uncategorized → Bug |
comment:2 by , 5 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 , 5 years ago
| Cc: | added |
|---|
comment:4 by , 5 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
Could i look into this and fix?
comment:7 by , 5 years ago
| Patch needs improvement: | unset |
|---|
comment:8 by , 5 years ago
| Needs tests: | set |
|---|---|
| Patch needs improvement: | set |
comment:9 by , 5 years ago
| Has patch: | unset |
|---|---|
| Needs tests: | unset |
comment:10 by , 5 years ago
| Has patch: | set |
|---|---|
| Patch needs improvement: | unset |
comment:11 by , 5 years ago
if anyone has time, please check this PR(https://github.com/django/django/pull/14065)
comment:13 by , 4 years ago
| Patch needs improvement: | set |
|---|
comment:14 by , 4 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 , 3 years ago
| Owner: | changed from to |
|---|
comment:20 by , 2 years 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!
Thanks for the report. In this case a
WHEREclause is returned before handling nullable columns.