Skip to content

Add docs for ALTER TABLE ... SCATTER #19757

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Sep 25, 2025
Merged

Add docs for ALTER TABLE ... SCATTER #19757

merged 2 commits into from
Sep 25, 2025

Conversation

rmloveland
Copy link
Contributor

(also ALTER INDEX ... SCATTER)

Fixes DOC-286

@rmloveland rmloveland marked this pull request as draft June 11, 2025 20:04
@netlify Netlify
Copy link

netlify bot commented Jun 11, 2025 •
edited
Loading

✅ Deploy Preview for cockroachdb-interactivetutorials-docs canceled.

Name Link
🔨 Latest commit 0530a9b
🔍 Latest deploy log https://app.netlify.com/projects/cockroachdb-interactivetutorials-docs/deploys/68d59de421c92a0008ca9dd5

@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 11, 2025 •
edited
Loading

Files changed:

@netlify Netlify
Copy link

netlify bot commented Jun 11, 2025 •
edited
Loading

✅ Deploy Preview for cockroachdb-api-docs canceled.

Name Link
🔨 Latest commit 0530a9b
🔍 Latest deploy log https://app.netlify.com/projects/cockroachdb-api-docs/deploys/68d59de4ef25500008d1433b

@netlify Netlify
Copy link

netlify bot commented Jun 11, 2025 •
edited
Loading

✅ Netlify Preview

Name Link
🔨 Latest commit 0530a9b
🔍 Latest deploy log https://app.netlify.com/projects/cockroachdb-docs/deploys/68d59de483bd4a000790e7bc
😎 Deploy Preview https://deploy-preview-19757--cockroachdb-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@rafiss rafiss self-requested a review August 18, 2025 19:40
@rmloveland rmloveland marked this pull request as ready for review August 18, 2025 20:02
rafiss
rafiss approved these changes Aug 28, 2025
View reviewed changes
Copy link
Contributor

@rafiss rafiss left a comment

Choose a reason for hiding this comment

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

a review from a KV engineer or PM may be useful here too

@wenyihu6 wenyihu6 self-requested a review September 8, 2025 20:57
wenyihu6
wenyihu6 reviewed Sep 9, 2025
View reviewed changes
wenyihu6
wenyihu6 reviewed Sep 9, 2025
View reviewed changes
@rmloveland
Copy link
Contributor Author

thanks for your comments @wenyihu6 ! i've made some updates and had some questions, PTAL

@rmloveland rmloveland requested a review from wenyihu6 September 10, 2025 15:58
wenyihu6
wenyihu6 reviewed Sep 10, 2025
View reviewed changes
wenyihu6
wenyihu6 approved these changes Sep 10, 2025
View reviewed changes
Copy link

@wenyihu6 wenyihu6 left a comment

Choose a reason for hiding this comment

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

The only blocking comment I have is I think we should mention about replica distribution in the examples as well (at least in words). Other comments are nice-to-have.

@rmloveland
Copy link
Contributor Author

The only blocking comment I have is I think we should mention about replica distribution in the examples as well (at least in words). Other comments are nice-to-have.

thanks for the review @wenyihu6! i've updated to mention the replica distribution (as well as leaseholder) to address your blocking comment and replied to the other ones

@rmloveland rmloveland requested a review from taroface September 15, 2025 17:56
@taroface
Copy link
Contributor

Nice! I'm still reviewing the doc - but SCATTER is missing from both SQL diagrams. We'll need to remove the lines exclude: []*regexp.Regexp{regexp.MustCompile("alter_scatter_index_stmt")}, and exclude: []*regexp.Regexp{regexp.MustCompile("alter_scatter_stmt")}, in diagrams.go and regenerate those diagrams, and make sure any of the exposed param names match what's documented.

@rmloveland rmloveland mentioned this pull request Sep 17, 2025
Merged
@rmloveland
Copy link
Contributor Author

Nice! I'm still reviewing the doc - but SCATTER is missing from both SQL diagrams. We'll need to remove the lines exclude: []*regexp.Regexp{regexp.MustCompile("alter_scatter_index_stmt")}, and exclude: []*regexp.Regexp{regexp.MustCompile("alter_scatter_stmt")}, in diagrams.go and regenerate those diagrams, and make sure any of the exposed param names match what's documented.

thanks for mentioning this, i admit i hadn't looked!

PR to update diagrams is up at cockroachdb/cockroach#153682

@rmloveland
Copy link
Contributor Author

@taroface any additional comments on this? diagrams PRs (cockroachdb/cockroach#153682 and backports) are all merged

taroface
taroface approved these changes Sep 24, 2025
View reviewed changes
Copy link
Contributor

@taroface taroface left a comment

Choose a reason for hiding this comment

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

Sorry for delay! LGTM, just a couple optional nits.

Also, the preview is broken, so please just make sure the diagrams are properly rendering before you merge.

@rmloveland rmloveland force-pushed the 20250611-DOC-286-alter-table-scatter branch from ccfee49 to b5a83d3 Compare September 25, 2025 18:39
@rmloveland
Copy link
Contributor Author

build is green, yay

porting changes to supported versions per https://www.cockroachlabs.com/docs/releases/release-support-policy.html#supported-versions

@rmloveland
Add docs for ALTER TABLE ... SCATTER
a2f49c5 
(also `ALTER INDEX ... SCATTER`)

Fixes DOC-286

NB. Changes applied to all supported versions v23.2 and later
@rmloveland rmloveland force-pushed the 20250611-DOC-286-alter-table-scatter branch from b5a83d3 to a2f49c5 Compare September 25, 2025 19:44
@rmloveland rmloveland enabled auto-merge (squash) September 25, 2025 19:54
@rmloveland rmloveland merged commit 5f84d46 into main Sep 25, 2025
6 checks passed
@rmloveland rmloveland deleted the 20250611-DOC-286-alter-table-scatter branch September 25, 2025 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rafiss rafiss rafiss approved these changes

@taroface taroface taroface approved these changes

@wenyihu6 wenyihu6 wenyihu6 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rmloveland @taroface @rafiss @wenyihu6