Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
134604: backupccl: Fix MR enum in non-MR temp sys DB r=dt a=dt

The temp db doesn't need to be MR just for us to copy stuff out of it into the real one.

Release note (bug fix): Fix a bug that prevented restoring cluster backup taken in an MR cluster that had configured a the system database with a region configuration into a non-multi-region cluster.
Epic: CRDB-42475.

135099: docgen: surface SECURITY DEFINER/INVOKER syntax for CREATE PROC r=annrpom a=taroface

Made an update to the CREATE PROCEDURE diagram that adds the [ EXTERNAL ] SECURITY DEFINER/INVOKER clause (relates to #129720):

<img width="869" alt="image" src="https://github.com/user-attachments/assets/a81a735d-dcef-49c8-82db-a30f6fe5e428">

However, I can't currently figure out how to surface this syntax for ALTER PROCEDURE. `@annrpom` , can you let me know if it's necessary for me to write it in manually?

Epic: none
Release note: none
Release justification: non-production code change

Co-authored-by: David Taylor <[email protected]>
Co-authored-by: Ryan Kuo <[email protected]>
  • Loading branch information
3 people committed Nov 13, 2024
3 parents 1759878 + f041746 + 466cbf9 commit 409ba28
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 17 deletions.
2 changes: 1 addition & 1 deletion docs/generated/sql/bnf/create_proc.bnf
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
create_proc_stmt ::=
'CREATE' ( 'OR' 'REPLACE' | ) 'PROCEDURE' routine_create_name '(' ( ( ( ( routine_param | routine_param | routine_param ) ) ( ( ',' ( routine_param | routine_param | routine_param ) ) )* ) | ) ')' ( ( ( ( 'AS' routine_body_str | 'LANGUAGE' ( 'SQL' | 'PLPGSQL' ) | ) ) ( ( ( 'AS' routine_body_str | 'LANGUAGE' ( 'SQL' | 'PLPGSQL' ) | ) ) )* ) | )
'CREATE' ( 'OR' 'REPLACE' | ) 'PROCEDURE' routine_create_name '(' ( ( ( ( routine_param | routine_param | routine_param ) ) ( ( ',' ( routine_param | routine_param | routine_param ) ) )* ) | ) ')' ( ( ( ( 'AS' routine_body_str | 'LANGUAGE' ( 'SQL' | 'PLPGSQL' ) | ( 'EXTERNAL' 'SECURITY' 'DEFINER' | 'EXTERNAL' 'SECURITY' 'INVOKER' | 'SECURITY' 'DEFINER' | 'SECURITY' 'INVOKER' ) ) ) ( ( ( 'AS' routine_body_str | 'LANGUAGE' ( 'SQL' | 'PLPGSQL' ) | ( 'EXTERNAL' 'SECURITY' 'DEFINER' | 'EXTERNAL' 'SECURITY' 'INVOKER' | 'SECURITY' 'DEFINER' | 'SECURITY' 'INVOKER' ) ) ) )* ) | )
8 changes: 8 additions & 0 deletions pkg/ccl/backupccl/restore_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -1183,6 +1183,7 @@ func createImportingDescriptors(
for _, desc := range mutableTables {
if desc.GetParentID() == tempSystemDBID {
desc.SetPublic()
desc.LocalityConfig = nil
}
}
}
Expand Down Expand Up @@ -1250,6 +1251,13 @@ func createImportingDescriptors(

if db, ok := dbsByID[regionTypeDesc.GetParentID()]; ok {
desc := db.DatabaseDesc()
if db.GetName() == restoreTempSystemDB {
t.TypeDesc().Kind = descpb.TypeDescriptor_ENUM
t.TypeDesc().RegionConfig = nil
// TODO(foundations): should these be rewritten instead of blank? Does it matter since we drop the whole DB before the job exits?
t.TypeDesc().ReferencingDescriptorIDs = nil
continue
}
if desc.RegionConfig == nil {
return errors.AssertionFailedf(
"found MULTIREGION_ENUM on non-multi-region database %s", desc.Name)
Expand Down
4 changes: 4 additions & 0 deletions pkg/ccl/backupccl/testdata/backup-restore/multiregion
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@ skip-under-duress
new-cluster name=s1 allow-implicit-access disable-tenant localities=us-east-1,us-west-1,eu-central-1
----

set-cluster-setting setting=sql.multiregion.system_database_multiregion.enabled value=true
----

exec-sql
ALTER DATABASE system SET PRIMARY REGION "us-east-1";
CREATE DATABASE d PRIMARY REGION "us-east-1" REGIONS "us-west-1", "eu-central-1" SURVIVE REGION FAILURE;
CREATE TABLE d.t (x INT);
INSERT INTO d.t VALUES (1), (2), (3);
Expand Down
36 changes: 20 additions & 16 deletions pkg/cmd/docgen/diagrams.go
Original file line number Diff line number Diff line change
Expand Up @@ -511,11 +511,14 @@ var specs = []stmtSpec{
nosplit: true,
},
{
name: "alter_proc",
stmt: "alter_proc_stmt",
inline: []string{"alter_proc_rename_stmt", "alter_proc_owner_stmt", "alter_proc_set_schema_stmt", "function_with_paramtypes", "func_params", "func_params_list"},
unlink: []string{"proc_name", "proc_new_name"},
replace: map[string]string{"db_object_name": "proc_name", "'RENAME' 'TO' name": "'RENAME' 'TO' proc_new_name"},
name: "alter_proc",
stmt: "alter_proc_stmt",
inline: []string{"alter_proc_rename_stmt", "alter_proc_owner_stmt", "alter_proc_set_schema_stmt", "function_with_paramtypes", "func_params", "func_params_list"},
unlink: []string{"proc_name", "proc_new_name"},
replace: map[string]string{
"db_object_name": "proc_name",
"'RENAME' 'TO' name": "'RENAME' 'TO' proc_new_name",
},
nosplit: true,
},
{
Expand Down Expand Up @@ -843,19 +846,20 @@ var specs = []stmtSpec{
{
name: "create_proc",
stmt: "create_proc_stmt",
inline: []string{"opt_or_replace", "opt_routine_param_with_default_list", "routine_param_with_default_list", "routine_param_with_default", "opt_create_routine_opt_list", "create_routine_opt_list", "create_routine_opt_item", "routine_as", "opt_link_sym", "create_routine_opt_item", "routine_return_stmt"},
inline: []string{"opt_or_replace", "opt_routine_param_with_default_list", "routine_param_with_default_list", "routine_param_with_default", "opt_create_routine_opt_list", "common_routine_opt_item", "create_routine_opt_list", "create_routine_opt_item", "routine_as", "opt_link_sym", "create_routine_opt_item", "routine_return_stmt"},
unlink: []string{"routine_body_str"},
replace: map[string]string{
"'DEFAULT'": "",
"common_routine_opt_item": "",
"'AS'": "'AS' routine_body_str",
"opt_routine_body": "",
"non_reserved_word_or_sconst": "( 'SQL' | 'PLPGSQL' )",
"'RETURN'": "",
"( 'SCONST' ) ( ',' 'SCONST' | )": "",
"'='": "",
"a_expr": "",
"'ATOMIC'": ""},
"'DEFAULT'": "",
"'AS'": "'AS' routine_body_str",
"opt_routine_body": "",
"'CALLED' 'ON' 'NULL' 'INPUT' | 'RETURNS' 'NULL' 'ON' 'NULL' 'INPUT' | 'STRICT' | 'IMMUTABLE' | 'STABLE' | 'VOLATILE' |": "",
"| 'LEAKPROOF' | 'NOT' 'LEAKPROOF'": "",
"non_reserved_word_or_sconst": "( 'SQL' | 'PLPGSQL' )",
"'RETURN'": "",
"( 'SCONST' ) ( ',' 'SCONST' | )": "",
"'='": "",
"a_expr": "",
"'ATOMIC'": ""},
nosplit: true,
},
{
Expand Down

0 comments on commit 409ba28

Please sign in to comment.