Opened 2 months ago

Closed 8 weeks ago

Last modified 8 weeks ago

#35344 closed Bug (fixed)

GeneratedField get_col output_field bug

Reported by: Johannes Westphal Owned by: Johannes Westphal
Component: Database layer (models, ORM) Version: dev
Severity: Release blocker Keywords:
Cc: Johannes Westphal Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Related issue #34838.

For generated fields, get_col gets called with output_field=<instance of the generated field>. Consequently, if an alias is specified, the output_field of the generated Col is of type GeneratedField instead of the actual output_field of the GeneratedField, since the current code only handles the case where get_col's output_field parameter is None, but not when it is self.

Error

File "django/contrib/postgres/fields/ranges.py", line 253, in as_postgresql
    cast_internal_type = self.lhs.output_field.base_field.get_internal_type()
AttributeError: 'GeneratedField' object has no attribute 'base_field'

Patch

  • django/db/models/fields/generated.py

    diff --git a/django/db/models/fields/generated.py b/django/db/models/fields/generated.py
    index 257feeeba2..5b6b188df0 100644
    a b class GeneratedField(Field):  
    3939        return Col(self.model._meta.db_table, self, self.output_field)
    4040
    4141    def get_col(self, alias, output_field=None):
    42         if alias != self.model._meta.db_table and output_field is None:
     42        if alias != self.model._meta.db_table and output_field in (None, self):
    4343            output_field = self.output_field
    4444        return super().get_col(alias, output_field)

Change History (12)

comment:2 by Johannes Westphal, 2 months ago

Version: 5.0dev

comment:3 by Simon Charette, 2 months ago

Triage Stage: UnreviewedAccepted

Nice catch and thank you for submitting a PR including a regression test, this is brilliant!

Pending a mention in the 5.0.4 release notes this LGTM!

Last edited 2 months ago by Simon Charette (previous) (diff)

comment:4 by Johannes Westphal, 2 months ago

Should I update the release notes in the PR?

comment:5 by Johannes Westphal, 2 months ago

Well, I added a mention in the 5.0.4 release notes in the PR.

comment:6 by Simon Charette, 2 months ago

yep, that' how it's done!

comment:7 by Natalia Bidart, 8 weeks ago

Thank you Simon and Johannes for your help, the PR looks good, I'm completing the review and I'll merge soon.

While I understand how the provided test is relevant for the fix, I would like to understand a bit better the high-level use case. Would you have an example of a query producing the reported error?

comment:8 by Natalia Bidart, 8 weeks ago

Triage Stage: AcceptedReady for checkin

comment:9 by Johannes Westphal, 8 weeks ago

Here a minimal example to reproduce the crash:

Model

from django.contrib.gis.db import models
from django.contrib.postgres.fields import DateTimeRangeField
from django.contrib.postgres.functions import TransactionNow

class DateTimeRange(models.Func):
    function = 'tstzrange'
    template = "%(function)s(%(expressions)s)"
    output_field = DateTimeRangeField()

    def __init__(self, a, b):
        super().__init__(a, b)

class P(models.Model):
    start = models.DateTimeField(db_default=TransactionNow())
    finish = models.DateTimeField(null=True)
    timerange = models.GeneratedField(expression=DateTimeRange(models.F('start'), models.F('finish')), output_field=DateTimeRangeField(), db_persist=True)

class T(models.Model):
    p1 = models.ForeignKey(P, on_delete=models.CASCADE, related_name='tp1s')
    p2 = models.ForeignKey(P, on_delete=models.CASCADE, related_name='tp2s')

Query

from django.utils import timezone

now = timezone.now()
T.objects.filter(
        p1__timerange__contains=now,
        p2__timerange__contains=now,
        ).count()

Traceback

Traceback (most recent call last):
  File "test.py", line 14, in <module>
    ).count()
  File "django/db/models/query.py", line 620, in count
    return self.query.get_count(using=self.db)
  File "django/db/models/sql/query.py", line 629, in get_count
    return obj.get_aggregation(using, {"__count": Count("*")})["__count"]
  File "django/db/models/sql/query.py", line 615, in get_aggregation
    result = compiler.execute_sql(SINGLE)
  File "django/db/models/sql/compiler.py", line 1549, in execute_sql
    sql, params = self.as_sql()
  File "django/db/models/sql/compiler.py", line 764, in as_sql
    self.compile(self.where) if self.where is not None else ("", [])
  File "django/db/models/sql/compiler.py", line 546, in compile
    sql, params = node.as_sql(self, self.connection)
  File "django/db/models/sql/where.py", line 151, in as_sql
    sql, params = compiler.compile(child)
  File "django/db/models/sql/compiler.py", line 544, in compile
    sql, params = vendor_impl(self, self.connection)
  File "django/contrib/postgres/fields/ranges.py", line 253, in as_postgresql
    cast_internal_type = self.lhs.output_field.base_field.get_internal_type()
AttributeError: 'GeneratedField' object has no attribute 'base_field'

comment:10 by Johannes Westphal <jojo@…>, 8 weeks ago

Resolution: fixed
Status: assignedclosed

In 5f18021:

Fixed #35344, Refs #34838 -- Corrected output_field of resolved columns for GeneratedFields in aliased tables.

Thanks Simon Charette for the review.

comment:11 by Natalia <124304+nessita@…>, 8 weeks ago

In 14ab15d6:

[5.0.x] Fixed #35344, Refs #34838 -- Corrected output_field of resolved columns for GeneratedFields in aliased tables.

Thanks Simon Charette for the review.

Backport of 5f180216409d75290478c71ddb0ff8a68c91dc16 from main

comment:12 by Johannes Westphal, 8 weeks ago

Thank you Natalia and Simon for the fast processing of my bug report and pull request.

Note: See TracTickets for help on using tickets.
Back to Top