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

[dnm] test RenderPlan with LIR introspection #30532

Closed

Conversation

teskje
Copy link
Contributor

@teskje teskje commented Nov 18, 2024

Motivation

Tips for reviewer

Checklist

  • This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
  • If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.

teskje and others added 6 commits November 16, 2024 16:36
This commit moves the `RustType` impl for `LirId` from a random place to
where the `LirId` type is defined.
This commit extracts the `FlatPlan` structural invariants previously
only defined in documentation into the type system. A `FlatPlan` now
consists of a sequence of `BindStage`s that define the bindings in the
plan, and a final `LetFreePlan` that's free of bindings.
This commit cleans up all the code around the render plan. It updates
comments in both `flat_plan.rs` and rendering, and renames the
`FlatPlan` types to better describe their current roles. For example,
the plan isn't fully flat anymore, so calling it `FlatPlan` is
misleading.

Specifically, the following types were renamed are:
 * `FlatPlan` -> `RenderPlan`
 * `FlatPlanStep` -> `Node`
 * `FlatPlanNode` -> `Expr`
This commit renames the `flat_plan` module to `render_plan`, following
the rename from `FlatPlan` to `RenderPlan` in the previous commit.
This commit changes `RenderPlan`'s `From<Plan>` impl into a `TryFrom`
impl and adds documentation about the shape of `Plan` expected for the
`try_from` to succeed.

Co-authored-by: Frank McSherry <[email protected]>
…ew-introspection-sources"

This reverts commit d3bea77, reversing
changes made to 0883e66.
@teskje teskje closed this Nov 22, 2024
@teskje teskje deleted the render_plan-introspection-test branch November 22, 2024 17:06
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

Successfully merging this pull request may close these issues.

1 participant