From 483e4d9e065f96bfc07e5bdd30eade5988e7deed Mon Sep 17 00:00:00 2001 From: richardjcai Date: Wed, 15 Dec 2021 16:08:05 -0500 Subject: [PATCH 1/4] restoreccl: insert system.namespace entry for synthetic public schemas during restore Release note (bug fix): Previously during restore we wouldn't insert a system.namespace entry for synthetic public schemas. --- pkg/ccl/backupccl/restore_job.go | 6 +++ .../backupccl/restore_old_versions_test.go | 42 +++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/pkg/ccl/backupccl/restore_job.go b/pkg/ccl/backupccl/restore_job.go index 73fefeab8fce..8d4f7f52239d 100644 --- a/pkg/ccl/backupccl/restore_job.go +++ b/pkg/ccl/backupccl/restore_job.go @@ -374,6 +374,12 @@ func WriteDescriptors( return err } b.CPut(catalogkeys.EncodeNameKey(codec, desc), desc.GetID(), nil) + + // We also have to put a system.namespace entry for the public schema + // if the database does not have a public schema backed by a descriptor. + if !desc.HasPublicSchemaWithDescriptor() { + b.CPut(catalogkeys.MakeSchemaNameKey(codec, desc.GetID(), tree.PublicSchema), keys.PublicSchemaID, nil) + } } // Write namespace and descriptor entries for each schema. diff --git a/pkg/ccl/backupccl/restore_old_versions_test.go b/pkg/ccl/backupccl/restore_old_versions_test.go index eb28e7f4c93f..36dbf9882f82 100644 --- a/pkg/ccl/backupccl/restore_old_versions_test.go +++ b/pkg/ccl/backupccl/restore_old_versions_test.go @@ -265,6 +265,17 @@ ORDER BY object_type, object_name`, [][]string{ t.Run(dir.Name(), restorePublicSchemaMixedVersion(exportDir)) } }) + + t.Run("missing_public_schema_namespace_entry", func(t *testing.T) { + dirs, err := ioutil.ReadDir(publicSchemaDirs) + require.NoError(t, err) + for _, dir := range dirs { + require.True(t, dir.IsDir()) + exportDir, err := filepath.Abs(filepath.Join(publicSchemaDirs, dir.Name())) + require.NoError(t, err) + t.Run(dir.Name(), restoreSyntheticPublicSchemaNamespaceEntry(exportDir)) + } + }) } func restoreOldVersionTestWithInterleave(exportDir string) func(t *testing.T) { @@ -878,3 +889,34 @@ func restorePublicSchemaMixedVersion(exportDir string) func(t *testing.T) { sqlDB.CheckQueryResults(t, `SELECT x FROM test.public.t`, [][]string{{"3"}, {"4"}}) } } + +func restoreSyntheticPublicSchemaNamespaceEntry(exportDir string) func(t *testing.T) { + return func(t *testing.T) { + const numAccounts = 1000 + _, _, _, tmpDir, cleanupFn := BackupRestoreTestSetup(t, MultiNode, numAccounts, InitManualReplication) + defer cleanupFn() + + _, _, sqlDB, cleanup := backupRestoreTestSetupEmpty(t, singleNode, tmpDir, + InitManualReplication, base.TestClusterArgs{ + ServerArgs: base.TestServerArgs{ + Knobs: base.TestingKnobs{ + JobsTestingKnobs: jobs.NewTestingKnobsWithShortIntervals(), + Server: &server.TestingKnobs{ + DisableAutomaticVersionUpgrade: 1, + BinaryVersionOverride: clusterversion.ByKey(clusterversion.PublicSchemasWithDescriptors - 1), + }, + }, + }}) + defer cleanup() + err := os.Symlink(exportDir, filepath.Join(tmpDir, "foo")) + require.NoError(t, err) + + sqlDB.Exec(t, fmt.Sprintf("RESTORE DATABASE d FROM '%s'", LocalFoo)) + + var dbID int + row := sqlDB.QueryRow(t, `SELECT id FROM system.namespace WHERE name = 'd'`) + row.Scan(&dbID) + + sqlDB.CheckQueryResults(t, fmt.Sprintf(`SELECT id FROM system.namespace WHERE name = 'public' AND "parentID"=%d`, dbID), [][]string{{"29"}}) + } +} From 66ce0ce55e69cc4cb1afb22bc4abd992b9d915bc Mon Sep 17 00:00:00 2001 From: richardjcai Date: Thu, 16 Dec 2021 16:05:12 -0500 Subject: [PATCH 2/4] sql: simplify inserts into database descs with empty schemas map Release note: None --- pkg/ccl/backupccl/restore_job.go | 15 ++------------- pkg/ccl/importccl/import_job.go | 13 +++---------- pkg/sql/catalog/dbdesc/database_desc.go | 14 ++++++++++++++ pkg/sql/create_schema.go | 5 +---- pkg/sql/drop_schema.go | 7 ++----- pkg/sql/reparent_database.go | 5 +---- 6 files changed, 23 insertions(+), 36 deletions(-) diff --git a/pkg/ccl/backupccl/restore_job.go b/pkg/ccl/backupccl/restore_job.go index a8b71c54a67b..3b4ddad1570c 100644 --- a/pkg/ccl/backupccl/restore_job.go +++ b/pkg/ccl/backupccl/restore_job.go @@ -1322,11 +1322,8 @@ func createImportingDescriptors( return err } db := desc.(*dbdesc.Mutable) - if db.Schemas == nil { - db.Schemas = make(map[string]descpb.DatabaseDescriptor_SchemaInfo) - } for _, sc := range schemas { - db.Schemas[sc.GetName()] = descpb.DatabaseDescriptor_SchemaInfo{ID: sc.GetID()} + db.AddSchemaToDatabase(sc.GetName(), descpb.DatabaseDescriptor_SchemaInfo{ID: sc.GetID()}) } if err := descsCol.WriteDescToBatch( ctx, false /* kvTrace */, db, b, @@ -1551,15 +1548,7 @@ func remapPublicSchemas( return err } - if db.Schemas == nil { - db.Schemas = map[string]descpb.DatabaseDescriptor_SchemaInfo{ - tree.PublicSchema: { - ID: id, - }, - } - } else { - db.Schemas[tree.PublicSchema] = descpb.DatabaseDescriptor_SchemaInfo{ID: id} - } + db.AddSchemaToDatabase(tree.PublicSchema, descpb.DatabaseDescriptor_SchemaInfo{ID: id}) // Every database must be initialized with the public schema. // Create the SchemaDescriptor. // In postgres, the user "postgres" is the owner of the public schema in a diff --git a/pkg/ccl/importccl/import_job.go b/pkg/ccl/importccl/import_job.go index 95a9fed65153..03932f0a635c 100644 --- a/pkg/ccl/importccl/import_job.go +++ b/pkg/ccl/importccl/import_job.go @@ -600,10 +600,6 @@ func (r *importResumer) prepareSchemasForIngestion( details.ParentID) } - if dbDesc.Schemas == nil { - dbDesc.Schemas = make(map[string]descpb.DatabaseDescriptor_SchemaInfo) - } - schemaMetadata.schemaRewrites = make(backupccl.DescRewriteMap) mutableSchemaDescs := make([]*schemadesc.Mutable, 0) for _, desc := range details.Schemas { @@ -624,8 +620,8 @@ func (r *importResumer) prepareSchemasForIngestion( schemaMetadata.newSchemaIDToName[id] = newMutableSchemaDescriptor.GetName() // Update the parent database with this schema information. - dbDesc.Schemas[newMutableSchemaDescriptor.Name] = - descpb.DatabaseDescriptor_SchemaInfo{ID: newMutableSchemaDescriptor.ID} + dbDesc.AddSchemaToDatabase(newMutableSchemaDescriptor.Name, + descpb.DatabaseDescriptor_SchemaInfo{ID: newMutableSchemaDescriptor.ID}) schemaMetadata.schemaRewrites[desc.Desc.ID] = &jobspb.RestoreDetails_DescriptorRewrite{ ID: id, @@ -1562,10 +1558,7 @@ func (r *importResumer) dropSchemas( Name: schemaDesc.Name, }) // Update the parent database with information about the dropped schema. - if dbDesc.Schemas == nil { - dbDesc.Schemas = make(map[string]descpb.DatabaseDescriptor_SchemaInfo) - } - dbDesc.Schemas[schema.Desc.Name] = descpb.DatabaseDescriptor_SchemaInfo{ID: dbDesc.ID, Dropped: true} + dbDesc.AddSchemaToDatabase(schema.Desc.Name, descpb.DatabaseDescriptor_SchemaInfo{ID: dbDesc.ID, Dropped: true}) } if err := descsCol.WriteDescToBatch(ctx, p.ExtendedEvalContext().Tracing.KVTracingEnabled(), diff --git a/pkg/sql/catalog/dbdesc/database_desc.go b/pkg/sql/catalog/dbdesc/database_desc.go index e6e90a663cbb..fe4ee7c0c224 100644 --- a/pkg/sql/catalog/dbdesc/database_desc.go +++ b/pkg/sql/catalog/dbdesc/database_desc.go @@ -478,6 +478,20 @@ func (desc *Mutable) SetDefaultPrivilegeDescriptor( desc.DefaultPrivileges = defaultPrivilegeDescriptor } +// AddSchemaToDatabase adds a schemaName and schemaInfo entry into the +// database's Schemas map. If the map is nil, then we create a map before +// adding the entry. +// If there is an existing entry in the map with schemaName as the key, +// it will be overridden. +func (desc *Mutable) AddSchemaToDatabase( + schemaName string, schemaInfo descpb.DatabaseDescriptor_SchemaInfo, +) { + if desc.Schemas == nil { + desc.Schemas = make(map[string]descpb.DatabaseDescriptor_SchemaInfo) + } + desc.Schemas[schemaName] = schemaInfo +} + // maybeRemoveDroppedSelfEntryFromSchemas removes an entry in the Schemas map corresponding to the // database itself which was added due to a bug in prior versions when dropping any user-defined schema. // The bug inserted an entry for the database rather than the schema being dropped. This function fixes the diff --git a/pkg/sql/create_schema.go b/pkg/sql/create_schema.go index 91a66f231a8a..c268438913eb 100644 --- a/pkg/sql/create_schema.go +++ b/pkg/sql/create_schema.go @@ -195,10 +195,7 @@ func (p *planner) createUserDefinedSchema(params runParams, n *tree.CreateSchema } // Update the parent database with this schema information. - if db.Schemas == nil { - db.Schemas = make(map[string]descpb.DatabaseDescriptor_SchemaInfo) - } - db.Schemas[desc.Name] = descpb.DatabaseDescriptor_SchemaInfo{ID: desc.ID} + db.AddSchemaToDatabase(desc.Name, descpb.DatabaseDescriptor_SchemaInfo{ID: desc.ID}) if err := p.writeNonDropDatabaseChange( params.ctx, db, diff --git a/pkg/sql/drop_schema.go b/pkg/sql/drop_schema.go index dd08d94b9c10..0708f2d86bc4 100644 --- a/pkg/sql/drop_schema.go +++ b/pkg/sql/drop_schema.go @@ -211,13 +211,10 @@ func (p *planner) dropSchemaImpl( } else { // TODO (rohany): This can be removed once RESTORE installs schemas into // the parent database. - if parentDB.Schemas == nil { - parentDB.Schemas = make(map[string]descpb.DatabaseDescriptor_SchemaInfo) - } - parentDB.Schemas[sc.GetName()] = descpb.DatabaseDescriptor_SchemaInfo{ + parentDB.AddSchemaToDatabase(sc.GetName(), descpb.DatabaseDescriptor_SchemaInfo{ ID: sc.GetID(), Dropped: true, - } + }) } // Update the schema descriptor as dropped. diff --git a/pkg/sql/reparent_database.go b/pkg/sql/reparent_database.go index 88b311909b69..122a03ce6e8d 100644 --- a/pkg/sql/reparent_database.go +++ b/pkg/sql/reparent_database.go @@ -130,10 +130,7 @@ func (n *reparentDatabaseNode) startExec(params runParams) error { Version: 1, }).BuildCreatedMutable() // Add the new schema to the parent database's name map. - if n.newParent.Schemas == nil { - n.newParent.Schemas = make(map[string]descpb.DatabaseDescriptor_SchemaInfo) - } - n.newParent.Schemas[n.db.Name] = descpb.DatabaseDescriptor_SchemaInfo{ID: schema.GetID()} + n.newParent.AddSchemaToDatabase(n.db.Name, descpb.DatabaseDescriptor_SchemaInfo{ID: schema.GetID()}) if err := p.createDescriptorWithID( ctx, From 1f9ad4585ee5ed96eb625b1ed948d37eb1906b9c Mon Sep 17 00:00:00 2001 From: Nick Vigilante Date: Thu, 16 Dec 2021 18:00:56 -0500 Subject: [PATCH 3/4] DOC-2096: Update diagrams.go to force scrolling on svg div blocks Release note: None --- pkg/cmd/docgen/diagrams.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/docgen/diagrams.go b/pkg/cmd/docgen/diagrams.go index 8e0f9845b110..afd30afb7399 100644 --- a/pkg/cmd/docgen/diagrams.go +++ b/pkg/cmd/docgen/diagrams.go @@ -243,7 +243,7 @@ func init() { // Wrap the output in a
so that the Markdown parser // doesn't attempt to parse the inside of the contained // as Markdown. - body = fmt.Sprintf(`
%s
`, body) + body = fmt.Sprintf(`
%s
`, body) // Remove blank lines and strip spaces. body = stripRE.ReplaceAllString(body, "\n") + "\n" } From ab09ae12390b3d58242d6cd5ec8d937107306160 Mon Sep 17 00:00:00 2001 From: Ricky Stewart Date: Fri, 17 Dec 2021 11:19:48 -0600 Subject: [PATCH 4/4] bazel: expose `devdarwinx86_64` toolchain via `platform` rules This becomes necessary after #73819, which sets `--incompatible_enable_cc_toolchain_resolution` for builds. The problem is that this flag causes Bazel to ignore `--crosstool_top` in favor of what toolchain it selects via [toolchain resolution](https://docs.bazel.build/versions/main/toolchains.html#toolchain-resolution). Therefore we update `.bazelrc` to specify the `--platforms` instead of `--crosstool_top`, and add an appropriate `toolchain` in `build/toolchains`. Release note: None --- .bazelrc | 2 +- WORKSPACE | 1 + build/toolchains/BUILD.bazel | 25 +++++++++++++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/.bazelrc b/.bazelrc index e8828ee87518..7ba7162770b3 100644 --- a/.bazelrc +++ b/.bazelrc @@ -37,7 +37,7 @@ build:crosslinuxarm '--workspace_status_command=./build/bazelutil/stamp.sh aarch build:crosslinuxarm --config=cross # developer configurations. Add e.g. --config=devdarwinx86_64 to turn these on. -build:devdarwinx86_64 --crosstool_top=@toolchain_dev_darwin_x86-64//:suite +build:devdarwinx86_64 --platforms=//build/toolchains:darwin_x86_64 # NOTE(ricky): This is consumed in `BUILD` files (see # `build/toolchains/BUILD.bazel`). build:devdarwinx86_64 --config=dev diff --git a/WORKSPACE b/WORKSPACE index 99aa1a22ceed..da7802dc2e40 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -370,6 +370,7 @@ register_toolchains( "//build/toolchains:cross_linux_arm_toolchain", "//build/toolchains:cross_macos_toolchain", "//build/toolchains:cross_windows_toolchain", + "//build/toolchains:dev_darwin_x86_64_toolchain", ) http_archive( diff --git a/build/toolchains/BUILD.bazel b/build/toolchains/BUILD.bazel index 89c0af2d99ca..eb7b0673e23f 100644 --- a/build/toolchains/BUILD.bazel +++ b/build/toolchains/BUILD.bazel @@ -86,6 +86,31 @@ platform( ], ) +toolchain( + name = "dev_darwin_x86_64_toolchain", + exec_compatible_with = [ + "@platforms//os:macos", + "@platforms//cpu:x86_64", + ], + target_compatible_with = [ + "@platforms//os:macos", + "@platforms//cpu:x86_64", + ], + target_settings = [ + ":dev", + ], + toolchain = "@toolchain_dev_darwin_x86-64//:toolchain", + toolchain_type = "@bazel_tools//tools/cpp:toolchain_type", +) + +platform( + name = "darwin_x86_64", + constraint_values = [ + "@platforms//os:macos", + "@platforms//cpu:x86_64", + ], +) + config_setting( name = "dbg_crdb_test", define_values = {