Merge #73875 #73942 #73948 #73982

73875: restoreccl: insert system.namespace entry for synthetic public schemas during restore r=dt a=RichardJCai

Release note (bug fix): Previously during restore we wouldn't insert a system.namespace
entry for synthetic public schemas.

Resolves #73535

73942: sql: simplify inserts into database descs with empty schemas map r=RichardJCai a=RichardJCai

Release note: None

73948: DOC-2096: Update diagrams.go to force scrolling on svg div blocks r=ericharmeling a=nickvigilante

Release note: None

73982: bazel: expose `devdarwinx86_64` toolchain via `platform` rules r=irfansharif a=rickystewart

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

Co-authored-by: richardjcai <[email protected]>
Co-authored-by: Nick Vigilante <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>

.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

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(

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 = {

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.
@@ -1321,11 +1327,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,
@@ -1550,15 +1553,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

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"}})
+	}
+}

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,
@@ -1563,10 +1559,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(),

pkg/cmd/docgen/diagrams.go

@@ -243,7 +243,7 @@ func init() {
 						// Wrap the output in a <div> so that the Markdown parser
 						// doesn't attempt to parse the inside of the contained
 						// <svg> as Markdown.
-						body = fmt.Sprintf(`<div>%s</div>`, body)
+						body = fmt.Sprintf(`<div style="overflow-x:auto;">%s</div>`, body)
 						// Remove blank lines and strip spaces.
 						body = stripRE.ReplaceAllString(body, "\n") + "\n"
 					}

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

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,

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.

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,