[Django] #36029: Deep conditions in FilteredRelation raise ProgrammingError if annotation reused

[Django]

#36029: Deep conditions in FilteredRelation raise ProgrammingError if annotation
reused
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Type: Bug
Status: new | Component: Database
| layer (models, ORM)
Version: 5.1 | Severity: Normal
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
I was debugging a query that failed like this:
{{{#!py
psycopg.errors.UndefinedTable: missing FROM-clause entry for table
"book_editor"
LINE 1: ...r" LEFT OUTER JOIN "filtered_relation_editor" ON (book_edito...
}}}

#33929 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
[https://github.com/django/django/blob/f05edb2b43c347d4929efd52c8e2b4e08839f542/tests/filtered_relation/tests.py#L624-L635
asserts] that a ValueError is raised in a similar situation, so maybe this
test should do that also?

{{{#!diff
diff --git a/tests/filtered_relation/tests.py
b/tests/filtered_relation/tests.py
index 82caba8662..e15efb447b 100644
--- a/tests/filtered_relation/tests.py
+++ b/tests/filtered_relation/tests.py
@@ -668,6 +668,15 @@ class FilteredRelationTests(TestCase):
),
)

+ def test_condition_deeper_relation_name_reused_annotation(self):
+ qs = Author.objects.annotate(
+ book_editor=FilteredRelation(
+ "book",
+ condition=Q(book__editor__name="b"),
+ ),
+ ).annotate(reused_annotation=F("book_editor"))
+ self.assertEqual(qs.count(), 2)
+
def test_with_empty_relation_name_error(self):
with self.assertRaisesMessage(ValueError, "relation_name cannot
be empty."):
FilteredRelation("", condition=Q(blank=""))
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36029>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36029: Deep conditions in FilteredRelation raise ProgrammingError if annotation
reused
-------------------------------------+-------------------------------------

Reporter: Jacob Walls | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: 5.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce):

* stage: Unreviewed => Accepted

Comment:

Thank you for the test case!
--
Ticket URL: <https://code.djangoproject.com/ticket/36029#comment:1>

[Django]

#36029: Deep conditions in FilteredRelation raise ProgrammingError if annotation
reused
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: 5.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Jacob Walls):

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:

{{{#!diff
diff --git a/tests/filtered_relation/tests.py
b/tests/filtered_relation/tests.py

index 82caba8662..80401f4186 100644
--- a/tests/filtered_relation/tests.py
+++ b/tests/filtered_relation/tests.py
@@ -668,6 +668,20 @@ class FilteredRelationTests(TestCase):
),
)

+ def test_condition_deeper_relation_name_implicit_exact(self):
+ msg = (
+ "FilteredRelation's condition doesn't support nested
relations "
+ "deeper than the relation_name (got "
+ "'book__editor__name' for 'book')."
+ )
+ with self.assertRaisesMessage(ValueError, msg):
+ Author.objects.annotate(

+ book_editor=FilteredRelation(
+ "book",
+ condition=Q(book__editor__name="b"),
+ ),
+ )

+
def test_with_empty_relation_name_error(self):
with self.assertRaisesMessage(ValueError, "relation_name cannot
be empty."):
FilteredRelation("", condition=Q(blank=""))
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/36029#comment:2>

[Django]

#36029: Deep conditions in FilteredRelation raise ProgrammingError if annotation
reused
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: 5.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Description changed by Jacob Walls:

Old description:

> I was debugging a query that failed like this:
> {{{#!py
> psycopg.errors.UndefinedTable: missing FROM-clause entry for table
> "book_editor"
> LINE 1: ...r" LEFT OUTER JOIN "filtered_relation_editor" ON
> (book_edito...
> }}}
>
> #33929 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
> [https://github.com/django/django/blob/f05edb2b43c347d4929efd52c8e2b4e08839f542/tests/filtered_relation/tests.py#L624-L635
> asserts] that a ValueError is raised in a similar situation, so maybe
> this test should do that also?
>

> {{{#!diff
> diff --git a/tests/filtered_relation/tests.py
> b/tests/filtered_relation/tests.py

> index 82caba8662..e15efb447b 100644
> --- a/tests/filtered_relation/tests.py
> +++ b/tests/filtered_relation/tests.py
> @@ -668,6 +668,15 @@ class FilteredRelationTests(TestCase):
> ),
> )
>
> + def test_condition_deeper_relation_name_reused_annotation(self):
> + qs = Author.objects.annotate(

> + book_editor=FilteredRelation(
> + "book",
> + condition=Q(book__editor__name="b"),
> + ),

> + ).annotate(reused_annotation=F("book_editor"))
> + self.assertEqual(qs.count(), 2)

> +
> def test_with_empty_relation_name_error(self):
> with self.assertRaisesMessage(ValueError, "relation_name cannot
> be empty."):
> FilteredRelation("", condition=Q(blank=""))
> }}}

New description:

I was debugging a query that failed like this:
{{{#!py
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
[https://github.com/django/django/blob/f05edb2b43c347d4929efd52c8e2b4e08839f542/tests/filtered_relation/tests.py#L624-L635
asserts] that a ValueError is raised in a similar situation, so maybe this

test should do that also? See comment:1

{{{#!diff
diff --git a/tests/filtered_relation/tests.py
b/tests/filtered_relation/tests.py

index 82caba8662..e15efb447b 100644
--- a/tests/filtered_relation/tests.py
+++ b/tests/filtered_relation/tests.py
@@ -668,6 +668,15 @@ class FilteredRelationTests(TestCase):
),
)

+ def test_condition_deeper_relation_name_reused_annotation(self):
+ qs = Author.objects.annotate(

+ book_editor=FilteredRelation(
+ "book",
+ condition=Q(book__editor__name="b"),
+ ),

+ ).annotate(reused_annotation=F("book_editor"))
+ self.assertEqual(qs.count(), 2)

+
def test_with_empty_relation_name_error(self):
with self.assertRaisesMessage(ValueError, "relation_name cannot
be empty."):
FilteredRelation("", condition=Q(blank=""))
}}}

--
--

Ticket URL: <https://code.djangoproject.com/ticket/36029#comment:3>

[Django]

#36029: Deep conditions in FilteredRelation raise ProgrammingError if annotation
reused
-------------------------------------+-------------------------------------

Reporter: Jacob Walls | Owner: Jacob
| Walls
Type: Bug | Status: assigned

Component: Database layer | Version: 5.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls):

* has_patch: 0 => 1
* owner: (none) => Jacob Walls
* status: new => assigned

Comment:

[https://github.com/django/django/pull/18975 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/36029#comment:4>

[Django]

#36029: Deep conditions in FilteredRelation raise ProgrammingError if annotation
reused
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Jacob
| Walls
Type: Bug | Status: assigned
Component: Database layer | Version: 5.1
(models, ORM) |
Severity: Normal | Resolution:

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/36029#comment:5>

[Django]

#36029: Deep conditions in FilteredRelation raise ProgrammingError if annotation
reused
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Jacob
| Walls

Type: Bug | Status: closed

Component: Database layer | Version: 5.1
(models, ORM) |

Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce <42296566+sarahboyce@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"c3a681659c4a982101ffee0d3ef205ac5b310e17" c3a6816]:
{{{#!CommitTicketReference repository=""
revision="c3a681659c4a982101ffee0d3ef205ac5b310e17"
Fixed #36029 -- Handled implicit exact lookups in condition depth checks
for FilteredRelation.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36029#comment:6>