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: create new indexes rather than a new table in TRUNCATE #52170

Closed
ajwerner opened this issue Jul 30, 2020 · 7 comments · Fixed by #52235
Closed

sql: create new indexes rather than a new table in TRUNCATE #52170

ajwerner opened this issue Jul 30, 2020 · 7 comments · Fixed by #52235
Assignees
Labels
A-schema-changes A-schema-descriptors Relating to SQL table/db descriptor handling. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@ajwerner
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Today the implementation of TRUNCATE creates a new table with the same structure as an old one. It then drops the old one and renames it to the new one. This has caused a number of problems and bugs. Worse yet, it means that a table ID is not a stable identifier, complicating how we store references to it in other tables.

Describe the solution you'd like

This was probably done because prior to 20.1, we required that every table have a primary index with ID 1. This is no longer the case since we implemented primary key changes. We should just drop all of the existing indexes and create new ones in truncate.

Describe alternatives you've considered
None

Additional context

This will make challenges related to renaming things which are referenced easier. See #10083 and #51090.

One problem here is that the index IDs might get quite high. This could be mitigated with #47981.

@blathers-crl
Copy link

blathers-crl bot commented Jul 30, 2020

Hi @ajwerner, I've guessed the C-ategory of your issue and suitably labeled it. Please re-label if inaccurate.

While you're here, please consider adding an A- label to help keep our repository tidy.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@blathers-crl blathers-crl bot added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Jul 30, 2020
@knz
Copy link
Contributor

knz commented Jul 31, 2020

One problem here is that the index IDs might get quite high.

Instead of a free list you can look at the minimum value used. Then when truncating, if all the indexes fit under the minimum, start from 1 again. This ensures indexes starting from 1 every 2 truncates.

@knz knz added A-schema-changes A-schema-descriptors Relating to SQL table/db descriptor handling. labels Jul 31, 2020
@knz
Copy link
Contributor

knz commented Jul 31, 2020

(I love this plan though.)

@petermattis
Copy link
Collaborator

This is a great idea!

@ajwerner
Copy link
Contributor Author

Full credit goes to @lucy-zhang

@awoods187
Copy link
Contributor

this is cool!

@thoszhang
Copy link
Contributor

@rohany tentatively assigning to you, feel free to update/change as necessary.

rohany added a commit to rohany/cockroach that referenced this issue Aug 11, 2020
Fixes cockroachdb#52170.

This commit updates the implementation of table truncation to create a
new set of indexes for the table, rather than creating a new table ID
for the table's data.

Release note: None
rohany added a commit to rohany/cockroach that referenced this issue Aug 13, 2020
Fixes cockroachdb#52170.

This commit updates the implementation of table truncation to create a
new set of indexes for the table, rather than creating a new table ID
for the table's data.

Release note: None
craig bot pushed a commit that referenced this issue Aug 13, 2020
52235: sql: change table truncation to create new indexes rather than new ID r=rohany a=rohany

Fixes #52170.

This commit updates the implementation of table truncation to create a
new set of indexes for the table, rather than creating a new table ID
for the table's data.

Release note: None

Co-authored-by: Rohan Yadav <[email protected]>
@craig craig bot closed this as completed in d07b25d Aug 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-schema-changes A-schema-descriptors Relating to SQL table/db descriptor handling. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants