Skip to content
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

sql: support DROP OWNED BY command #55381

Closed
solongordon opened this issue Oct 9, 2020 · 2 comments
Closed

sql: support DROP OWNED BY command #55381

solongordon opened this issue Oct 9, 2020 · 2 comments
Assignees
@jasonmchan
Labels
A-schema-changes A-sql-privileges SQL privilege handling and permission checks. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) E-starter Might be suitable for a starter project for new employees or team members. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) X-anchored-telemetry The issue number is anchored by telemetry references.

Comments

@solongordon
Copy link
Contributor

solongordon commented Oct 9, 2020 •
edited by vy-ton
Loading

To facilitate dropping roles, we should support Postgres's DROP OWNED BY command. This drops all owned objects as well as any grants on objects not owned by the role.

Relevant Postgres docs:
https://www.postgresql.org/docs/current/sql-drop-owned.html
https://www.postgresql.org/docs/current/role-removal.html

Epic CRDB-13664

Jira issue: CRDB-3673
@solongordon solongordon added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-sql-privileges SQL privilege handling and permission checks. labels Oct 9, 2020
@angelapwen angelapwen self-assigned this Oct 20, 2020
@angelapwen angelapwen mentioned this issue Oct 21, 2020
Merged
craig bot pushed a commit that referenced this issue Oct 21, 2020
Merge #55816
37188b0 
55816: sql: allow parsing of DROP OWNED BY r=otan a=angelapwen

Part 1 of addressing #55381.

Add parsing and telemetry support for DROP OWNED BY command.
Includes small refactors of the related REASSIGN OWNED BY
command such as appending "_by" to some instances of
"reassign_owned" for readability.

Plan to follow up with implementation of DROP OWNED BY for
feature parity with PostgresQL, which will resolve the issue
linked.

Note that this command is also making use of `role_spec_list` 
which as noted in #54696 does not properly parse 
`CURRENT_USER` or `SESSION_USER` and uses strings for
roles/usernames rather than expressions. This issue is out of 
scope for this PR and will be resolved separately.

Release note: None

Co-authored-by: angelapwen <angelaw@cockroachlabs.com>
@angelapwen angelapwen mentioned this issue Oct 23, 2020
Open
@angelapwen angelapwen removed their assignment Apr 21, 2021
@angelapwen
Copy link

I'm tying up loose ends as I offboard this week and want to make sure that this issue is being tracked in the new SQL Experience board. I've removed myself from assigned so others will feel free to take it on. Thank you ✨

@rafiss rafiss added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label May 12, 2021
@rafiss rafiss added the E-starter Might be suitable for a starter project for new employees or team members. label Jun 8, 2021
@blathers-crl blathers-crl bot added the T-sql-schema-deprecated Use T-sql-foundations instead label Sep 2, 2021
@rafiss rafiss added the X-anchored-telemetry The issue number is anchored by telemetry references. label Sep 2, 2021
@exalate-issue-sync exalate-issue-sync bot removed the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label Mar 2, 2022
@RichardJCai RichardJCai mentioned this issue Mar 9, 2022
Closed
jasonmchan pushed a commit to jasonmchan/cockroach that referenced this issue Jun 15, 2022
sql/schemachanger: implement DROP OWNED BY
eb377e6 
Previously, we did not support the DROP OWNED BY statement (cockroachdb#55381).
This commit adds partial support for DROP OWNED BY in the declarative
schema changer.

Followup work is needed to support checking privileges/roles in the
declarative schema changer and to support the CASCADE modifier.

Release note: None
@jasonmchan jasonmchan mentioned this issue Jun 15, 2022
Merged
jasonmchan pushed a commit to jasonmchan/cockroach that referenced this issue Jun 16, 2022
sql/schemachanger: implement DROP OWNED BY
37304b7 
Previously, we did not support the DROP OWNED BY statement (cockroachdb#55381).
This commit adds partial support for DROP OWNED BY in the declarative
schema changer. Followup work is needed to support the CASCADE modifier.

Release note (sql change): Support `DROP OWNED BY`.
jasonmchan pushed a commit to jasonmchan/cockroach that referenced this issue Jun 21, 2022
sql/schemachanger: implement DROP OWNED BY
f93df50 
Previously, we did not support the DROP OWNED BY statement (cockroachdb#55381).
This commit adds partial support for DROP OWNED BY in the declarative
schema changer. Followup work is needed to support the CASCADE modifier.

Release note (sql change): Support `DROP OWNED BY`.
jasonmchan pushed a commit to jasonmchan/cockroach that referenced this issue Jun 21, 2022
sql/schemachanger: implement DROP OWNED BY
44f2326 
Previously, we did not support the DROP OWNED BY statement (cockroachdb#55381).
This commit adds partial support for DROP OWNED BY in the declarative
schema changer. Followup work is needed to support the CASCADE modifier.

Release note (sql change): Support `DROP OWNED BY`.
jasonmchan pushed a commit to jasonmchan/cockroach that referenced this issue Jun 23, 2022
sql/schemachanger: implement DROP OWNED BY
3dac516 
Previously, we did not support the DROP OWNED BY statement (cockroachdb#55381).
This commit adds partial support for DROP OWNED BY in the declarative
schema changer. Followup work is needed to support the CASCADE modifier.

Release note (sql change): Support `DROP OWNED BY`.
jasonmchan pushed a commit to jasonmchan/cockroach that referenced this issue Jun 24, 2022
sql/schemachanger: implement DROP OWNED BY
c9edde5 
Previously, we did not support the DROP OWNED BY statement (cockroachdb#55381).
This commit adds partial support for DROP OWNED BY in the declarative
schema changer. Followup work is needed to support the CASCADE modifier.

Release note (sql change): Support `DROP OWNED BY`.
jasonmchan pushed a commit to jasonmchan/cockroach that referenced this issue Jun 28, 2022
sql/schemachanger: implement DROP OWNED BY
421856b 
Previously, we did not support the DROP OWNED BY statement (cockroachdb#55381).
This commit adds partial support for DROP OWNED BY in the declarative
schema changer. Followup work is needed to support the CASCADE modifier.

Release note (sql change): Support `DROP OWNED BY`.
craig bot pushed a commit that referenced this issue Jun 28, 2022
@adityamaru @erikgrinaker
@maryliag @ericharmeling @nvanbenschoten
Merge #82352 #82623 #82718 #82936 #83229 #83420 #83428 #83468 #83520
5541cf8 
82352: server, sql: surface session txnCount, recent txn fingerprints, active time r=xinhaoz a=xinhaoz

Finishing up Gerardo's PR, original review here: #80717

--------------------------------------------------
Partially addresses #74257.

Previously, the status server did not provide session details such as
total number of transactions executed, transaction fingerprint
IDs, and total active time. This change adds the aforementioned session
details to the `serverpb.Session` struct.

To track recently executed transaction fingerprint IDs, a FIFO cache
`TxnFingerprintIDCache` is introduced with its corresponding cluster
setting `TxnFingerprintIDBufferCapacity` to control the capacity. The
default capacity is set at 100 fingerprints.

The total number of transactions executed is filled using the existing
`txnCounter` from the `extraTxnState` in `connExecutor`. The total active
time is calculated by introducing a `timeutil.StopWatch` to the connection
executor, which is started and stopped when a transaction is started and
finished respectively.

Release note (api change): the `serverpb.Session` struct now has three
new fields: number of transactions executed, transaction fingerprint
IDs, and total active time.

82623: backupinfo: introduce a backupinfo package r=stevendanna a=adityamaru

The backupinfo package contains logic related to interacting
with information and metadata describing the backup. After this
change we have `backupdest` depending on `backupinfo`.

Release note: None

82718: kvserver: emit MVCC range tombstones over rangefeeds r=aliher1911 a=erikgrinaker

This patch adds MVCC range tombstone support in rangefeeds. Whenever an
MVCC range tombstone is written, a new `MVCCDeleteRangeOp` logical op
is recorded and emitted across the rangefeed as a `RangeFeedDeleteRange`
event. MVCC range tombstones will only be written when the
`MVCCRangeTombstones` version gate has been enabled.

Changefeeds will emit an error for these events. We do not expect to see
these in online spans with changefeeds, since they are initially only
planned for use with schema GC and import rollbacks.

The rangefeed client library has been extended with support for these
events, but no existing callers handle them for the same reason as
changefeeds. Initial scans do not emit regular tombstones, and thus not
range tombstones either, but catchup scans will emit them if
encountered.

This patch has rudimentary testing of MVCC range tombstones in
rangefeeds. A later patch will add a data-driven test harness for
rangefeeds with more exhaustive tests.

Resolves #82449.
Touches #70433.

Release note: None

82936: sql/schemachanger: implement DROP OWNED BY r=jasonmchan a=jasonmchan

Previously, we did not support the DROP OWNED BY statement (#55381).
This commit adds partial support for DROP OWNED BY in the declarative
schema changer. Followup work is needed to support the CASCADE modifier.

Release note (sql change): Support `DROP OWNED BY`.

83229: ui: remove option 10/30 min from SQL Activity page r=maryliag a=maryliag

Note to reviewers: only 2nd commit is relevant to this PR

Previously we had the options for 10 and 30min on
SQL Activity pages, which created some confusion, since
we would always show the last 1h info.
This commit remove those 2 options.
If the user select any of those options on the Metrics
page, it will get updated to 1h on the SQL Activity
pages.

<img width="444" alt="Screen Shot 2022-06-22 at 5 43 53 PM" src="https://user-images.githubusercontent.com/1017486/175144243-2f084e0b-5e09-4874-9640-e7eea6179343.png">

https://www.loom.com/share/226e54322df6456aa2039b5c54f72eb1


Fixes #82914

Release note (ui change): Removal of the 10 and 30min options
on the SQL Activity page.

83420: ui: improve tooltip UX with text updates r=ericharmeling a=ericharmeling

Fixes #81374.
Fixes #83256.
Fixes #81248.
Fixes #79018.

Note the following:

- The updates resolving #79018 effectively revert the tooltip text for Rows Read to the original wording (which [was updated for accuracy](e379e9d#diff-492398441e971e355a687a4ce333a9766e2195287d0227682444d5dc0eb7ee1a)). I assume this is okay. `@kevin-v-ngo`
- The updates resolving #81248 do not in fact refer to the time intervals as date ranges, as this language is misleading (a 1h interval is an interval and not a date range). Instead, this update just removes the anchor and the link to the non-existent Interval Range section of https://www.cockroachlabs.com/docs/stable/ui-statements-page.html. We may want to consider updating the docs to call the "time picker" data type a time interval and not a date range. This appears to have been the case in previous releases (https://www.cockroachlabs.com/docs/v21.1/ui-statements-page#time-interval). `@stbof` 

Release note (ui change): Updated tooltips on the Statements and
Transactions pages in the DB Console for improved UX.

83428: sql: rename anonymizedStmt in sqlstats pkg to stmtNoConstants r=ericharmeling a=ericharmeling

Note that this commit does not change any files outside the sqlstats package.

Fixes #80725.

Release note: None

83468: ui: update all dates to use same format r=maryliag a=maryliag

Update all dates to use the same format.

Fixes #81159

Release note: None

83520: kv: don't try to reject lease transfer when flushing proposal buffer r=nvanbenschoten a=nvanbenschoten

Fixes #83498.
Fixes #83402.
Fixes #83308.

This was fallout from #82758.

This commit adds logic to `propBuf.maybeRejectUnsafeProposalLocked` to avoid
trying to reject proposals based on the state of the raft group when the group
is not provided (e.g. when flushing the buffer). We already had this logic for
`RequestLease` (indirectly), but did not for `TransferLease`.

Co-authored-by: Gerardo Torres <gerardo.torrescastro@cockroachlabs.com>
Co-authored-by: Aditya Maru <adityamaru@gmail.com>
Co-authored-by: Erik Grinaker <grinaker@cockroachlabs.com>
Co-authored-by: Jason Chan <jason.chan@cockroachlabs.com>
Co-authored-by: Marylia Gutierrez <marylia@cockroachlabs.com>
Co-authored-by: Eric Harmeling <eric.harmeling@cockroachlabs.com>
Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
@postamar
Copy link
Contributor

postamar commented Jul 8, 2022 •
edited
Loading

Addressed by #82936

@postamar postamar closed this as completed Jul 8, 2022
@exalate-issue-sync exalate-issue-sync bot added T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) and removed T-sql-schema-deprecated Use T-sql-foundations instead labels May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jasonmchan jasonmchan

Labels
A-schema-changes A-sql-privileges SQL privilege handling and permission checks. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) E-starter Might be suitable for a starter project for new employees or team members. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) X-anchored-telemetry The issue number is anchored by telemetry references.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@solongordon @rafiss @postamar @angelapwen @ZhouXing19 @jasonmchan