sql, stable ids for roles: complete remaining role id migrations in system tables #87079

RichardJCai · 2022-08-29T23:05:56Z

Role Ids were added to system.users in #81457.

In 22.2, once the migration is done and we're on V22_2SystemUsersIDColumnIsBackfilled version, we're guaranteed every user has an ID. Furthermore, in 23.1, we're guaranteed that all users have ids.

Eventually, we want to start using IDs to refer to users (ie for privilege checks) instead of usernames and support operations such as alter role rename.

We still have to migrate the following tables:

The system.role_members migration is PR is out #85928 and will wait for the 22.2 branch cut before merging.

The migrations should look like the migration done in #85845

Jira issue: CRDB-19140
Epic CRDB-19134

The text was updated successfully, but these errors were encountered:

ajwerner · 2022-10-16T23:49:39Z

Not explicitly mentioned, but the privileges stored in descriptors need to be migrated.

86968: sql: SHOW QUERIES lazily interpolates placeholders r=jordanlewis a=jordanlewis SHOW QUERIES (and crdb_internal.node_queries, cluster_queries) interpolates placeholder values into the statement so that it is possible to see the placeholder values of a prepared statement - but it used to do this unconditionally during statement execution. This is an expensive process that spends a lot of CPU for little reason, since the interpolation was happening in the hot path of every query. Now, we include the placeholder values as a separate array in the internal representation of active queries, and interpolate the values only when they're being pulled out to examine, to avoid the unconditional runtime interpolation costs. As a side effect of this change, the original comments in a query are now included in SHOW QUERIES and the two active queries tables. Release note (sql change): the query field in the crdb_internal.node_queries, crdb_internal.cluster_queries, and SHOW QUERIES commands now includes the original comments in the queries. Informs CRDB-17299 89753: diagrams: Diagrams job no longer fails when no diagrams change r=nickvigilante a=nickvigilante If there are PRs that don't affect the diagrams or their related files, the Publish SQL Grammar Diagrams job will fail, because `git commit` produces a non-zero exit code if there are no files changed in the working directory. This fixes that problem. Release note: None Fixes DOC-5642 90784: ui: simplify insights sagas, fix contention query filter r=ericharmeling a=ericharmeling This PR simplifies the insights sagas by moving the insights store refresh interval to the components and removing the root-level reset saga. The commit also updates the SQL query for the transaction contention events to not JOIN with the internal insights table, and to filter out all unresolved txn fingerprint IDs. Fixes #90142. Release note: None Loom: https://www.loom.com/share/f12685c712124560b326f211b44a06d1 90800: server: use GRANT statement for test server auth user role grant r=knz,rafiss a=andyyang890 Previously, the admin role is granted to the auth user by manually inserting a row into system.role_members. This change replaces the row insertion with a GRANT statement to abstract away the interaction with system tables. Part of #87079 Release note: None 90812: bazel: set correct flags for `printf` analyzer r=rail,healthy-pod a=rickystewart Closes #80006. Release note: None Epic: CRDB-8349 90838: vendor: bump Pebble to 37cf5274896b r=nicktrav a=jbowens ``` 37cf5274 crossversion: explicitly set artifacts directory fd7e9660 vfs: use `Frsize` for computing total size on Linux 5c65b3d5 Fix lint errors from cockroachdb/pebble#2051 ``` Epic: none Release note: None Co-authored-by: Jordan Lewis <[email protected]> Co-authored-by: Nick Vigilante <[email protected]> Co-authored-by: Eric Harmeling <[email protected]> Co-authored-by: Andy Yang <[email protected]> Co-authored-by: Ricky Stewart <[email protected]> Co-authored-by: Jackson Owens <[email protected]>

91517: sql: add role_id and member_id columns to system.role_members table r=rafiss a=andyyang890 This patch adds user ID columns to the system.role_members table. The new `role_id` column corresponds to the existing `role` column and the new `member_id` column corresponds to the existing `member` column. Migrations are also added to alter and backfill the table in older clusters. Part of #87079 Release note: None Co-authored-by: Andy Yang <[email protected]>

93301: sql,backupccl: set system.role_members ID columns to NOT NULL r=stevendanna,rafiss a=andyyang890 This patch sets the newly added `role_id` and `member_id` columns in `system.role_members` to be NOT NULL. It also changes the `RESTORE` logic to be able to handle the case when restoring from a backup where the `system.role_members` table did not have the columns. Part of #87079 Release note: None 94796: changefeedccl: add new fine-grained permissions r=jayshrivastava a=jayshrivastava This change updates permissions semantics related to creating and managing changefeeds. ### Summary The `CONTROLCHANGEFEED` role option will be deprecated in the future (see #94757). With this change, usages of `CONTROLCHANGEFEED` will come with a deprecation warning. Its existing behavior (see rules for creating changefeeds below) remains the same. The `SELECT` and `CHANGEFEED` privileges will be used for changefeeds henceforth: The `SELECT` privilege on a set of tables allows a user to run core changefeeds against them. The `CHANGEFEED` privilege on a set of tables allows a user to run enterprise changefeeds on them, and also manage the underlying changefeed job (ie. view, pause, cancel, and resume the job). Notably, a new cluster setting `changefeed.permissions.enforce_external_connections` is added and set to `false` by default. Enabling this setting restricts users with `CHANGEFEED` on a set of tables to create enterprise changefeeds into external connections only. To use a given external connection, a user typically needs the `USAGE` privilege on it. Note `ALTER DEFAULT PRIVILEGES` can be used with both the `CHANGEFEED` and `SELECT` privileges to assign course-grained permissions (ie. assign permissions to all tables in a schema rather than manually assign them for each table). ### Additional Details #### Creating a Changefeed Before this change, to create a changefeed, these checks were made in order: (a) If the user has the `CONTROLCHANGEFEED` role, then they require the `SELECT`privilege on all targeted tables (b) Otherwise, the user requires the `CHANGEFEED` privilege on all targeted tables With this change, these checks are updated: (a) If the user has the `CONTROLCHANGEFEED` role, then they require the `SELECT` privilege on all targeted tables. Note: creating a changefeed this way will now produce a deprecation notice. (b) If the changefeed is a core changefeed, they require the `SELECT` privilege on all targeted tables (c) Otherwise, the user requires the `CHANGEFEED` privilege on all targeted tables. Note: If `changefeed.permissions.enforce_external_connections` (disabled by default) is set to true, then the user will only be able to create a changefeed into an external connection which they have the `USAGE` privilege on. #### Managing a Changefeed Before this change, to manage a changefeed job `J` (defined a viewing, pausing, resuming, and canceling), a user `U` could do so if they met at least one of the following conditions: (a) `U` is an admin (b) `U` is not an admin and `J` is owned by `U` (only for SHOW JOBS) (c) `U` is not an admin, `J` is not owned by an admin, and `U` has the `CONTROLJOB` role With this change, the conditions are updated: (a) `U` is an admin (b) `U` is not an admin and `J` is owned by `U` (only for `SHOW JOBS` or `SHOW CHANGEFEED JOBS`) (c) `U` is not an admin, `J` is not owned by an admin, and `U` has the `CONTROLJOB` role (d) `U` is not an admin, `J` is not owned by an admin, `J` is a changefeed job, and `U` has the `CHANGEFEED` privilege on targeted tables #### Altering a Changefeed Before this change, permissions related to altering changefeeds with `ALTER CHANGEFEED` were not explicitly defined (we did not have tests to assert its behavior, but there were some permissions checks regardless). Basically, a user needed access to view a job (ie. look up it’s job ID via `SHOW JOBS`) and they needed to be able to create a new job. After all, `ALTER CHANGEFEED` is essentially the same as creating a new job after stopping the old one. With this change, the same rules apply: the user needs to be able to access the existing job and to be able to create a new changefeed with the new rules introduced in this change respectively. Fixes: #94756 Fixes: #92261 Fixes: #87884 Fixes: #85082 Fixes: #94759 Informs: #94757 Epic: CRDB-19709 Release note (enterprise change): The `CONTROLCHANGEFEED` role option will be deprecated in the future (see #94757). With this change, usages of `CONTROLCHANGEFEED` will come with a deprecation warning. Its existing behavior (see rules for creating changefeeds above) remains the same. The `SELECT` and `CHANGEFEED` privileges will be used for changefeeds henceforth: The `SELECT` privilege on a set of tables allows a user to run core changefeeds against them. The `CHANGEFEED` privilege on a set of tables allows a user to run enterprise changefeeds on them, and also manage the underlying changefeed job (ie. view, pause, cancel, and resume the job). Notably, a new cluster setting `changefeed.permissions.enforce_external_connections` is added and set to `false` by default. Enabling this setting restricts users with `CHANGEFEED` on a set of tables to create enterprise changefeeds into external connections only. To use a given external connection, a user typically needs the `USAGE` privilege on it. Note `ALTER DEFAULT PRIVILEGES` can be used with both both the `CHANGEFEED` and `SELECT` privileges to assign coarse-grained permissions (ie. assign permissions to all tables in a schema rather than manually assign them for each table). 94968: server: tenant access to node diagnostics and metrics r=abarganier,knz a=dhartunian Previously, tenants were unable to access cluster-level node information, and could not make timeseries queries. This is due to the fact that tenants do not have access to gossip data and cannot make aritrary KV requests. Any of these requests must go through the `kvtenant.Connector` interface which authorizes these requests and proxies them over gRPC to the KV server. Future changes will gate these abilities behind Tenant privileges. Epic: CRDB-12100 Resolves: #94967 Release note: None 95168: pkg/ui: Remove deprecated tracez frontend code. r=benbardin a=benbardin Release note: None Part of: #83679 95327: grunning: synchronize access to results in `TestProportionalGoroutines` r=rickystewart a=healthy-pod This code change fixes a race in `TestProportionalGoroutines`. Release note: None Epic: none Co-authored-by: Andy Yang <[email protected]> Co-authored-by: Jayant Shrivastava <[email protected]> Co-authored-by: David Hartunian <[email protected]> Co-authored-by: Ben Bardin <[email protected]> Co-authored-by: healthy-pod <[email protected]>

96205: sql: simplify user id querying for GRANT ROLE r=ZhouXing19 a=andyyang890 This patch inlines the querying of the role and member user ids within the GRANT ROLE logic instead of making separate internal executor queries. Part of #87079 Release note: None 96222: server: add missing RPC metrics for secondary tenants r=dhartunian a=knz This patch adds the missing two AddMetricStruct calls on the rpcContext. I have manually audited the remainder of the code to compare with that of `NewServer()` in `server.go`. These were the main two that were missing (there's another one, which I will also fix in a later commit). Generally, this entire function would benefit from the same approach we've used when we refactored `PreStart`: reorganize the code so that the two functions (from `server.go` and `tenant.go`) can be compared side-by-side to identify mismatches. This is left to future work. Release note: None Epic: CRDB-14537 Co-authored-by: Andy Yang <[email protected]> Co-authored-by: Raphael 'kena' Poss <[email protected]>

andyyang890 · 2023-02-06T23:34:20Z

Do we also need to migrate system.scheduled_jobs and system.external_connections? They both have an owner column.

rafiss · 2023-02-09T19:56:26Z

Yes, let's do those ones also.

93090: sql: add user_id column to system.privileges table r=rafiss a=andyyang890 This patch adds a new `user_id` column to the `system.privileges` table, which corresponds to the existing `username` column. Migrations are also added to alter and backfill the table in older clusters. Part of #87079 Release note: None Co-authored-by: Andy Yang <[email protected]>

96295: changefeedccl: Rename and remove CDC functions r=miretskiy a=miretskiy This PR renames and removes CDC specific functions, while maintaining backward compatability. * `cdc_is_delete()` function removed. It is replaced with `event_op()` function which returns a string describing the type of event. If the changefeed is running with `diff` option, then this function returns `insert`, `update`, or `delete`. If changefeed is running without the `diff` option, we can't tell an update from insert, so this function returns `upsert` or `delete`. * `cdc_mvcc_timestamp()` function removed. This information can be accessed via cockroach standard system column `crdb_internal_mvcc_timestamp`. The same timestamp column is avaiable in the previous row state `(cdc_prev).crdb_internal_mvcc_timestamp` * `cdc_updated_timestamp()` function renamed as `event_schema_timestamp()` Fixes #92482 Epic: CRDB-17161 Release note (enterprise change): Deprecate and replace some of the changefeed specific functions made available in preview mode in 22.2 release. While these functions are deprecated, old changefeed transformations should continue to function properly. Customers are encouraged to closely monitor their changefeeds created during upgrade. While effort was made to maintain backward compatibility, the updated changefeed transformation may produce slightly different output (different column names, etc). 97041: parser: add `QUERY` to `bare_label_keyword` r=ZhouXing19 a=ZhouXing19 As revealed by #84309, some keywords not added to `bare_label_keywords` in `sql.y` make the `SELECT ... <keyword>` statement error out, which is not compatible with postgres. This commit is to add the `QUERY` keyword per a support ticket. We're not adding the whole `unreserved_keyword` list here as having some of them in `bare_label_keyword`, such as `DAY`, brings `reduce` errors. Postgres: ``` postgres=# select substring('stringstringstring',1,10) query; query ------------ stringstri (1 row) ``` CRDB: ``` root@localhost:26257/defaultdb> select substring('stringstringstring',1,10) query; invalid syntax: statement ignored: at or near "query": syntax error SQLSTATE: 42601 DETAIL: source SQL: select substring('stringstringstring',1,10) query ^ ``` informs #84309 Release Note(bug fix): fixed the syntax error for `SELECT ... QUERY` (without AS) statement. 97134: storage: include version in benchmark fixture directory r=RaduBerinde a=RaduBerinde This benchmark is sometimes failing in CI and I cannot reproduce locally. The error is consistent with a leftover fixture from an older version (which shouldn't happen because CI should be cleaning up the repo). This change adds the version to the fixture name so that this kind of cross-version problem cannot occur. This is a good idea regardless of the CI issue. Informs #97061 Release note: None Epic: none 97149: sql: add missing STORING clause in system.privileges user ID migration r=rafiss a=andyyang890 This patch fixes a discrepancy between the bootstrap schema and user ID migration for the system.privileges table. Part of #87079 Release note: None Co-authored-by: Yevgeniy Miretskiy <[email protected]> Co-authored-by: Jane Xing <[email protected]> Co-authored-by: Radu Berinde <[email protected]> Co-authored-by: Andy Yang <[email protected]>

98522: ccl/oidcccl: support principal matching on list claims r=dhartunian a=cameronnunez Previously, matching on ID token claims was not possible if the claim key specified had a corresponding value that was a list, not a string. With this change, matching can now occur on claims that are list valued in order to add login capabilities to DB Console. It is important to note that this change does NOT offer the user the ability to choose between possible matches; it simply selects the first match to log the user in. This change also adds more verbose logging about ID token details. Epic: none Fixes: #97301, #97468 Release note (enterprise change): The cluster setting `server.oidc_authentication.claim_json_key` for DB Console SSO now accepts list-valued token claims. Release note (general change): Increasing the logging verbosity is more helpful with troubleshooting DB Console SSO issues. 98739: sql: simplify V23_1ExternalConnectionsTableHasOwnerIDColumn gating r=adityamaru a=andyyang890 Informs #87079 Release note: None 98892: kvcoord: Use correct timestamp when restarting range r=miretskiy a=miretskiy Recent changes to rangefeed library (#97957) introduced a silly bug (incorrect code completion/copy paste). Use correct timestamp when resuming range feed. Issue: None Epic: None Release note: None Co-authored-by: Cameron Nunez <[email protected]> Co-authored-by: Andy Yang <[email protected]> Co-authored-by: Yevgeniy Miretskiy <[email protected]>

98897: server: fix tenant auth for status server r=knz,THardy98 a=dhartunian Previously, the authentication for gRPC endpoints that are exposed via HTTP on the tenant was not implemented correctly. Because the HTTP session is decoded into gRPC metadata, and that metadata was contained in the Context object passed through to the Tenant Connector, the username from the tenant could leak into the kv layer and be treated as an authenticated username. If that username happened to match one in the system tenant it would be accepted as valid. Additinally, some endpoints were missing their authentication code. This did not break functionality because a gRPC request without any metadata is treated as an internal request with admin permissions. *Warning*: If a request contains a validated username as part of gRPC metadata and that metadata is preserved as the request is handed down to the KV layer, it could be interpreted as a valid user on the system tenant and cause an escalation of privileges. This commit adds authentication to the HotRangesV2 endpoint and SpanStats endpoints which were missing it, and contains tests that ensure that the endpoints return errors when the user does not have the correct permissions. Epic: CRDB-12100 Release note: None 98958: sql,backupccl: set system table user ID columns to be NOT NULL r=rafiss,stevendanna a=andyyang890 This PR sets the user ID columns in system tables to be NOT NULL and when applicable, updates the `RESTORE` logic to account for the case where a backup may have been created before the user ID column was added. Part of #87079 Release note: None Co-authored-by: David Hartunian <[email protected]> Co-authored-by: Andy Yang <[email protected]>

…99123 98712: storage: expose separate WriteBatch interface r=sumeerbhola a=jbowens Adapt NewUnindexedBatch to no longer take a writeOnly parameter. Instead, a new NewWriteBatch method is exposed that returns a WriteBatch type that does not provide Reader facilities. Future work may remove UnindexedBatch altogether, updating callers to explicitly maintain separate Readers and WriteBatches. Epic: None Release note: None 98807: spanconfig: add TestBlockedKVSubscriberDisablesQueues r=irfansharif a=irfansharif Regression test for #98422 which disables {split,replicate,mvccGC} queues until subscribed to span configs. We're only slightly modifying the existing merge-queue specific TestBlockedKVSubscriberDisablesMerges. Release note: None 98823: sql: address a couple of old TODOs r=yuzefovich a=yuzefovich This commit addresses a couple of old TODOs in the `connExecutor.dispatchToExecutionEngine` method. First, it simply removes the now-stale TODO about needing to `Step()` the txn before executing the query. The TODO mentioned things about the old way FK checks were performed, but we now always run checks in a deferred fashion, and perform the `Step`s correctly there for cascades and checks, thus, the TODO can be removed. (The TODO itself explicitly mentions that it can be removed once we do checks and cascades in the deferred fashion.) Second, it addresses a TODO about how some plan information is saved. Up until 961e66f the cleanup of `planNode` tree and `flow` infra was intertwined, so in 6ae4881 in order to "sample" the plan with correct info (some of which is only available _after_ the execution is done) we delegated that sampling to `planTop.close`. However, with `planNode` tree and `flow` cleanups refactored and de-coupled, we no longer have to close the plan early on. As a result, we now close the plan in a defer right after we create it (now it's the only place we do so), and this commit makes it so that we record the plan info explicitly right after returning from the execution engine, thus, addressing the second TODO. Epic: None Release note: None 98896: tracing: fix WithEventListeners option with verbose parent r=yuzefovich a=yuzefovich Previously, when using `WithEventListeners` span option and not explicitly specifying `WithRecording` option, we could incorrectly use the structured recording. In particular, this was the case when the parent has verbose recording, and this is now fixed. At the moment, this span option is used only by the backup and restore processors, so the impact would be that their verbose recording wouldn't be included into the session recording (perhaps we don't actually expose that anyway). Epic: None Release note: None 99079: sql: fix hint for unknown udfs in views and funcs r=rharding6373 a=rharding6373 Fixes a hint for some instances of an "unknown function" error that was not properly applied previously. Informs: #99002 Epic: None Release note: None 99095: ui: fix inflight request replacement, cleanup database details api r=THardy98 a=THardy98 Changes to allow in-flight request replacement in this PR: #98331 caused breaking changes to `KeyedCachedDataReducer`s. Despite the added `allowReplacementRequests` flag being `false`, in-flight requests would be replaced (specifically, when fetching for initial state). This would prevent all requests but the last one from being stored in the reducer state and leave the remaining requests in a permanent `inFlight` status. Consequently, our pages would break as the permanent `inFlight` status on these requests would prevent us from ever fetching data, putting these pages in a permanent loading state. This PR adds checks to ensure `allowReplacementRequests` is enabled when we receive a response, before deciding whether to omit it from the reducer state. It's important to note that this is still not an ideal solution for `KeyedCachedDataReducer`s, should we ever want to replace inflight requests for one. The checks to replace an in-flight request still occur at the reducer-level, whereas in a `KeyedCachedDataReducer` we'd like them to occur at the state-level (i.e. instead of replacement checks for all requests across the reducer, we should check for requests that impact a specific key's state). This way we scope replacements to each key in the reducer, allowing us to fetch data for each key independently (which is the intended functionality). Enabling this for a `KeyedCachedDataReducer` without making this change I believe will cause the breaking behaviour above. This PR also includes some cleanup to the database details API, namely: - camel casing response fields - encapsulating the database span stats response into its own object (instead of across multiple objects), this helps scope any errors deriving from the query to the single stats object **BEFORE** https://www.loom.com/share/57c9f278376a4551977398316d6ed7b6 **AFTER** https://www.loom.com/share/b2d4e074afca4b42a4f711171de3fd72 Release note (bug fix): Fixed replacement of in-flight requests for `KeyedCachedDataReducer`s to prevent permanent loading on requests stuck on an `inFlight` status. 99097: upgrades: fix backfilling of special roles in user ID migrations r=rafiss a=andyyang890 upgrades: fix system.privileges user ID migration This patch adds a missing case for the public role in the backfilling portion of the system.privileges user ID migration. Release note: None --- upgrades: fix system.database_role_settings user ID migration This patch adds a missing case for the empty role in the backfilling portion of the system.database_role_settings user ID migration. Release note: None --- Part of #87079 99113: server: use no-op cost controller for shared-process tenants r=knz a=stevendanna In the long run, all rate limiting of tenants should be controlled by a tenant capability. However, at the moment, we do not have the infrastructure to read tenant capabilities from the tenant process. Since, the main user of shared-process tenants is the new, experimental unified-architecture were we do not anticipate needing the cost controller for most users, this PR injects a no-op cost controller for shared-process tenants. Epic: CRDB-23559 Release note: None 99120: tracing: allow collection of stack history for Structured spans as well r=dt a=adityamaru Previously, we mandated that the span must be Verbose to be able to collect and emit a Structured event containing stack history. This change permits Structured spans to also collect stack history. Release note: None Epic: none 99123: sql: skip flaky TestSQLStatsDataDriven r=ericharmeling a=ericharmeling This commit skips TestSQLStatsDataDriven. This test is flaky. Part of #89861. Epic: none Release note: None Co-authored-by: Jackson Owens <[email protected]> Co-authored-by: irfan sharif <[email protected]> Co-authored-by: Yahor Yuzefovich <[email protected]> Co-authored-by: rharding6373 <[email protected]> Co-authored-by: Thomas Hardy <[email protected]> Co-authored-by: Andy Yang <[email protected]> Co-authored-by: Steven Danna <[email protected]> Co-authored-by: adityamaru <[email protected]> Co-authored-by: Eric Harmeling <[email protected]>

99533: sql,backupccl: add more tests for user ID migrations r=rafiss a=andyyang890 **sql: add mixed version test for system.privileges user ID migration** This patch adds a mixed version logic test that ensures granting of system privileges continues to work properly in a cluster that has both 22.2 and 23.1 nodes. The relevant version gate being tested here is V23_1SystemPrivilegesTableHasUserIDColumn. Release note: None --- **sql: add mixed version test for system.database_role_settings user IDs** This patch adds a mixed version logic test that ensures setting default session variables continues to work properly in a cluster that has both 22.2 and 23.1 nodes. The relevant version gate being tested here is V23_1DatabaseRoleSettingsHasRoleIDColumn. Release note: None --- **backupccl: update TestRestoreOldVersions subtest for system.privileges** This patch updates the TestRestoreOldVersions subtest for the system.privileges table to also test that a row for the public role is correctly restored. Release note: None --- Part of #87079 Part of #92342 99761: sql/pgwire: buffer messages during COPY TO and startup r=rafiss a=rafiss fixes #97314 backport fixes #99546 --- ### sql: buffer COPY OUT data Rather than sending each COPY result row one-by-one, now the data will get buffered, then flushed when the buffer size limit is reached or when sending ReadyForQuery. This fixes an issue that was causing the ruby-pg test to hang, since it assumes the data will be buffered. --- ### pgwire: buffer startup messages when creating connection This avoids sending each ParameterStatus one-by-one. --- ### sql: refactor CopyIn handling This makes it so we don't need to manually send a CommandComplete. Instead, when the CopyInResult is closed, CommandComplete will be sent, similar to how it works for other message types. Release note: None 99953: sql/schemachanger: address bugs with column families r=fqazi a=fqazi This PR addresses the following bugs with column families: 1. On master and 23.1 after the removal of oprules, we have scenarios where not implemented assertions can be hit for column families when rollbacks occur. These changes add a more concrete assertion that just ensures that the column family is cleaned up, and rules to ensure appropriate sequencing for removal. 2. When UPDATE/INSERTs were executed concurrently while adding a new column family, we could end up writing to the old primary key with the new column family. In happy path cases where everything was successful, this didn't matter, but if a rollback occurred we would have values left in the old primary index that runtime couldn't handle. 3. We had no way of validating DML with concurrent schema changes in cases with rollbacks, these modifications add tests and the framework required this case. Release note (bug fix): Concurrent DML while adding a new column with a new column family can lead to corruption in the existing primary index. If a rollback occurs the table may no longer be accessible. Co-authored-by: Andy Yang <[email protected]> Co-authored-by: Rafi Shamim <[email protected]> Co-authored-by: Faizan Qazi <[email protected]>

100017: upgrades: update system.web_sessions migration to handle orphaned rows r=rafiss a=andyyang890 This patch updates the system.web_sessions user ID migration to delete orphaned rows (i.e. rows corresponding to users that have been dropped) after backfilling user IDs. They need to be deleted since they block the addition of the NOT NULL constraint on the column. Part of #87079 Release note: None 100204: physicalplan: debugging for segfault in fakeSpanResolverIterator.Seek r=rharding6373,cucaroach a=michae2 Break up a line that is segfaulting into several lines, so that we can tell which part is to blame if it happens again. Informs: #100051 Informs: #100108 Epic: None Release note: None Co-authored-by: Andy Yang <[email protected]> Co-authored-by: Michael Erickson <[email protected]>

RichardJCai added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) labels Aug 29, 2022

RichardJCai mentioned this issue Aug 29, 2022

sql: lookup user info using stable ids #87080

Open

exalate-issue-sync bot assigned andyyang890 Oct 11, 2022

ajwerner mentioned this issue Oct 17, 2022

security, sql: User IDs For Internal User Information #78963

Closed

7 tasks

andyyang890 mentioned this issue Oct 27, 2022

server: use GRANT statement for test server auth user role grant #90800

Merged

andyyang890 mentioned this issue Nov 23, 2022

sql: add role_id and member_id columns to system.role_members table #91517

Merged

andyyang890 mentioned this issue Dec 8, 2022

sql: add user_id column to system.privileges table #93090

Merged

andyyang890 mentioned this issue Dec 9, 2022

sql,backupccl: set system.role_members ID columns to NOT NULL #93301

Merged

andyyang890 mentioned this issue Jan 30, 2023

sql: simplify user id querying for GRANT ROLE #96205

Merged

This was referenced Feb 10, 2023

sql: add user_id column to system.web_sessions table #96896

Merged

sql: add missing STORING clause in system.privileges user ID migration #97149

Merged

This was referenced Mar 16, 2023

sql,jobs: add owner_id column to system.scheduled_jobs table #98737

Draft

sql: simplify V23_1ExternalConnectionsTableHasOwnerIDColumn gating #98739

Merged

andyyang890 mentioned this issue Mar 19, 2023

sql,backupccl: set system table user ID columns to be NOT NULL #98958

Merged

andyyang890 mentioned this issue Mar 21, 2023

upgrades: fix backfilling of special roles in user ID migrations #99097

Merged

andyyang890 mentioned this issue Mar 24, 2023

sql,backupccl: add more tests for user ID migrations #99533

Merged

andyyang890 mentioned this issue Mar 30, 2023

upgrades: update system.web_sessions migration to handle orphaned rows #100017

Merged

andyyang890 mentioned this issue Mar 30, 2023

release-23.1: sql,backupccl: add more tests for user ID migrations #100032

Merged

blathers-crl bot mentioned this issue Mar 31, 2023

release-23.1: upgrades: update system.web_sessions migration to handle orphaned rows #100244

Merged

andyyang890 mentioned this issue Apr 14, 2023

sql: complete remaining user ID migrations for system tables #101512

Open

1 task

andyyang890 closed this as completed Apr 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sql, stable ids for roles: complete remaining role id migrations in system tables #87079

sql, stable ids for roles: complete remaining role id migrations in system tables #87079

RichardJCai commented Aug 29, 2022 •

edited by andyyang890

Loading

ajwerner commented Oct 16, 2022

andyyang890 commented Feb 6, 2023

rafiss commented Feb 9, 2023

sql, stable ids for roles: complete remaining role id migrations in system tables #87079

sql, stable ids for roles: complete remaining role id migrations in system tables #87079

Comments

RichardJCai commented Aug 29, 2022 • edited by andyyang890 Loading

ajwerner commented Oct 16, 2022

andyyang890 commented Feb 6, 2023

rafiss commented Feb 9, 2023

RichardJCai commented Aug 29, 2022 •

edited by andyyang890

Loading