Descendant `update` queries do not include SQL for the CTE
rebeccacremona opened this issue · 5 comments
First, thank you for this terrific library!
When migrating a project from django-mptt, I discovered that with Django 4.2.13
and django-tree-queries 0.19.0
, node.descendants().update(...)
raises django.db.utils.ProgrammingError
: the generated SQL is an UPDATE
that references the __tree
table, but does not include the SQL for building it. In comparison, node.ancestors().update(...)
splits the work into two queries, issuing a SELECT
then an UPDATE
.
Reproduction
#
# Minimal setup
#
# Use the model definition from the docs.
from tree_queries.models import TreeNode
class Node(TreeNode):
name = models.CharField(max_length=100)
# Run migrations, then create a node.
node = Node(name='The Only Node')
node.save()
another_node = Node(name='Another Node', parent=node)
another_node.save()
# Verify that node.descendants() and node.ancestors() work as expected
node.descendants()
node.ancestors()
another_node.descendants()
another_node.ancestors()
#
# Attempt to update descendant queryset, and log the SQL
#
from django.db.utils import ProgrammingError
try:
node.descendants().update(name='A New Name')
except ProgrammingError as e:
# ProgrammingError: missing FROM-clause entry for table "__tree"
# STATEMENT: UPDATE "sample_node" SET "name" = 'A New Name' WHERE ((1 = ANY(__tree.tree_path)) AND NOT ("sample_node"."id" = 1))
print(e)
#
# For comparison, update ancestor queryset.
#
assert another_node.ancestors().update(name='A New Name') == 1
# It works, in an unexpected way:
# it first does a SELECT that includes the `WITH RECURSIVE __rank_table` clause,
# and then does an UPDATE with the found ids
# STATEMENT: UPDATE "sample_node" SET "name" = 'A New Name' WHERE "sample_node"."id" IN (1)
Workaround
We were able to work around this easily in our application by manually taking the same two-query approach as with ancestors
:
descendant_ids = list(node.descendants().values_list('id', flat=True))
Node.objects.filter(ids__in=descendant_ids).update(name='A New Name')
Hi
Thanks for the nice words!
The problem is that we're interfering with some internal parts of the Django ORM when we assing our own TreeQuery
as the query class which should be used.
.update()
runs query.chain(...)
which in turn overrides the query class used by Django:
https://github.com/django/django/blob/adae619426b6f50046b3daaa744db52989c9d6db/django/db/models/query.py#L1227
We could try replacing .update()
with our own method, but we'd probably require a TreeUpdateQuery
or something for that. This sounds quite involved to me (I may be wrong) so maybe a better approach would be to detect these conditions and error out in a more obvious way.
Addendum: I wonder if you could keep it in the database by using a subquery. This seems to work nicely:
>>> Page.objects.filter(id__in=p.descendants(include_self=True).values("id")).update(title="blub")
11
Nice that that works!
In our very specific situation, I ended up wanting to keep around a list of IDs I could reference later, so making two separate queries within a transaction ended up being perfect, but I have to imagine that the subquery approach would work best for others!
I have added tests which verify the behavior with .descendants().update()
in 73ee469
It still doesn't work but at least we'll know if anything changes.
I have added an example to the docs describing how to work around this issue, and I'm going to close it as wontfix. I hope someone finds the time and energy to fix it for real (feel free to submit PRs!) but having a documented workaround is good enough for me.