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]: Initial read holds implementation #22976

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

maddyblue
Copy link
Contributor

@maddyblue maddyblue commented Nov 6, 2023

WIP.

See MaterializeInc/database-issues#6895, MaterializeInc/database-issues#5094

Motivation

  • This PR adds a known-desirable feature.

Tips for reviewer

Checklist

  • This PR has adequate test coverage / QA involvement has been duly considered.
  • 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).
  • This PR includes the following user-facing behavior changes:


The `HOLD` object has an associated time that must be written down somewhere on creation then updated on advancement.
This will use a persist shard to back it, and is thus similar to a 1-row table.
As a result of not storing it in the catalog (see Alternatives for the motivation), the `AT <time>` syntax must not be part of the `create_sql` of the `HOLD` object and must be purified away.
Copy link
Contributor Author

@maddyblue maddyblue Nov 6, 2023

Choose a reason for hiding this comment

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

Unclear if this needs to matter or not. It's possible the AT should be purified (that is, if not present, it is added to the statement) and persisted in the catalog (and show up for the SHOW commands), but bootstrap is smart enough to ignore it on restart. Thoughts?

@maddyblue maddyblue changed the title [DNM]: Initial read holds implementation and design doc [DNM]: Initial read holds implementation Nov 13, 2023
@maddyblue maddyblue mentioned this pull request Nov 13, 2023
5 tasks
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