django-tree-queries

django-tree-queries copied to clipboard

Unions, intersections and differences do not seem to work when tree fields are ussed. Since this is not mentioned in the limitations I am not sure if this is deliberate. Specifically, multiple errors are thrown when performing these queries when they include tree fields. I am using django-tree-queries 0.14 and django 4.2.1.

Minimal example:

from tree_queries.models import TreeNode
class TestClass(TreeNode):
    class Meta:
        app_label = "test"

TestClass.objects.with_tree_fields().union(TestClass.objects.with_tree_fields())
# .intersection(...) and .difference(...) throw the same errors

This throws an error:

File ".../venv/lib/python3.11/site-packages/django/db/models/sql/compiler.py", line 550, in <listcomp>
    query.get_compiler(self.using, self.connection, self.elide_empty)
TypeError: TreeQuery.get_compiler() takes from 1 to 3 positional arguments but 4 were given

which seems to be related to the elide_empty not being passed as a keyword argument by this django code. Changing the signature of TreeQuery.get_compiler to get_compiler(self, using=None, connection=None, elide_empty = True, **kwargs) fixes this issue but raises another one. Instead, I get:

File ".../venv/lib/python3.11/site-packages/django/db/models/sql/query.py", line 2151, in add_fields
    raise FieldError(
django.core.exceptions.FieldError: Cannot resolve keyword 'tree_depth' into field. Choices are: __orderbycol1, id, parent, parent_id

It seems like a custom implementation of get_combinator_sql(self, combinator, all) would probably be required on TreeQuery to make this work.

Thanks for the report!

elide_empty was added in https://github.com/django/django/commit/f3112fde981052801e0c2121027424496c03efdf , its absence hasn't been noticed yet it seems.

It would definitely be nice of django-tree-queries supported those types of queries.

Hi!

Just encountered this myself. There's a comment that says:

def get_compiler(self, using=None, connection=None, **kwargs):
        ...
        # **kwargs passes on elide_empty from Django 4.0 onwards
        return TreeCompiler(self, connection, using, **kwargs)

But Django has never sent elide_empty as kwargs, only as positional args. Proof of this is the commit https://github.com/django/django/commit/f3112fde981052801e0c2121027424496c03efdf where it was a positional argument, which was added 3 years ago, before there were no argument at all.

So, to be compiler compliant, django-tree-queries should use the same footprint as the regular Query.get_compiler:

    def get_compiler(self, using=None, connection=None, elide_empty=True):
        if using is None and connection is None:
            raise ValueError("Need either using or connection")
        if using:
            connection = connections[using]
        return connection.ops.compiler(self.compiler)(
            self, connection, using, elide_empty
        )

which should then pass elide_empty into TreeCompiler.

I'll be glad to create a PR with this change as soon as I find some spare time! Hopefully, this is all that's needed to support unions and such, but I wouldn't know since I don't know the internals.

I'll be glad to create a PR with this change as soon as I find some spare time! Hopefully, this is all that's needed to support unions and such, but I wouldn't know since I don't know the internals.

Would be great if that was the case! I'd appreciate a PR.