dimagi / django-cte

Common Table Expressions (CTE) for Django
Other
325 stars 45 forks source link

CTE statement is not being generated #8

Open nirizr opened 5 years ago

nirizr commented 5 years ago

First of all, sorry for posting this as an issue. I could not think of any other place to ask. This is the first time I'm trying django-cte so my mistake could be silly, however I mostly copied the README example.

This is my CTE code:

class Annotation(models.Model):
  objects = django_cte.CTEManager()

  uuid = models.UUIDField(null=True, unique=True)
  instance = models.ForeignKey(Instance, models.CASCADE,
                               related_name='annotations')
  type = models.CharField(max_length=64, choices=TYPE_CHOICES)
  data = models.TextField()
  dependencies = models.ManyToManyField('self', symmetrical=False,
                                        related_name="dependents",
                                        through="Dependency")

   def make_regions_cte(cte):
        return Annotation.objects.filter(
            # start with root nodes
            id__in=annotation_ids
        ).values(
            "id",
            "uuid",
            depth=value0,
        ).union(
            # recursive union: get descendants
            cte.join(Annotation, uuid=cte.col.uuid).values(
                "id",
                "uuid",
                depth=cte.col.depth + value1,
            ),
            all=True,
        )

    cte = With.recursive(make_regions_cte)

    annotations = (
        cte.join(Annotation, uuid=cte.col.uuid)
        .with_cte(cte)
        .annotate(
            depth=cte.col.depth,
        )
        .order_by("depth")
    )

This is the resulting SQL query:

WITH RECURSIVE cte AS ((SELECT "collab_annotation"."id", "collab_annotation"."uuid", ("cte"."depth" + 1) AS "depth" FROM "collab_annotation" INNER JOIN "cte" ON "collab_annotation"."uuid" = ("cte"."uuid"))) SELECT "collab_annotation"."id", "collab_annotation"."uuid", "collab_annotation"."instance_id", "collab_annotation"."type", "collab_annotation"."data", "cte"."depth" AS "depth" FROM "collab_annotation" INNER JOIN "cte" ON "collab_annotation"."uuid" = ("cte"."uuid") ORDER BY "depth" ASC  LIMIT 21; args=(1,)

This is the error I'm getting:

django.db.utils.ProgrammingError: recursive query "cte" does not have the form non-recursive-term UNION [ALL] recursive-term
LINE 1: WITH RECURSIVE cte AS ((SELECT "collab_annotation"."id", "co...

Thanks for reading through!

millerdev commented 5 years ago

This is caused by a "feature" of django that removes a UNION clause if django can deduce that the unioned query will return no rows (when the WHERE clause will always evaluate to false, for example). Prior to django-cte, this was probably a safe thing to do since standard union queries do not reference names defined outside of the query as recursive CTEs do.

My guess is the filter on the first CTE query has an empty list:

return Annotation.objects.filter(
    # start with root nodes
    id__in=annotation_ids  # <---- annotation_ids is probably an empty list

You can work around this by checking if the query is empty first, and not doing the CTE in that case:

roots = Annotation.objects.filter(id__in=annotation_ids)

if roots.query.is_empty():
    # construct a query that does not use a CTE since the CTE is empty
    ...
else:
    # construct CTE query as in your example
    ...

Another way to work around it would be to add an expression that will fool django. Something like this (although I'm fairly certain Literal doesn't work this way so this is not a working example):

return Annotation.objects.filter(
    # start with root nodes
    Q(id__in=annotation_ids) |
    Q(Literal('1 = 2'))  # <--- broken example. Literal doesn't work like that
).values(
    ...
nirizr commented 5 years ago

Thanks! That clarification really made it a lot clearer! I've managed to notice I had a typo in the parameter I was reading from the query.

I'm wondering if you think it's worth the trouble of trying to detect (and warn users) somehow?

millerdev commented 5 years ago

Adding a warning is good idea. I'll see what I can do.

nirizr commented 5 years ago

Cool!

I would have given it a shot myself, but to be honest I don't quite understand how this works behind the scenes, definitely not well enough to do something like that.