Opened 3 months ago

Closed 5 weeks ago

Last modified 5 weeks ago

#28497 closed Bug (fixed)

QuerySet.filter() with a sliced QuerySet crashes

Reported by: Sergey Fedoseev Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Release blocker Keywords:
Cc: Simon Charette Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

PersonSkill.objects.filter(skill=Skill.objects.all()[:1])

ProgrammingError: ОШИБКА:  подзапрос должен вернуть только один столбец (subquery should return single column)
LINE 1: ...nskill" WHERE "test_app_personskill"."skill_id" = (SELECT "t...

Introduced in ec50937bcbe160e658ef881021402e156beb0eaf.

Change History (10)

comment:1 Changed 3 months ago by Tim Graham

Severity: NormalRelease blocker
Summary: filtering with non-values QuerySet crashesQuerySet.filter() with a sliced QuerySet crashes
Triage Stage: UnreviewedAccepted
Version: 1.11master

comment:2 Changed 3 months ago by Simon Charette

Cc: Simon Charette added

We should try to error out with a more appropriate message but I don't think we ever officially supported passing a queryset to an __exact lookup. I vaguely remember a ticket about it but I can't find it anymore.

I think you should be using an __in lookup instead as your query will fail with more than one row returned by a subquery used as an expression as soon as the table backing up your Skill model has more than two rows.

comment:3 Changed 3 months ago by Tim Graham

Maybe #25284 is the related ticket Simon thought of.

comment:4 Changed 3 months ago by Simon Charette

That's the ticket I was referring to, thanks for looking that up Tim!

comment:5 Changed 3 months ago by Tim Graham

Here's a regression test. It's not clear to me why this shouldn't be allowed. What should the error message say?

  • tests/lookup/tests.py

    diff --git a/tests/lookup/tests.py b/tests/lookup/tests.py
    index 25098af..7eeaa26 100644
    a b class LookupTests(TestCase): 
    138138        with self.assertNumQueries(expected_num_queries):
    139139            self.assertEqual(Author.objects.in_bulk(authors.keys()), authors)
    140140
     141    def test_filter_with_sliced(self):
     142         self.assertSequenceEqual(
     143            Article.objects.filter(author=Author.objects.all()[:1]),
     144            [self.a4, self.a2, self.a3, self.a1],
     145        )
     146
    141147    def test_values(self):
    142148        # values() returns a list of dictionaries instead of object instances --
    143149        # and you can specify which fields you want to retrieve.

comment:6 Changed 3 months ago by Simon Charette

What should the error message say?

That __in should be used with multiple elements or a single model instance should be passed? Maybe we could fix the issue and officialize and support as well.

Any thoughts Sergey?

comment:7 in reply to:  5 Changed 2 months ago by Sergey Fedoseev

Replying to Tim Graham:

It's not clear to me why this shouldn't be allowed.

+1

comment:8 Changed 6 weeks ago by Tim Graham

Has patch: set

comment:9 Changed 5 weeks ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In 1b73ccc4:

Fixed #28497 -- Restored the ability to use sliced QuerySets with exact.

Regression in ec50937bcbe160e658ef881021402e156beb0eaf.

Thanks Simon Charette for review.

comment:10 Changed 5 weeks ago by Tim Graham <timograham@…>

In 9b3b7804:

[2.0.x] Fixed #28497 -- Restored the ability to use sliced QuerySets with exact.

Regression in ec50937bcbe160e658ef881021402e156beb0eaf.

Thanks Simon Charette for review.

Backport of 1b73ccc4bf78af905f72f4658cf463f38ebf7b97 from master

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