Skip to content

Commit

Permalink
sql: prevent allocations by avoiding some name pointers
Browse files Browse the repository at this point in the history
We don't need pointers for these names. They generally won't escape.

Release note: None
  • Loading branch information
ajwerner committed Oct 4, 2022
1 parent b4927a1 commit cb11765
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 18 deletions.
2 changes: 1 addition & 1 deletion pkg/sql/catalog/resolver/resolver_bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func BenchmarkResolveExistingObject(b *testing.B) {
require.NoError(b, err)
b.ResetTimer()
for i := 0; i < b.N; i++ {
desc, _, err := resolver.ResolveExistingObject(ctx, rs, uon, tc.flags)
desc, _, err := resolver.ResolveExistingObject(ctx, rs, &uon, tc.flags)
require.NoError(b, err)
require.NotNil(b, desc)
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/sql/parser/sql.y
Original file line number Diff line number Diff line change
Expand Up @@ -13654,7 +13654,6 @@ typed_literal:
// types, return an unimplemented error message.
var typ tree.ResolvableTypeReference
var ok bool
var err error
var unimp int
typ, ok, unimp = types.TypeForNonKeywordTypeName(typName)
if !ok {
Expand All @@ -13663,8 +13662,9 @@ typed_literal:
// In this case, we don't think this type is one of our
// known unsupported types, so make a type reference for it.
aIdx := sqllex.(*lexer).NewAnnotation()
typ, err = name.ToUnresolvedObjectName(aIdx)
un, err := name.ToUnresolvedObjectName(aIdx)
if err != nil { return setErr(sqllex, err) }
typ = &un
case -1:
return unimplemented(sqllex, "type name " + typName)
default:
Expand All @@ -13677,7 +13677,7 @@ typed_literal:
aIdx := sqllex.(*lexer).NewAnnotation()
res, err := name.ToUnresolvedObjectName(aIdx)
if err != nil { return setErr(sqllex, err) }
$$.val = &tree.CastExpr{Expr: tree.NewStrVal($2), Type: res, SyntaxMode: tree.CastPrepend}
$$.val = &tree.CastExpr{Expr: tree.NewStrVal($2), Type: &res, SyntaxMode: tree.CastPrepend}
}
}
| const_typename SCONST
Expand Down
3 changes: 2 additions & 1 deletion pkg/sql/schema_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,8 @@ func (sr *schemaResolver) canResolveDescUnderSchema(
case catalog.SchemaUserDefined:
return sr.authAccessor.CheckPrivilegeForUser(ctx, scDesc, privilege.USAGE, sr.sessionDataStack.Top().User())
default:
panic(errors.AssertionFailedf("unknown schema kind %d", kind))
forLog := kind // prevents kind from escaping
panic(errors.AssertionFailedf("unknown schema kind %d", forLog))
}
}

Expand Down
7 changes: 4 additions & 3 deletions pkg/sql/sem/tree/function_definition.go
Original file line number Diff line number Diff line change
Expand Up @@ -386,14 +386,15 @@ func QualifyBuiltinFunctionDefinition(
// GetBuiltinFuncDefinitionOrFail is similar to GetBuiltinFuncDefinition but
// returns an error if function is not found.
func GetBuiltinFuncDefinitionOrFail(
fName *FunctionName, searchPath SearchPath,
fName FunctionName, searchPath SearchPath,
) (*ResolvedFunctionDefinition, error) {
def, err := GetBuiltinFuncDefinition(fName, searchPath)
if err != nil {
return nil, err
}
if def == nil {
return nil, errors.Wrapf(ErrFunctionUndefined, "unknown function: %s()", ErrString(fName))
forError := fName // prevent fName from escaping
return nil, errors.Wrapf(ErrFunctionUndefined, "unknown function: %s()", ErrString(&forError))
}
return def, nil
}
Expand All @@ -409,7 +410,7 @@ func GetBuiltinFuncDefinitionOrFail(
// error is still checked and return from the function signature just in case
// we change the iterating function in the future.
func GetBuiltinFuncDefinition(
fName *FunctionName, searchPath SearchPath,
fName FunctionName, searchPath SearchPath,
) (*ResolvedFunctionDefinition, error) {
if fName.ExplicitSchema {
return ResolvedBuiltinFuncDefs[fName.Schema()+"."+fName.Object()], nil
Expand Down
13 changes: 6 additions & 7 deletions pkg/sql/sem/tree/name_part.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,23 +218,22 @@ func MakeUnresolvedName(args ...string) UnresolvedName {
}

// ToUnresolvedObjectName converts an UnresolvedName to an UnresolvedObjectName.
func (u *UnresolvedName) ToUnresolvedObjectName(idx AnnotationIdx) (*UnresolvedObjectName, error) {
func (u *UnresolvedName) ToUnresolvedObjectName(idx AnnotationIdx) (UnresolvedObjectName, error) {
if u.NumParts == 4 {
return nil, pgerror.Newf(pgcode.Syntax, "improper qualified name (too many dotted names): %s", u)
return UnresolvedObjectName{}, pgerror.Newf(pgcode.Syntax, "improper qualified name (too many dotted names): %s", u)
}
return NewUnresolvedObjectName(
return MakeUnresolvedObjectName(
u.NumParts,
[3]string{u.Parts[0], u.Parts[1], u.Parts[2]},
idx,
)
}

// ToFunctionName converts an UnresolvedName to a FunctionName.
func (u *UnresolvedName) ToFunctionName() (*FunctionName, error) {
func (u *UnresolvedName) ToFunctionName() (FunctionName, error) {
un, err := u.ToUnresolvedObjectName(NoAnnotation)
if err != nil {
return nil, errors.Newf("invalid function name: %s", u.String())
return FunctionName{}, errors.Newf("invalid function name: %s", u.String())
}
fn := un.ToFunctionName()
return &fn, nil
return un.ToFunctionName(), nil
}
20 changes: 17 additions & 3 deletions pkg/sql/sem/tree/object_name.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,13 +136,26 @@ func (*UnresolvedObjectName) tableExpr() {}
func NewUnresolvedObjectName(
numParts int, parts [3]string, annotationIdx AnnotationIdx,
) (*UnresolvedObjectName, error) {
u := &UnresolvedObjectName{
n, err := MakeUnresolvedObjectName(numParts, parts, annotationIdx)
if err != nil {
return nil, err
}
return &n, nil
}

// MakeUnresolvedObjectName creates an unresolved object name, verifying that it
// is well-formed.
func MakeUnresolvedObjectName(
numParts int, parts [3]string, annotationIdx AnnotationIdx,
) (UnresolvedObjectName, error) {
u := UnresolvedObjectName{
NumParts: numParts,
Parts: parts,
AnnotatedNode: AnnotatedNode{AnnIdx: annotationIdx},
}
if u.NumParts < 1 {
return nil, newInvTableNameError(u)
forErr := u // prevents u from escaping
return UnresolvedObjectName{}, newInvTableNameError(&forErr)
}

// Check that all the parts specified are not empty.
Expand All @@ -154,7 +167,8 @@ func NewUnresolvedObjectName(
}
for i := 0; i < lastCheck; i++ {
if len(u.Parts[i]) == 0 {
return nil, newInvTableNameError(u)
forErr := u // prevents u from escaping
return UnresolvedObjectName{}, newInvTableNameError(&forErr)
}
}
return u, nil
Expand Down

0 comments on commit cb11765

Please sign in to comment.