-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
sql/rowexec: make bulk row writer (materialized view/ctas) write-at-now #78563
Conversation
Release note: none.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not too familiar with this code, but don't we also need to change the writeTS
to be around the current time, and check the version gate?
I'm not following; if writeAtBatchTS is true we're just going to ignore writeTS and batchTS so why change it?
I've never really understood why we needed the gate: if we just set it the request field, and let an old server ignore it because it is too old, that's the same as not setting it because the version is too old, isn't it? |
Yeah, I think you're right. As long as all SST timestamps are uniform and match
I don't know the details of the bulk IO code well enough to say. If neither the old code nor the new code cares about some SSTs being written as-is and others being rewritten to the current timestamp in mixed-version clusters then it should be fine. |
If writeAtBatchTS is true, we set batchTS to now() on first key of each flush and we set key ts to batchTS (ignoring writeTS), so yeah, that'll be the case. |
TFTR! bors r+ |
Build failed: |
bors r+ |
Build succeeded: |
Looks like you fixed #62932. Thanks! |
Release note: none.