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/schemachanger: infrastructure for supporting CREATE statements #104348
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
postamar
reviewed
Jun 6, 2023
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 like this, the general idea is sound. I'm concerned about how many rules this adds, but that wouldn't be a dealbreaker if unavoidable. See my only non-nit comment.
pkg/sql/schemachanger/scplan/internal/rules/current/dep_statement_phase.go
Outdated
Show resolved
Hide resolved
pkg/sql/schemachanger/scplan/internal/rules/current/dep_statement_phase.go
Show resolved
Hide resolved
pkg/sql/schemachanger/scplan/internal/rules/current/dep_statement_phase.go
Outdated
Show resolved
Hide resolved
fqazi
force-pushed
the
statementPhaseAsRule
branch
2 times, most recently
from
June 7, 2023 10:33
3de75fd
to
aa7e0b4
Compare
Previously, we had code inside the stage builder to enforce that only a single transition was allowed per-statement phase. This would have made it hard to implement CREATE statements since we need to apply multiple transitions. To address this, this patch adds dep rules to limit the number of transitions, which will be later disabled for create statements. To help support this a new precedence type is introduced. Epic: none Release note: None merge to 0 <pkg>: <short description - lowercase, no final period> <what was there before: Previously, ...> <why it needed to change: This was inadequate because ...> <what you did about it: To address this, this patch ...>
fqazi
force-pushed
the
statementPhaseAsRule
branch
from
June 7, 2023 20:02
aa7e0b4
to
8f7638f
Compare
Previously, the two version invariant was enforced for all non-dropped descriptors. When creating new descriptors if they are going to be decomposed into individual elements, then similar logic needs to skip added descriptors. To address this, this patch disables these rules for added descriptors. Epic: none Release note: None
fqazi
force-pushed
the
statementPhaseAsRule
branch
from
June 8, 2023 13:51
8f7638f
to
a202661
Compare
Is this PR superseded by #104350 at this point? |
@postamar This is superseded, killing this one off. |
craig bot
pushed a commit
that referenced
this pull request
Jun 29, 2023
104350: sql/schemachanger: support for create sequence inside the declarative schema changer r=fqazi a=fqazi This patch implements CREATE SEQUENCE in the declarative schema changer. The first three commits can be ignored and are included in (#104348, which should be reviewed and merged first). This patch will: - Skip validation/backfill for descriptors in an added state - Fixes prefix resolution creating new objects, where two-part names were not handled properly before - Adds support for CREATE sequence in the declarative schema changer Fixes: #104351 105454: roachtest: update `mixedversion` to always use secure clusters r=srosenberg a=renatolabs Secure clusters are closer to production deployments and also allow us to tests features that we couldn't before, like creating new users with passwords during a test, and then performing SQL operations with those users. In the process of getting this to work, there were a few bugs that needed to be fixed (first commit), and the `cluster` interface needed a small update as well (second commit). Epic: CRDB-19321 Release note: None 105765: ui: fix db page Node/Regions column rendering r=xinhaoz a=xinhaoz Previously, the db page was not updating its columns if the `showNodeRegionsColumn` prop changed. The db details page was also not filtering out the regions column when desired. Epic: none Release note (bug fix): node/regions columns in db and db details page should properly render. This column is hidden for tenants and otherwise is shown for clusters with > 1 node. 105770: persistedsqlstats: skip TestSQLStatsPersistedLimitReached under stress r=zachlite a=zachlite TestSQLStatsPersistedLimitReached succeeds under 'normal' testing conditions. We can and should get this test coverage when we can. Informs #97488 Epic: none Release note: None 105799: kv: expose env var to configure raft entry cache size r=erikgrinaker a=nvanbenschoten Informs #98666. This commit introduces a new `COCKROACH_RAFT_ENTRY_CACHE_SIZE` which can be used to configure the size of the raft entry cache. The default value is 16 MiB. Release note: None Co-authored-by: Faizan Qazi <faizan@cockroachlabs.com> Co-authored-by: Renato Costa <renato@cockroachlabs.com> Co-authored-by: Xin Hao Zhang <xzhang@cockroachlabs.com> Co-authored-by: zachlite <zachlite@gmail.com> Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
To support CREATE statements, we need to be able to do multiple transitions for elements in a single stage. To help support this, this patch will add the following: