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.

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

Granting BACKUP to all tables causes stack trace if there's a sequence #117861

Closed
smcvey opened this issue Jan 17, 2024 · 0 comments · Fixed by #120685
Closed

Granting BACKUP to all tables causes stack trace if there's a sequence #117861

smcvey opened this issue Jan 17, 2024 · 0 comments · Fixed by #120685
Assignees
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-support Originated from a customer P-2 Issues/test failures with a fix SLA of 3 months T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@smcvey
Copy link
Contributor

smcvey commented Jan 17, 2024

Describe the problem

Attempting to grant BACKUP to all tables causes a stack trace if there are sequences.

To Reproduce

New cluster:

CREATE SEQUENCE s;
GRANT BACKUP ON ALL TABLES IN SCHEMA public TO <user>;

Creates this output:

ERROR: internal error: table descriptor is not valid
tabledesc.Mutable: {ID: 104, Version: 1, ModificationTime: "1705494412.925153755,0", ParentID: 100, ParentSchemaID: 101, State: PUBLIC, Sequence: true, NextColumnID: 0, Columns: [{ID: 1, TypeID: 20, Null: false}], NextFamilyID: 0, Families: [{ID: 0, Columns: [1]}], PrimaryIndex: 1, NextIndexID: 0, Indexes: [{ID: 1, Unique: false, KeyColumns: [{ID: 1, Dir: ASC}]}]}
: relation "s" (104): user nobody must not have [BACKUP] privileges on sequence "s"
SQLSTATE: XX000
DETAIL: stack trace:
github.com/cockroachdb/cockroach/pkg/sql/table.go:337: writeTableDescToBatch()
github.com/cockroachdb/cockroach/pkg/sql/table.go:297: writeSchemaChangeToBatch()
github.com/cockroachdb/cockroach/pkg/sql/grant_revoke.go:372: startExec()
github.com/cockroachdb/cockroach/pkg/sql/plan.go:520: func2()
github.com/cockroachdb/cockroach/pkg/sql/walk.go:112: func1()
github.com/cockroachdb/cockroach/pkg/sql/walk.go:299: visitInternal()
github.com/cockroachdb/cockroach/pkg/sql/walk.go:79: visit()
github.com/cockroachdb/cockroach/pkg/sql/walk.go:43: walkPlan()
github.com/cockroachdb/cockroach/pkg/sql/plan.go:523: startExec()
github.com/cockroachdb/cockroach/pkg/sql/plan_node_to_row_source.go:175: Start()
github.com/cockroachdb/cockroach/pkg/sql/colexec/columnarizer.go:183: Init()
github.com/cockroachdb/cockroach/pkg/sql/colflow/stats.go:94: init()
github.com/cockroachdb/cockroach/pkg/sql/colexecerror/error.go:92: CatchVectorizedRuntimeError()
github.com/cockroachdb/cockroach/pkg/sql/colflow/stats.go:103: Init()
github.com/cockroachdb/cockroach/pkg/sql/colflow/flow_coordinator.go:245: func2()
github.com/cockroachdb/cockroach/pkg/sql/colexecerror/error.go:92: CatchVectorizedRuntimeError()
github.com/cockroachdb/cockroach/pkg/sql/colflow/flow_coordinator.go:244: init()
github.com/cockroachdb/cockroach/pkg/sql/colflow/flow_coordinator.go:278: Run()
github.com/cockroachdb/cockroach/pkg/sql/colflow/vectorized_flow.go:320: Run()
github.com/cockroachdb/cockroach/pkg/sql/distsql_running.go:906: Run()
github.com/cockroachdb/cockroach/pkg/sql/distsql_running.go:1946: PlanAndRun()
github.com/cockroachdb/cockroach/pkg/sql/distsql_running.go:1661: func3()
github.com/cockroachdb/cockroach/pkg/sql/distsql_running.go:1664: PlanAndRunAll()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:2342: execWithDistSQLEngine()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:1882: dispatchToExecutionEngine()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:1104: execStmtInOpenState()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:145: func1()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:3317: execWithProfiling()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:144: execStmt()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:2228: func1()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:2233: execCmd()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:2150: run()

HINT: You have encountered an unexpected error.

Expected behavior
When applying with the ALL TABLES, it should just ignore sequences completely (considering that attempting to apply BACKUP directly to a sequence isn't allowed).

Environment:

  • CockroachDB version: 23.2.0-rc.2

Jira issue: CRDB-35387

@smcvey smcvey added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Jan 17, 2024
@rafiss rafiss added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label Jan 17, 2024
@blathers-crl blathers-crl bot added this to Triage in SQL Foundations Jan 17, 2024
@rafiss rafiss added O-support Originated from a customer P-2 Issues/test failures with a fix SLA of 3 months labels Jan 17, 2024
fqazi added a commit to fqazi/cockroach that referenced this issue Mar 19, 2024
Previously, grant privilege to all tables attempted to apply privileges
meant only for tables on to sequences. This could lead to validation
error preventing certain combinations like "GRANT BACKUP ON ALL
TABLES..." from working correctly. To address this, this patch adds
support for correctly propogating the object type and skipping over
unsupported privileges. When an unsupported privilege is encountered on
an object other then a sequence a warning is now logged.

Fixes: cockroachdb#117861

Release note (bug fix): GRANT <...> ON ALL TABLES could fail if
sequences existed and they did not support a privilege (for example
BACKUP).
craig bot pushed a commit that referenced this issue Mar 19, 2024
120672: workload/schemachange: fix expected errors for enum ref in UDF r=rafiss a=rafiss

The code was conflating enums with enum members. Even if an enum member is being dropped, there's no problem with referencing that enum type when creating a UDF.

Now these cases are distinguished.

fixes #120614
Release note: None

120685: sql: grant to all tables did not skip privileges on sequences r=fqazi a=fqazi

Previously, grant privilege to all tables attempted to apply privileges meant only for tables on to sequences. This could lead to validation error preventing certain combinations like "GRANT BACKUP ON ALL TABLES..." from working correctly. To address this, this patch adds support for correctly propogating the object type and skipping over unsupported privileges. When an unsupported privilege is encountered on an object other then a sequence a warning is now logged.

Fixes: #117861

Release note (bug fix): GRANT <...> ON ALL TABLES could fail if sequences existed and they did not support a privilege (for example BACKUP).

120697: drt: delete paused IMPORTs before init in tpcc_drop r=itsbilal a=ajstorm

In the cct_tpcc_drop workload, we want to remove any paused IMPORTs before we try and re-init the database. Paused IMPORTs leave the affected tables offline, thus preventing init from succeeding. We're left with paused IMPORTs on occasion due to the fact that when running chaos, node failures could prevent IMPORT from making sufficient progress in the necessary time to be retried.

Release note: none
Epic: none

Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com>
Co-authored-by: Faizan Qazi <faizan@cockroachlabs.com>
Co-authored-by: Adam Storm <storm@cockroachlabs.com>
@craig craig bot closed this as completed in 007005f Mar 19, 2024
SQL Foundations automation moved this from Triage to Done Mar 19, 2024
blathers-crl bot pushed a commit that referenced this issue Apr 9, 2024
Previously, grant privilege to all tables attempted to apply privileges
meant only for tables on to sequences. This could lead to validation
error preventing certain combinations like "GRANT BACKUP ON ALL
TABLES..." from working correctly. To address this, this patch adds
support for correctly propogating the object type and skipping over
unsupported privileges. When an unsupported privilege is encountered on
an object other then a sequence a warning is now logged.

Fixes: #117861

Release note (bug fix): GRANT <...> ON ALL TABLES could fail if
sequences existed and they did not support a privilege (for example
BACKUP).
fqazi added a commit to fqazi/cockroach that referenced this issue Apr 9, 2024
Previously, grant privilege to all tables attempted to apply privileges
meant only for tables on to sequences. This could lead to validation
error preventing certain combinations like "GRANT BACKUP ON ALL
TABLES..." from working correctly. To address this, this patch adds
support for correctly propogating the object type and skipping over
unsupported privileges. When an unsupported privilege is encountered on
an object other then a sequence a warning is now logged.

Fixes: cockroachdb#117861

Release note (bug fix): GRANT <...> ON ALL TABLES could fail if
sequences existed and they did not support a privilege (for example
BACKUP).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-support Originated from a customer P-2 Issues/test failures with a fix SLA of 3 months T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
Development

Successfully merging a pull request may close this issue.

3 participants