Skip to content

Commit

Permalink
sql/schemachanger: fix prefix resolution errors during CREATE
Browse files Browse the repository at this point in the history
Previously, in the declarative schema changer, when resolving prefixes
for new objects (CREATE FUNCTION / CREATE SEQUENCE), we would incorrectly
generate internal errors if the resolution failed. This was incorrect and
a regression versus the legacy schema changer. To address this, this patch
refactors the resolution logic to generate appropriate errors for target
objects when the prefix can't be resolved.

Fixes: #106302

Release note: None
  • Loading branch information
fqazi committed Jul 13, 2023
1 parent 8dcde9c commit 9da5dd8
Show file tree
Hide file tree
Showing 8 changed files with 45 additions and 32 deletions.
10 changes: 10 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/sequences
Original file line number Diff line number Diff line change
Expand Up @@ -2393,3 +2393,13 @@ statement error pgcode 42809 \"test.public.multiple_seq_test_tbl\" is not a sequ
SELECT pg_sequence_last_value('multiple_seq_test_tbl'::regclass)

subtest end

# For #106302 we had cases where internal errors could be returned in the prefix
# did not resolve properly
subtest create_prefix

statement error pgcode 3F000 cannot create \"timestamptz.primary.is\" because the target database or schema does not exist
CREATE SEQUENCE TIMESTAMPTZ . PRIMARY . IS;

statement error pgcode 3F000 cannot create \"timestamptz.is\" because the target database or schema does not exist
CREATE SEQUENCE TIMESTAMPTZ . IS;
24 changes: 19 additions & 5 deletions pkg/sql/schemachanger/scbuild/builder_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -848,11 +848,25 @@ func (b *builderState) ResolveSchema(
return b.QueryByID(sc.GetID())
}

// ResolvePrefix implements the scbuildstmt.NameResolver interface.
func (b *builderState) ResolvePrefix(
prefix tree.ObjectNamePrefix, requiredSchemaPriv privilege.Kind,
// ResolveTargetObject implements the scbuildstmt.NameResolver interface.
func (b *builderState) ResolveTargetObject(
name *tree.UnresolvedObjectName, requiredSchemaPriv privilege.Kind,
) (dbElts scbuildstmt.ElementResultSet, scElts scbuildstmt.ElementResultSet) {
db, sc := b.cr.MustResolvePrefix(b.ctx, prefix)
objName := name.ToTableName()
db, sc := b.cr.MayResolvePrefix(b.ctx, objName.ObjectNamePrefix)
// If the database or schema are not found during resolution,
// then generate an appropriate error.
if db == nil || sc == nil {
if !objName.ExplicitSchema && !objName.ExplicitCatalog {
panic(
pgerror.New(pgcode.InvalidName, "no database specified"),
)
}
panic(errors.WithHint(pgerror.Newf(pgcode.InvalidSchemaName,
"cannot create %q because the target database or schema does not exist",
tree.ErrString(&objName)),
"verify that the current database and search_path are valid and/or the target database exists"))
}
b.ensureDescriptor(db.GetID())
if sc.SchemaKind() == catalog.SchemaVirtual {
panic(sqlerrors.NewCannotModifyVirtualSchemaError(sc.GetName()))
Expand All @@ -861,7 +875,7 @@ func (b *builderState) ResolvePrefix(
panic(unimplemented.NewWithIssue(104687, "cannot create UDFs under a temporary schema"))
}
b.ensureDescriptor(sc.GetID())
b.checkOwnershipOrPrivilegesOnSchemaDesc(prefix, sc, scbuildstmt.ResolveParams{RequiredPrivilege: requiredSchemaPriv})
b.checkOwnershipOrPrivilegesOnSchemaDesc(objName.ObjectNamePrefix, sc, scbuildstmt.ResolveParams{RequiredPrivilege: requiredSchemaPriv})
return b.QueryByID(db.GetID()), b.QueryByID(sc.GetID())
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/schemachanger/scbuild/dependencies.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,10 @@ type CatalogReader interface {
// If withOffline is set, we include offline schema descs into our search.
MayResolveSchema(ctx context.Context, name tree.ObjectNamePrefix, withOffline bool) (catalog.DatabaseDescriptor, catalog.SchemaDescriptor)

// MustResolvePrefix looks up a database and schema given the prefix at best
// MayResolvePrefix looks up a database and schema given the prefix at best
// effort, meaning the prefix may not have explicit catalog and schema name.
// It fails if the db or schema represented by the prefix does not exist.
MustResolvePrefix(ctx context.Context, name tree.ObjectNamePrefix) (catalog.DatabaseDescriptor, catalog.SchemaDescriptor)
MayResolvePrefix(ctx context.Context, name tree.ObjectNamePrefix) (catalog.DatabaseDescriptor, catalog.SchemaDescriptor)

// MayResolveTable looks up a table by name.
MayResolveTable(ctx context.Context, name tree.UnresolvedObjectName) (catalog.ResolvedObjectPrefix, catalog.TableDescriptor)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func CreateFunction(b BuildCtx, n *tree.CreateFunction) {
}
b.IncrementSchemaChangeCreateCounter("function")

dbElts, scElts := b.ResolvePrefix(n.FuncName.ObjectNamePrefix, privilege.CREATE)
dbElts, scElts := b.ResolveTargetObject(n.FuncName.ToUnresolvedObjectName(), privilege.CREATE)
_, _, sc := scpb.FindSchema(scElts)
_, _, db := scpb.FindDatabase(dbElts)
_, _, scName := scpb.FindNamespace(scElts)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (
)

func CreateSequence(b BuildCtx, n *tree.CreateSequence) {
dbElts, scElts := b.ResolvePrefix(n.Name.ObjectNamePrefix, privilege.CREATE)
dbElts, scElts := b.ResolveTargetObject(n.Name.ToUnresolvedObjectName(), privilege.CREATE)
_, _, schemaElem := scpb.FindSchema(scElts)
_, _, dbElem := scpb.FindDatabase(dbElts)
_, _, scName := scpb.FindNamespace(scElts)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -338,12 +338,10 @@ type NameResolver interface {
// ResolveSchema retrieves a schema by name and returns its elements.
ResolveSchema(name tree.ObjectNamePrefix, p ResolveParams) ElementResultSet

// ResolvePrefix retrieves database and schema given the name prefix. The
// requested schema must exist and current user must have the required
// privilege.
ResolvePrefix(
prefix tree.ObjectNamePrefix, requiredSchemaPriv privilege.Kind,
) (dbElts ElementResultSet, scElts ElementResultSet)
// ResolveTargetObject retrieves database and schema given the unresolved
// object name. The requested schema must exist and current user must have the
// required privilege.
ResolveTargetObject(prefix *tree.UnresolvedObjectName, requiredSchemaPriv privilege.Kind) (dbElts ElementResultSet, scElts ElementResultSet)

// ResolveUserDefinedTypeType retrieves a type by name and returns its elements.
ResolveUserDefinedTypeType(name *tree.UnresolvedObjectName, p ResolveParams) ElementResultSet
Expand Down
20 changes: 7 additions & 13 deletions pkg/sql/schemachanger/scdeps/build_deps.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,22 +127,19 @@ func (d *buildDeps) MayResolveSchema(
return db, schema
}

func (d *buildDeps) MustResolvePrefix(
func (d *buildDeps) MayResolvePrefix(
ctx context.Context, name tree.ObjectNamePrefix,
) (catalog.DatabaseDescriptor, catalog.SchemaDescriptor) {
) (db catalog.DatabaseDescriptor, sc catalog.SchemaDescriptor) {
const withOffline = false
if name.ExplicitSchema {
if name.ExplicitCatalog {
db, sc := d.MayResolveSchema(ctx, name, withOffline)
if sc == nil || db == nil {
panic(errors.AssertionFailedf("prefix %s does not exist", name.String()))
}
db, sc = d.MayResolveSchema(ctx, name, withOffline)
return db, sc
}

// Two parts: D.T.
// Try to use the current database, and be satisfied if it's sufficient to find the object.
db, sc := d.MayResolveSchema(ctx, tree.ObjectNamePrefix{
db, sc = d.MayResolveSchema(ctx, tree.ObjectNamePrefix{
CatalogName: tree.Name(d.CurrentDatabase()),
SchemaName: name.SchemaName,
ExplicitCatalog: true,
Expand All @@ -161,23 +158,20 @@ func (d *buildDeps) MustResolvePrefix(
ExplicitSchema: true,
},
withOffline)
if db != nil && sc != nil {
return db, sc
}
panic(errors.AssertionFailedf("prefix %s does not exist", name.String()))
return db, sc
}

path := d.sessionData.SearchPath
iter := path.IterWithoutImplicitPGSchemas()
for scName, ok := iter.Next(); ok; scName, ok = iter.Next() {
name.SchemaName = tree.Name(scName)
name.ExplicitSchema = true
db, sc := d.MayResolveSchema(ctx, name, false /* withOffline */)
db, sc = d.MayResolveSchema(ctx, name, false /* withOffline */)
if sc != nil {
return db, sc
}
}
panic(errors.AssertionFailedf("prefix %s does not exist", name.String()))
return nil, nil
}

// MayResolveTable implements the scbuild.CatalogReader interface.
Expand Down
5 changes: 1 addition & 4 deletions pkg/sql/schemachanger/scdeps/sctestdeps/test_deps.go
Original file line number Diff line number Diff line change
Expand Up @@ -286,13 +286,10 @@ func (s *TestState) MayResolveSchema(
return db, sc
}

func (s *TestState) MustResolvePrefix(
func (s *TestState) MayResolvePrefix(
ctx context.Context, name tree.ObjectNamePrefix,
) (catalog.DatabaseDescriptor, catalog.SchemaDescriptor) {
db, sc := s.mayResolvePrefix(name)
if sc == nil {
panic(errors.AssertionFailedf("prefix %s does not exist", name.String()))
}
return db.(catalog.DatabaseDescriptor), sc.(catalog.SchemaDescriptor)
}

Expand Down

0 comments on commit 9da5dd8

Please sign in to comment.