Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CTE statement is not being generated #8

Open
nirizr opened this issue Nov 7, 2018 · 4 comments
Open

CTE statement is not being generated #8

nirizr opened this issue Nov 7, 2018 · 4 comments

Comments

@nirizr
Copy link

nirizr commented Nov 7, 2018

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
Copy link
Contributor

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
Copy link
Author

nirizr commented Nov 8, 2018

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
Copy link
Contributor

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

@nirizr
Copy link
Author

nirizr commented Nov 8, 2018

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.

pgammans added a commit to bva-icx/django-dag that referenced this issue Sep 29, 2021
If the original query has no root
* Prevent crash due to invalid query optimisation of the CTE.
  see dimagi/django-cte#8
* The returned query needs to support the expected annotations or code
  that consumes this will crash
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants