-
Notifications
You must be signed in to change notification settings - Fork 479
correct & update example to add/drop constraint #21542
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
base: main
Are you sure you want to change the base?
Conversation
Files changed: |
✅ Deploy Preview for cockroachdb-interactivetutorials-docs canceled.
|
✅ Deploy Preview for cockroachdb-api-docs canceled.
|
✅ Netlify Preview
To edit notification comments on pull requests, go to your Netlify project configuration. |
rmloveland
left a comment
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.
LGTM modulo the multi DDL inside one transaction thing which we explicitly recommend against in https://www.cockroachlabs.com/docs/v25.4/online-schema-changes#schema-changes-within-transactions
| gem "rss" | ||
| gem "webrick" | ||
| gem "jekyll-minifier" | ||
| gem "bundler", "~> 2.4" |
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.
i think this should be a separate docs-infra type PR
| #### Demo | ||
| {% include_cached youtube.html video_id="MPx-LXY2D-c" %} |
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.
oh man -1000 points to slytherin for adding YT links and then making private during a "cleanup"
| #### Drop and add a primary key constraint | ||
|
|
||
| Suppose that you want to add `name` to the composite primary key of the `users` table, [without creating a secondary index of the existing primary key](#changing-primary-keys-with-add-constraint-primary-key). | ||
| Suppose that you want to add `creation_time` to the composite primary key of the `promo_codes` table, [without creating a secondary index of the existing primary key](#changing-primary-keys-with-add-constraint-primary-key). |
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.
not needed due to https://www.cockroachlabs.com/docs/releases/release-support-policy.html#supported-versions but now that it's here 🆗
| ALTER TABLE promo_codes ALTER COLUMN creation_time SET NOT NULL; | ||
| ~~~ | ||
|
|
||
| 1. Turn off autocommit for DDL for the current session, so that multiple DDL statements can execute within one explicit transaction: |
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.
we should not recommend this, running multiple DDLs in one transaction is not recommended: https://www.cockroachlabs.com/docs/v24.3/online-schema-changes#schema-changes-within-transactions
recommend doing this in two separate implicit transactions one after the other (unless there is a reason i'm missing here why they need to be in the same transaction? if it's just to keep the example neater i'd argue that's not a good reason)
| #### Demo | ||
| {% include_cached youtube.html video_id="MPx-LXY2D-c" %} |
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.
-1000 points to ravenclaw
| #### Demo | ||
| {% include_cached youtube.html video_id="MPx-LXY2D-c" %} |
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.
-1000 points to slytherin
| ALTER TABLE promo_codes ALTER COLUMN creation_time SET NOT NULL; | ||
| ~~~ | ||
|
|
||
| 1. Turn off autocommit for DDL for the current session, so that multiple DDL statements can execute within one explicit transaction: |
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.
please revise to not do multi DDLs in one transaction per the holy book
| #### Demo | ||
| {% include_cached youtube.html video_id="MPx-LXY2D-c" %} |
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.
-1000 points to gryffindor (i was hoping to avoid dinging gryffindor here but have to be fair)
| ALTER TABLE promo_codes ALTER COLUMN creation_time SET NOT NULL; | ||
| ~~~ | ||
|
|
||
| 1. Turn off autocommit for DDL for the current session, so that multiple DDL statements can execute within one explicit transaction: |
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.
same as above re: please revise to not do multi-ddl in one transaction, i think these can just be two implicit transactions in a row
| #### Demo | ||
| {% include_cached youtube.html video_id="MPx-LXY2D-c" %} |
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.
i shall look askance at YT links from now on i must admit
DOC-14789
Update the
ALTER TABLEexample to drop & add constraint atomically. For the following reasons, this example did not work (and may have never worked):primary, nottablename_pkeyas per convention).users) is referenced by several FKs in the MovR dataset, so the txn actually rolls back.autocommit_before_ddlwas introduced, which causes the txn to commit before issuing the DDL, which causes the statements to fail.create_table_with_schema_lockedwas enabled by default, and the MovR tables were adjusted accordingly. This means the tables disallow schema changes unless theschema_lockedstorage param is disabled on the table.The examples are updated (appropriately for each version) to use a different MovR table (
promo_codes), use the correct PK name, disableautocommit_before_ddl, and disable/re-enableschema_lockedto allow the example to work.