Opened 11 hours ago

Last modified 3 hours ago

#36029 new Bug

Deep conditions in FilteredRelation raise ProgrammingError if annotation reused

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

Description (last modified by Jacob Walls)

I was debugging a query that failed like this:

psycopg.errors.UndefinedTable: missing FROM-clause entry for table "book_editor"
LINE 1: ...r" LEFT OUTER JOIN "filtered_relation_editor" ON (book_edito...

#34229 seems related. Here, the key is using a deeper condition relation (book__editor) than the filtered relation (book) and then reusing that attempted annotation in another annotate().

Failing unit test. The test right above asserts that a ValueError is raised in a similar situation, so maybe this test should do that also? See comment:1

  • tests/filtered_relation/tests.py

    diff --git a/tests/filtered_relation/tests.py b/tests/filtered_relation/tests.py
    index 82caba8662..e15efb447b 100644
    a b class FilteredRelationTests(TestCase):  
    668668                ),
    669669            )
    670670
     671    def test_condition_deeper_relation_name_reused_annotation(self):
     672        qs = Author.objects.annotate(
     673            book_editor=FilteredRelation(
     674                "book",
     675                condition=Q(book__editor__name="b"),
     676            ),
     677        ).annotate(reused_annotation=F("book_editor"))
     678        self.assertEqual(qs.count(), 2)
     679
    671680    def test_with_empty_relation_name_error(self):
    672681        with self.assertRaisesMessage(ValueError, "relation_name cannot be empty."):
    673682            FilteredRelation("", condition=Q(blank=""))

Change History (3)

comment:1 by Sarah Boyce, 7 hours ago

Triage Stage: UnreviewedAccepted

Thank you for the test case!

comment:2 by Jacob Walls, 3 hours ago

Here's a better unit test that asserts ValueError is raised. This passes if you adjust the lookup to add an explicit __exact, so hopefully the solve is to just account for implicit exact when counting relation depths:

  • tests/filtered_relation/tests.py

    diff --git a/tests/filtered_relation/tests.py b/tests/filtered_relation/tests.py
    index 82caba8662..80401f4186 100644
    a b class FilteredRelationTests(TestCase):  
    668668                ),
    669669            )
    670670
     671    def test_condition_deeper_relation_name_implicit_exact(self):
     672        msg = (
     673            "FilteredRelation's condition doesn't support nested relations "
     674            "deeper than the relation_name (got "
     675            "'book__editor__name' for 'book')."
     676        )
     677        with self.assertRaisesMessage(ValueError, msg):
     678            Author.objects.annotate(
     679                book_editor=FilteredRelation(
     680                    "book",
     681                    condition=Q(book__editor__name="b"),
     682                ),
     683            )
     684
    671685    def test_with_empty_relation_name_error(self):
    672686        with self.assertRaisesMessage(ValueError, "relation_name cannot be empty."):
    673687            FilteredRelation("", condition=Q(blank=""))

comment:3 by Jacob Walls, 3 hours ago

Description: modified (diff)
Note: See TracTickets for help on using tickets.
Back to Top