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

[Epic] Stabilize WITH MUTUALLY RECURSIVE #17012

Closed
21 of 26 tasks
antiguru opened this issue Jan 6, 2023 · 6 comments
Closed
21 of 26 tasks

[Epic] Stabilize WITH MUTUALLY RECURSIVE #17012

antiguru opened this issue Jan 6, 2023 · 6 comments
Assignees
Labels
docs-impact Issue will result in some docs impact Epic [Auto] Multi-stage issue

Comments

@antiguru
Copy link
Member

antiguru commented Jan 6, 2023

Product Outcome

Materialize currently supports recursive queries, although it's guarded behind the unsafe flag. We should work towards stabilizing the feature so that we can test it, potentially with customer involvement.

This is a continuation of #11176.

Specification

Stabilize the support for recursive queries. This specifically means:

  • Enable optimizer transformations in the presence of recursive queries.
  • Add testing for recursive queries, including malformed/infinite recursions. For example, divergent dataflows cannot be canceled at the moment (Cancellation of divergent recursive dataflows #16800). Edit: This is fixed now.
  • Create documentation and marketing material to explain how to use recursive queries and what benefit they can bring.

Design

Design doc (original PR: #17820).

Documentation

Recursive CTEs

Required Issues (Must Haves)

Blockers

Discretionary Issues (Nice To Haves)

Follow-up Issues

Success metric

We measure success in the number of queries that use WITH MUTUALLY RECURSIVE:

  • as the absolute number,
  • and the number relative to all queries/views.
@antiguru antiguru added the Epic [Auto] Multi-stage issue label Jan 6, 2023
@frankmcsherry
Copy link
Contributor

frankmcsherry commented Feb 13, 2023

An addition to the specification: WITH MUTUALLY RECURSIVE doesn't currently work render either nested or in sequence in the same dataflow (it panics, because the current dataflow implementation cannot support this). It shouldn't be that much work for me to fix this, but it involves using a new timestamp type (and works in prototype in the DD repo, but surely needs some love porting it to MZ).

@aalexandrov
Copy link
Contributor

An addition to the specification: WITH MUTUALLY RECURSIVE doesn't currently work render either nested or in sequence in the same dataflow (it panics, because the current dataflow implementation cannot support this).

The original design doc lists this as a non-goal:

Nested mutual recursion.

There should either be at most one WITH MUTUALLY RECURSIVE, or appear so after query optimization. We are not able to render nested mutually recursive fragements.

@frankmcsherry IIUC you want to make the above statement a goal for this epic. Can you confirm?

@frankmcsherry
Copy link
Contributor

I'm pretty sure we will be able to do this, and if that turns out to be correct I think there is a simplicity win in terms of not having to explain / check / test the corner cases. It was not obvious that we would be able to do this at the time the doc was written, and ruling it out seemed best at that time.

@frankmcsherry
Copy link
Contributor

I'm pretty sure we will be able to do this,

I'm upgrading this to "we have merged the code that does this".

@aalexandrov
Copy link
Contributor

Thanks. This is already reflected as done in the design doc.

@aalexandrov
Copy link
Contributor

Closing the epic. I'll try to do a final pass on the design doc to reflect more precisely what we managed to do and what is left as future work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-impact Issue will result in some docs impact Epic [Auto] Multi-stage issue
Projects
None yet
Development

No branches or pull requests

4 participants