-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
importer: clean up importResumer.dropTables #84872
importer: clean up importResumer.dropTables #84872
Conversation
pkg/sql/importer/import_job.go
Outdated
// older-format (v1.1) descriptor. This enables ClearTableData to use a | ||
// RangeClear for faster data removal, rather than removing by chunks. | ||
intoTable.TableDesc().DropTime = int64(1) | ||
if err := gcjob.ClearTableData( |
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.
hmm wasn't this previously conditional on the table actually being empty? What if we have a table that is !empty
, I don't think we want to ClearRange
it right?
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.
oooof. Good catch. Done.
Previously, importResumer.DropTables() assumed that IMPORT INTO could act upon multiple tables, which isn't actually the case, leading to overly complex code. This pr is a simple refactor, in preparation for more import rollback work in cockroachdb#76722 and cockroachdb#70428. Release note: none
3750eed
to
fbaf116
Compare
TFTR! bors=adityamaru |
bors r=adityamaru |
Build succeeded: |
Previously, importResumer.DropTables() assumed that IMPORT INTO could act upon
multiple tables, which isn't actually the case, leading to overly complex code.
This pr is a simple refactor, in preparation for more import rollback work
in #76722 and #70428.
Release note: none