Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#32134 closed Bug (fixed)

Upgrading 2.2>3.0 causes named tuples used as arguments to __range to error.

Reported by: Gordon Wrigley Owned by: Adam Johnson
Component: Database layer (models, ORM) Version: 3.0
Severity: Normal Keywords:
Cc: Adam Johnson 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

I noticed this while upgrading a project from 2.2 to 3.0.

This project passes named 2-tuples as arguments to range queryset filters. This works fine on 2.2. On 3.0 it causes the following error: TypeError: __new__() missing 1 required positional argument: 'far'.

This happens because django.db.models.sql.query.Query.resolve_lookup_value goes into the tuple elements to resolve lookups and then attempts to reconstitute the tuple with the resolved elements.
When it attempts to construct the new tuple it preserves the type (the named tuple) but it passes a iterator to it's constructor.
NamedTuples don't have the code path for copying an iterator, and so it errors on insufficient arguments.

The fix is to * expand the contents of the iterator into the constructor.

Change History (11)

comment:1 by Adam Johnson, 4 years ago

Cc: Adam Johnson added
Component: UncategorizedDatabase layer (models, ORM)
Has patch: set
Owner: changed from nobody to Adam Johnson
Status: newassigned
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

comment:2 by Mariusz Felisiak, 4 years ago

Patch needs improvement: set
Severity: NormalRelease blocker
Version: 3.03.1

It works in Django 3.0, as far as I'm aware it's a regression in Django 3.1, see 8be79984dce7d819879a6e594ca69c5f95a08378.

Marking as "needs improvement" because the patch breaks the original fix, see #30687.

comment:3 by Gordon Wrigley, 4 years ago

I definitely experience the problem on 3.0.10 and 3.0.0 and the couple of random ones I tried in between.

Last edited 4 years ago by Gordon Wrigley (previous) (diff)

comment:4 by Mariusz Felisiak, 4 years ago

Severity: Release blockerNormal
Version: 3.13.0

After double-checking, it's a regression introduced in Django 3.0 (fa6076daf460df2c2c5ff9f62862f050bc4427f4) when we backported 8be79984dce7d819879a6e594ca69c5f95a08378. Therefore it doesn't qualify for a backport.

comment:5 by Mariusz Felisiak, 4 years ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:6 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

Resolution: fixed
Status: assignedclosed

In a56586ea:

Fixed #32134 -- Fixed crash of range lookup with namedtuple.

Regression in 8be79984dce7d819879a6e594ca69c5f95a08378.

Thanks Gordon Wrigley for the report.

comment:7 by Gordon Wrigley, 4 years ago

It seems weird to me that named tuples would work in latest 2.2 and latest 3.1 but not latest 3.0. That sounds like a regression bug in the 3.0 line.
For me personally it not being backported is somewhat inconvenient. Due to 3rd party lib issues this project can't go straight to 3.1, but I can work around
the lack of backport by overriding the new methods on the named tuples.

in reply to:  7 comment:8 by Mariusz Felisiak, 4 years ago

Replying to Gordon Wrigley:

It seems weird to me that named tuples would work in latest 2.2 and latest 3.1 but not latest 3.0. That sounds like a regression bug in the 3.0 line.
For me personally it not being backported is somewhat inconvenient. Due to 3rd party lib issues this project can't go straight to 3.1, but I can work around
the lack of backport by overriding the new methods on the named tuples.

It will not be backported to Django 3.1. It was introduced in Django 3.0, but reported after the Django 3.1 release. Per our backporting policy this means it doesn't qualify for a backport to 3.0.x or 3.1.x anymore. See Django’s release process for more details.

"Regressions from older versions of Django introduced in the current release series."

Moreover, Django 3.0 is in extended support so it doesn't receive bugfixes anymore (except security patches).

comment:9 by Claude Paroz, 4 years ago

Gordon, you might write to the django-developers mailing list, maybe you can get some support to amend the backport policy in case of regressions. It also looks weird to me that we backport *regressions* to versions n and n-2, but not n-1.

in reply to:  9 comment:10 by Mariusz Felisiak, 4 years ago

Replying to Claude Paroz:

Gordon, you might write to the django-developers mailing list, maybe you can get some support to amend the backport policy in case of regressions. It also looks weird to me that we backport *regressions* to versions n and n-2, but not n-1.

I'm not sure if I understand, we didn't backport this fix to any stable branch. Also, using namedtuple with the __range lookup wasn't tested or documented.

comment:11 by Claude Paroz, 4 years ago

Oh sorry Mariusz, I was wrong about the backport claim. I'm still a bit uneased by this strict backport policy for regressions, but this was already discussed and there is nothing new here.

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