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

backupccl: prevent temporary tables from backupccl #53478

Merged
merged 1 commit into from
Aug 27, 2020

Conversation

otan
Copy link
Contributor

@otan otan commented Aug 26, 2020

Filter out temporary tables from resolvable descriptors.

Release justification: low risk, high benefit changes

Resolves: #50902

Release note (sql change): Fix a bug where temporary tables may be
included in BACKUP commands.

@otan otan requested review from a team and pbardea and removed request for a team August 26, 2020 15:48
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@otan otan force-pushed the backupccl_temp_table branch 2 times, most recently from 4d824db to eba07f7 Compare August 26, 2020 16:51
Copy link
Contributor

@pbardea pbardea left a comment

Choose a reason for hiding this comment

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

Thanks! One small testing comment

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @otan and @pbardea)


pkg/ccl/backupccl/testdata/backup-restore/temp-tables, line 39 at r1 (raw file):


exec-sql
BACKUP DATABASE d1 TO 'nodelocal://0/d1_backup/'

For completeness, can we also add a test for BACKUP DATABASE d1.*?

Copy link
Contributor Author

@otan otan left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @pbardea)


pkg/ccl/backupccl/testdata/backup-restore/temp-tables, line 39 at r1 (raw file):

Previously, pbardea (Paul Bardea) wrote…

For completeness, can we also add a test for BACKUP DATABASE d1.*?

Done.

Filter out temporary tables from resolvable descriptors.

Release justification: low risk, high benefit changes

Release note (sql change): Fix a bug where temporary tables may be
included in `BACKUP` commands.
@otan otan force-pushed the backupccl_temp_table branch from eba07f7 to d3b7085 Compare August 27, 2020 01:29
@otan
Copy link
Contributor Author

otan commented Aug 27, 2020

bors r=pbardea

@craig
Copy link
Contributor

craig bot commented Aug 27, 2020

Build succeeded:

@craig craig bot merged commit 1f1ac2e into cockroachdb:master Aug 27, 2020
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.

backupccl: temporary tables should not be included in backup
3 participants