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.

Sign up for GitHub

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

Jump to bottom

opt: don't hold on to the memo after optimization #59065

Closed
RaduBerinde opened this issue Jan 15, 2021 · 0 comments · Fixed by #61863
Closed

opt: don't hold on to the memo after optimization #59065

RaduBerinde opened this issue Jan 15, 2021 · 0 comments · Fixed by #61863
Assignees
@RaduBerinde
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@RaduBerinde
Copy link
Member

We currently hold on to the memo for the duration of a query. The memo could use a lot of memory, e.g. due to join reordering.

I believe that holding on to the memo is only necessary when extracting diagnostics (and for EXPLAIN (OPT,ENV)); even in those cases, we could clear the interner to release expressions that are not part of the final plan.
@RaduBerinde RaduBerinde added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Jan 15, 2021
@RaduBerinde RaduBerinde self-assigned this Jan 15, 2021
@RaduBerinde RaduBerinde mentioned this issue Mar 11, 2021
Merged
RaduBerinde added a commit to RaduBerinde/cockroach that referenced this issue Mar 11, 2021
@RaduBerinde
sql: don't save memo unnecessarily
20a69c8 
We save a reference to the Memo, which is useful for explaining plans.
However, this means that we're holding on to the memory used by the
entire explored memo during execution of the query. This change makes
it so that we only save it only if we're building an explain plan.

Fixes cockroachdb#59065.

Release note: None
@RaduBerinde RaduBerinde mentioned this issue Mar 11, 2021
Merged
craig bot pushed a commit that referenced this issue Mar 12, 2021
@RaduBerinde
Merge #61863
4a39f9a 
61863: sql: don't save memo unnecessarily r=RaduBerinde a=RaduBerinde

We save a reference to the Memo, which is useful for explaining plans.
However, this means that we're holding on to the memory used by the
entire explored memo during execution of the query. This change makes
it so that we only save it only if we're building an explain plan.

Fixes #59065.

Release note: None

Co-authored-by: Radu Berinde <[email protected]>
@craig craig bot closed this as completed in 0416f01 Mar 12, 2021
@RaduBerinde RaduBerinde mentioned this issue Mar 12, 2021
Merged
RaduBerinde added a commit to RaduBerinde/cockroach that referenced this issue Mar 12, 2021
@RaduBerinde
sql: don't save memo unnecessarily
6a3c663 
We save a reference to the Memo, which is useful for explaining plans.
However, this means that we're holding on to the memory used by the
entire explored memo during execution of the query. This change makes
it so that we only save it only if we're building an explain plan.

Fixes cockroachdb#59065.

Release note: None
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@RaduBerinde RaduBerinde

Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

sql: don't save memo unnecessarily RaduBerinde/cockroach
1 participant
@RaduBerinde