-
Notifications
You must be signed in to change notification settings - Fork 470
Add docs for CREATE TABLE ... AS OF SYSTEM TIME
#19674
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
✅ Deploy Preview for cockroachdb-api-docs canceled.
|
✅ Deploy Preview for cockroachdb-interactivetutorials-docs canceled.
|
✅ Netlify Preview
To edit notification comments on pull requests, go to your Netlify project configuration. |
bf54120
to
6b1ca8c
Compare
0c99b3d
to
a8f42f6
Compare
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.
these changes lgtm!
i wasn't sure if @dikshant also wanted to have CREATE MATERIALIZED VIEW ... AS OF SYSTEM TIME
documented. i'll leave that decision to him.
Yeah we also want the materialized view part to be documented. But I'll leave it to Rich to decide if he wants to do that in this PR or a new one. |
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.
Overall LGTM but One more example that I think we should add!
I'll do it in another PR! would rather get this smaller one now in so folks can start using this ASAP |
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.
Couple suggestions...
@taroface thanks for your feedback on this, in addition to the per-comment updates in 2a368ce, I went and updated the "undo accidental table deletion" example to be a bit more realistic by having the user drop PTAL and let me know what you think! |
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, 1 follow-up comment
~~~ | ||
|
||
~~~ | ||
CREATE TABLE AS |
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.
Do we also need the line NOTICE: CREATE TABLE ... AS does not copy over indexes, default expressions, or constraints; the new table has a hidden rowid primary key column
as you have further below? It might be a little confusing why it only shows up in one case and not the other.
Fixes DOC-13310