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

sql: ensure backfills use the same timestamp in their evalContext #54891

Conversation

ajwerner
Copy link
Contributor

@ajwerner ajwerner commented Sep 28, 2020

This fixes a bug whereby column backfills which are restarted and rely on
now() or functions like it actually return the same value throughout
the backfill.

Release note (bug fix): Fixed a bug in column additions which utilized time
functions that could lead to multiple different values being used throughout
a backfill if an internal restart occurred.

@ajwerner ajwerner requested a review from thoszhang September 28, 2020 22:01
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@ajwerner ajwerner force-pushed the ajwerner/fix-bug-in-schema-changer-with-restarts-and-checkpoints branch from 7792321 to 6d529cd Compare September 28, 2020 22:01
@thoszhang thoszhang requested a review from dt September 28, 2020 22:13
Copy link
Contributor

@thoszhang thoszhang left a comment

Choose a reason for hiding this comment

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

I'm fine with the first commit (in particular, with backporting it), but I'm not entirely sold on changing the backfill timestamp behavior for 20.2, since it is a flaw we've had for a while. I think we should at least think of that as a separate change and discuss it in a separate PR.

Reviewed 2 of 2 files at r1, 3 of 3 files at r2.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @dt)

@ajwerner
Copy link
Contributor Author

@lucy-zhang the first commit is now #54900. When that merges I'll rebase here and we can discuss.

This fixes a bug whereby column backfills which are restarted and rely on
`now()` or functions like it actually return the same value throughout
the backfill.

Release note (bug fix): Fixed a bug in column additions which utilized time
functions that could lead to multiple different values being used throughout
a backfill if an internal restart occurred.
@ajwerner ajwerner force-pushed the ajwerner/fix-bug-in-schema-changer-with-restarts-and-checkpoints branch from 6d529cd to a5a8f02 Compare September 29, 2020 12:33
@ajwerner ajwerner changed the title sql: fix bugs in backfill sql: ensure backfills use the same timestamp in their evalContext Sep 29, 2020
@ajwerner
Copy link
Contributor Author

I'm fine with not backporting this.

@tbg tbg added the X-noremind Bots won't notify about PRs with X-noremind label May 6, 2021
@ajwerner ajwerner closed this Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
X-noremind Bots won't notify about PRs with X-noremind
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants