sql/catalog: remove bool found parameter from schema access

This should be inferred from the return values.

Release note: None

pkg/sql/alter_schema.go

@@ -58,13 +58,10 @@ func (p *planner) AlterSchema(ctx context.Context, n *tree.AlterSchema) (planNod
 	if err != nil {
 		return nil, err
 	}
-	found, schema, err := p.ResolveMutableSchemaDescriptor(ctx, db.ID, string(n.Schema.SchemaName), true /* required */)
+	schema, err := p.ResolveMutableSchemaDescriptor(ctx, db.ID, string(n.Schema.SchemaName), true /* required */)
 	if err != nil {
 		return nil, err
 	}
-	if !found {
-		return nil, pgerror.Newf(pgcode.InvalidSchemaName, "schema %q does not exist", n.Schema.String())
-	}
 	switch schema.SchemaKind() {
 	case catalog.SchemaPublic, catalog.SchemaVirtual, catalog.SchemaTemporary:
 		return nil, pgerror.Newf(pgcode.InvalidSchemaName, "cannot modify schema %q", n.Schema.String())

pkg/sql/catalog/accessor.go

@@ -44,7 +44,7 @@ type Accessor interface {
 	// exists under the target database.
 	GetSchemaByName(
 		ctx context.Context, txn *kv.Txn, dbID descpb.ID, scName string, flags tree.SchemaLookupFlags,
-	) (bool, SchemaDescriptor, error)
+	) (SchemaDescriptor, error)
 
 	// GetObjectNamesAndIDs returns the list of all objects in the given
 	// database and schema.

pkg/sql/catalog/descs/collection.go

@@ -625,14 +625,14 @@ func (tc *Collection) getObjectByName(
 	dbID := db.GetID()
 
 	// Resolve the schema.
-	foundSchema, scDesc, err := tc.GetImmutableSchemaByName(ctx, txn, dbID, schemaName,
+	scDesc, err := tc.GetImmutableSchemaByName(ctx, txn, dbID, schemaName,
 		tree.SchemaLookupFlags{
 			Required:       flags.Required,
 			AvoidCached:    avoidCachedForParent,
 			IncludeDropped: flags.IncludeDropped,
 			IncludeOffline: flags.IncludeOffline,
 		})
-	if err != nil || !foundSchema {
+	if err != nil || scDesc == nil {
 		return false, nil, err
 	}
 	schemaID := scDesc.GetID()
@@ -932,7 +932,7 @@ func filterDescriptorState(
 // mutable descriptor usable by the transaction. RequireMutable is ignored.
 func (tc *Collection) GetMutableSchemaByName(
 	ctx context.Context, txn *kv.Txn, dbID descpb.ID, schemaName string, flags tree.SchemaLookupFlags,
-) (bool, catalog.SchemaDescriptor, error) {
+) (catalog.SchemaDescriptor, error) {
 	flags.RequireMutable = true
 	return tc.getSchemaByName(ctx, txn, dbID, schemaName, flags)
 }
@@ -941,32 +941,32 @@ func (tc *Collection) GetMutableSchemaByName(
 // immutable descriptor usable by the transaction. RequireMutable is ignored.
 func (tc *Collection) GetImmutableSchemaByName(
 	ctx context.Context, txn *kv.Txn, dbID descpb.ID, schemaName string, flags tree.SchemaLookupFlags,
-) (bool, catalog.SchemaDescriptor, error) {
+) (catalog.SchemaDescriptor, error) {
 	flags.RequireMutable = false
 	return tc.getSchemaByName(ctx, txn, dbID, schemaName, flags)
 }
 
 // GetSchemaByName returns true and a ResolvedSchema object if the target schema
 // exists under the target database.
 func (tc *Collection) GetSchemaByName(
-	ctx context.Context, txn *kv.Txn, dbID descpb.ID, schemaName string, flags tree.SchemaLookupFlags,
-) (found bool, _ catalog.SchemaDescriptor, _ error) {
-	return tc.getSchemaByName(ctx, txn, dbID, schemaName, flags)
+	ctx context.Context, txn *kv.Txn, dbID descpb.ID, scName string, flags tree.SchemaLookupFlags,
+) (catalog.SchemaDescriptor, error) {
+	return tc.getSchemaByName(ctx, txn, dbID, scName, flags)
 }
 
 // getSchemaByName resolves the schema and, if applicable, returns a descriptor
 // usable by the transaction.
 func (tc *Collection) getSchemaByName(
 	ctx context.Context, txn *kv.Txn, dbID descpb.ID, schemaName string, flags tree.SchemaLookupFlags,
-) (bool, catalog.SchemaDescriptor, error) {
+) (catalog.SchemaDescriptor, error) {
 	// Fast path public schema, as it is always found.
 	if schemaName == tree.PublicSchema {
-		return true, schemadesc.GetPublicSchema(), nil
+		return schemadesc.GetPublicSchema(), nil
 	}
 
 	if tc.virtualSchemas != nil {
 		if sc, ok := tc.virtualSchemas.GetVirtualSchema(schemaName); ok {
-			return true, sc.Desc(), nil
+			return sc.Desc(), nil
 		}
 	}
 
@@ -978,7 +978,7 @@ func (tc *Collection) getSchemaByName(
 				schemaName == tc.sessionData.SearchPath.GetTemporarySchemaName() {
 				schemaID, found := tc.sessionData.GetTemporarySchemaIDForDb(uint32(dbID))
 				if found {
-					return true, schemadesc.NewTemporarySchema(
+					return schemadesc.NewTemporarySchema(
 						tc.sessionData.SearchPath.GetTemporarySchemaName(),
 						descpb.ID(schemaID),
 						dbID,
@@ -988,14 +988,14 @@ func (tc *Collection) getSchemaByName(
 		}
 		exists, schemaID, err := catalogkv.ResolveSchemaID(ctx, txn, tc.codec(), dbID, schemaName)
 		if err != nil {
-			return false, nil, err
+			return nil, err
 		} else if !exists {
 			if flags.Required {
-				return false, nil, sqlerrors.NewUndefinedSchemaError(schemaName)
+				return nil, sqlerrors.NewUndefinedSchemaError(schemaName)
 			}
-			return false, nil, nil
+			return nil, nil
 		}
-		return true, schemadesc.NewTemporarySchema(
+		return schemadesc.NewTemporarySchema(
 			schemaName,
 			schemaID,
 			dbID,
@@ -1005,9 +1005,9 @@ func (tc *Collection) getSchemaByName(
 	// Otherwise, the schema is user-defined. Get the descriptor.
 	desc, err := tc.getUserDefinedSchemaByName(ctx, txn, dbID, schemaName, flags)
 	if err != nil || desc == nil {
-		return false, nil, err
+		return nil, err
 	}
-	return true, desc, nil
+	return desc, nil
 }
 
 // GetMutableDatabaseByID returns a mutable database descriptor with
@@ -1969,15 +1969,11 @@ func (tc *Collection) GetObjectNamesAndIDs(
 		IncludeDropped: flags.IncludeDropped,
 		IncludeOffline: flags.IncludeOffline,
 	}
-	ok, schema, err := tc.GetImmutableSchemaByName(ctx, txn, dbDesc.GetID(), scName, schemaFlags)
+	schema, err := tc.GetImmutableSchemaByName(ctx, txn, dbDesc.GetID(), scName, schemaFlags)
 	if err != nil {
 		return nil, nil, err
 	}
-	if !ok {
-		if flags.Required {
-			tn := tree.MakeTableNameWithSchema(tree.Name(dbDesc.GetName()), tree.Name(scName), "")
-			return nil, nil, sqlerrors.NewUnsupportedSchemaUsageError(tree.ErrString(&tn.ObjectNamePrefix))
-		}
+	if schema == nil { // required must have been false
 		return nil, nil, nil
 	}
 

pkg/sql/catalog/descs/collection_test.go

@@ -296,13 +296,13 @@ func TestAddUncommittedDescriptorAndMutableResolution(t *testing.T) {
 			flags.RequireMutable = true
 			flags.Required = true
 
-			ok, schema, err := descriptors.GetMutableSchemaByName(ctx, txn, dbID, "sc", flags)
+			schema, err := descriptors.GetMutableSchemaByName(ctx, txn, dbID, "sc", flags)
 			require.NoError(t, err)
-			require.True(t, ok)
+			require.NotNil(t, schema)
 
-			ok, resolved, err := descriptors.GetMutableSchemaByName(ctx, txn, dbID, "sc", flags)
+			resolved, err := descriptors.GetMutableSchemaByName(ctx, txn, dbID, "sc", flags)
 			require.NoError(t, err)
-			require.True(t, ok)
+			require.NotNil(t, schema)
 
 			require.Same(t, schema, resolved)
 
@@ -492,7 +492,7 @@ CREATE TABLE test.schema.t(x INT);
 				if err != nil {
 					return err
 				}
-				_, schemaDesc, err := descsCol.GetMutableSchemaByName(ctx, txn, dbDesc.GetID(), "schema", tree.SchemaLookupFlags{Required: true})
+				schemaDesc, err := descsCol.GetMutableSchemaByName(ctx, txn, dbDesc.GetID(), "schema", tree.SchemaLookupFlags{Required: true})
 				if err != nil {
 					return err
 				}

pkg/sql/create_table.go

@@ -96,7 +96,7 @@ func (n *createTableNode) ReadingOwnWrites() {}
 func (p *planner) getSchemaIDForCreate(
 	ctx context.Context, codec keys.SQLCodec, dbID descpb.ID, scName string,
 ) (descpb.ID, error) {
-	_, res, err := p.ResolveUncachedSchemaDescriptor(ctx, dbID, scName, true /* required */)
+	res, err := p.ResolveUncachedSchemaDescriptor(ctx, dbID, scName, true /* required */)
 	if err != nil {
 		return 0, err
 	}

pkg/sql/drop_database.go

@@ -84,14 +84,10 @@ func (p *planner) DropDatabase(ctx context.Context, n *tree.DropDatabase) (planN
 	d := newDropCascadeState()
 
 	for _, schema := range schemas {
-		found, res, err := p.ResolveMutableSchemaDescriptor(ctx, dbDesc.ID, schema, true /* required */)
+		res, err := p.ResolveMutableSchemaDescriptor(ctx, dbDesc.ID, schema, true /* required */)
 		if err != nil {
 			return nil, err
 		}
-		if !found {
-			log.Warningf(ctx, "could not find schema %s under database %d", schema, dbDesc.ID)
-			continue
-		}
 		if err := d.collectObjectsInSchema(ctx, p, dbDesc, res); err != nil {
 			return nil, err
 		}

pkg/sql/drop_schema.go

@@ -69,11 +69,11 @@ func (p *planner) DropSchema(ctx context.Context, n *tree.DropSchema) (planNode,
 			return nil, err
 		}
 
-		found, sc, err := p.ResolveMutableSchemaDescriptor(ctx, db.ID, scName, false /* required */)
+		sc, err := p.ResolveMutableSchemaDescriptor(ctx, db.ID, scName, false /* required */)
 		if err != nil {
 			return nil, err
 		}
-		if !found {
+		if sc == nil {
 			if n.IfExists {
 				continue
 			}

pkg/sql/rename_table.go

@@ -120,7 +120,7 @@ func (n *renameTableNode) startExec(params runParams) error {
 			return err
 		}
 
-		_, targetSchemaDesc, err = p.ResolveUncachedSchemaDescriptor(ctx, targetDbDesc.GetID(), oldTn.Schema(), true)
+		targetSchemaDesc, err = p.ResolveUncachedSchemaDescriptor(ctx, targetDbDesc.GetID(), oldTn.Schema(), true)
 		if err != nil {
 			return err
 		}

pkg/sql/resolver.go

@@ -52,28 +52,35 @@ func (p *planner) ResolveUncachedDatabaseByName(
 // ResolveUncachedSchemaDescriptor looks up a schema from the store.
 func (p *planner) ResolveUncachedSchemaDescriptor(
 	ctx context.Context, dbID descpb.ID, name string, required bool,
-) (found bool, schema catalog.SchemaDescriptor, err error) {
+) (schema catalog.SchemaDescriptor, err error) {
 	p.runWithOptions(resolveFlags{skipCache: true}, func() {
-		found, schema, err = p.Accessor().GetSchemaByName(
+		schema, err = p.Accessor().GetSchemaByName(
 			ctx, p.txn, dbID, name, tree.SchemaLookupFlags{
 				Required: required, RequireMutable: true,
 			},
 		)
 	})
-	return found, schema, err
+	if err != nil || schema == nil {
+		return nil, err
+	}
+	return schema, err
 }
 
 // ResolveUncachedSchemaDescriptor looks up a mutable descriptor for a schema
 // from the store.
 func (p *planner) ResolveMutableSchemaDescriptor(
 	ctx context.Context, dbID descpb.ID, name string, required bool,
-) (found bool, schema catalog.SchemaDescriptor, err error) {
-	return p.Accessor().GetSchemaByName(
+) (schema catalog.SchemaDescriptor, err error) {
+	schema, err = p.Accessor().GetSchemaByName(
 		ctx, p.txn, dbID, name, tree.SchemaLookupFlags{
 			Required:       required,
 			RequireMutable: true,
 		},
 	)
+	if err != nil || schema == nil {
+		return nil, err
+	}
+	return schema, nil
 }
 
 // runWithOptions sets the provided resolution flags for the
@@ -177,13 +184,13 @@ func (p *planner) LookupSchema(
 	}
 	sc := p.Accessor()
 	var resolvedSchema catalog.SchemaDescriptor
-	found, resolvedSchema, err = sc.GetSchemaByName(
+	resolvedSchema, err = sc.GetSchemaByName(
 		ctx, p.txn, dbDesc.GetID(), scName, p.CommonLookupFlags(false /* required */),
 	)
-	if err != nil {
+	if err != nil || resolvedSchema == nil {
 		return false, nil, err
 	}
-	return found, &catalog.ResolvedObjectPrefix{
+	return true, &catalog.ResolvedObjectPrefix{
 		Database: dbDesc,
 		Schema:   resolvedSchema,
 	}, nil
@@ -457,7 +464,7 @@ func getDescriptorsFromTargetListForPrivilegeChange(
 		}
 
 		for _, sc := range targetSchemas {
-			_, resSchema, err := p.ResolveMutableSchemaDescriptor(
+			resSchema, err := p.ResolveMutableSchemaDescriptor(
 				ctx, sc.dbDesc.ID, sc.schema, true /* required */)
 			if err != nil {
 				return nil, err

pkg/sql/set_schema.go

@@ -46,7 +46,7 @@ func (p *planner) prepareSetSchema(
 	schemaID := desc.GetParentSchemaID()
 
 	// Lookup the schema we want to set to.
-	_, res, err := p.ResolveUncachedSchemaDescriptor(ctx, databaseID, schema, true /* required */)
+	res, err := p.ResolveUncachedSchemaDescriptor(ctx, databaseID, schema, true /* required */)
 	if err != nil {
 		return 0, err
 	}

pkg/sql/sqlerrors/errors.go

@@ -61,13 +61,6 @@ func NewUndefinedSchemaError(name string) error {
 	return pgerror.Newf(pgcode.InvalidSchemaName, "unknown schema %q", name)
 }
 
-// NewUnsupportedSchemaUsageError creates an error for an invalid
-// schema use, e.g. mydb.someschema.tbl.
-func NewUnsupportedSchemaUsageError(name string) error {
-	return pgerror.Newf(pgcode.InvalidSchemaName,
-		"unsupported schema specification: %q", name)
-}
-
 // NewCCLRequiredError creates an error for when a CCL feature is used in an OSS
 // binary.
 func NewCCLRequiredError(err error) error {