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

Introduce an abstraction for handling stateful traversal of the tree #1018

Merged
merged 7 commits into from
Oct 30, 2024

Conversation

vil1
Copy link
Contributor

@vil1 vil1 commented Oct 23, 2024

While still being able to accumulate errors, produce partial results, etc.

The base principle is to build a representation of the computation (parsing, visiting, optimizing, generating, etc) that will eventually get run. This representation keeps track of a state (of arbitrary type if we wanted to, but we practically stick to RemorphContext for now) along with the result it is producing.

This way, we can interleave any step of the computation with updates to this state, like counting the statements processed thus far (for any definition of what a statement is, including arbitrarily nested subqueries), the branch of the IR tree currently being processed, etc.

Similarly, at any point of the computation, we can inspect the current state, to guide said computation (by customizing error messages with contextual information for example).

@vil1 vil1 requested review from nfx and jimidle October 23, 2024 15:44
Copy link

github-actions bot commented Oct 23, 2024

Coverage tests results

443 tests  ±0   409 ✅ ±0   4s ⏱️ ±0s
  6 suites ±0    34 💤 ±0 
  6 files   ±0     0 ❌ ±0 

Results for commit 26f8ea9. ± Comparison against base commit d69ffdd.

♻️ This comment has been updated with latest results.

@vil1 vil1 marked this pull request as ready for review October 29, 2024 17:22
@vil1 vil1 force-pushed the feat/manage-state branch 2 times, most recently from 77c44fd to ede2f07 Compare October 30, 2024 10:26
@vil1 vil1 requested a review from nfx October 30, 2024 10:30
@vil1 vil1 added the internal technical pr's not end user facing label Oct 30, 2024
Copy link
Collaborator

@nfx nfx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@nfx nfx merged commit 50a3f23 into main Oct 30, 2024
8 checks passed
@nfx nfx deleted the feat/manage-state branch October 30, 2024 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal technical pr's not end user facing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants