From 666d47dc71da626d0a1be0a62427561503978205 Mon Sep 17 00:00:00 2001 From: Rohan Yadav Date: Mon, 31 Aug 2020 16:11:12 -0400 Subject: [PATCH] sql: update `format_type` to handle user defined types Fixes #53684. Update the `format_type` builtin to handle user defined types. Release justification: bug fix Release note: None --- pkg/sql/faketreeeval/evalctx.go | 13 +++++++++++ .../logictest/testdata/logic_test/pg_builtins | 15 ++++++++++++ pkg/sql/sem/builtins/pg_builtins.go | 23 ++++++++++++++++--- pkg/sql/sem/tree/eval.go | 1 + 4 files changed, 49 insertions(+), 3 deletions(-) diff --git a/pkg/sql/faketreeeval/evalctx.go b/pkg/sql/faketreeeval/evalctx.go index 5b7eebdb800d..58db1900119f 100644 --- a/pkg/sql/faketreeeval/evalctx.go +++ b/pkg/sql/faketreeeval/evalctx.go @@ -22,6 +22,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented" "github.com/cockroachdb/errors" + "github.com/lib/pq/oid" ) // DummySequenceOperators implements the tree.SequenceOperators interface by @@ -119,6 +120,18 @@ func (ep *DummyEvalPlanner) EvalSubquery(expr *tree.Subquery) (tree.Datum, error return nil, errors.WithStack(errEvalPlanner) } +// ResolveTypeByOID implements the tree.TypeReferenceResolver interface. +func (ep *DummyEvalPlanner) ResolveTypeByOID(_ context.Context, _ oid.Oid) (*types.T, error) { + return nil, errors.WithStack(errEvalPlanner) +} + +// ResolveType implements the tree.TypeReferenceResolver interface. +func (ep *DummyEvalPlanner) ResolveType( + _ context.Context, _ *tree.UnresolvedObjectName, +) (*types.T, error) { + return nil, errors.WithStack(errEvalPlanner) +} + // DummyPrivilegedAccessor implements the tree.PrivilegedAccessor interface by returning errors. type DummyPrivilegedAccessor struct{} diff --git a/pkg/sql/logictest/testdata/logic_test/pg_builtins b/pkg/sql/logictest/testdata/logic_test/pg_builtins index e2f4b966e1f0..f441ff9cdfe7 100644 --- a/pkg/sql/logictest/testdata/logic_test/pg_builtins +++ b/pkg/sql/logictest/testdata/logic_test/pg_builtins @@ -18,3 +18,18 @@ SELECT pg_my_temp_schema() # Regression test for #49072. statement ok SELECT has_table_privilege('root'::NAME, 0, 'select') + +# Regression test for #53684. +statement ok +CREATE TYPE typ AS ENUM ('hello') + +query T +SELECT format_type(oid, 0) FROM pg_catalog.pg_type WHERE typname = 'typ' +---- +typ + +# Nothing breaks if we put a non-existing oid into format_type. +query T +SELECT format_type(152100, 0) +---- +unknown (OID=152100) diff --git a/pkg/sql/sem/builtins/pg_builtins.go b/pkg/sql/sem/builtins/pg_builtins.go index 5b45d38ef68c..7d00d9e31a57 100644 --- a/pkg/sql/sem/builtins/pg_builtins.go +++ b/pkg/sql/sem/builtins/pg_builtins.go @@ -16,6 +16,7 @@ import ( "time" "github.com/cockroachdb/cockroach/pkg/keys" + "github.com/cockroachdb/cockroach/pkg/sql/catalog" "github.com/cockroachdb/cockroach/pkg/sql/catalog/catconstants" "github.com/cockroachdb/cockroach/pkg/sql/parser" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" @@ -827,7 +828,7 @@ var pgBuiltins = map[string]builtinDefinition{ }, ), - "format_type": makeBuiltin(tree.FunctionProperties{NullableArgs: true}, + "format_type": makeBuiltin(tree.FunctionProperties{NullableArgs: true, DistsqlBlocklist: true}, tree.Overload{ Types: tree.ArgTypes{{"type_oid", types.Oid}, {"typemod", types.Int}}, ReturnType: tree.FixedReturnType(types.String), @@ -838,9 +839,25 @@ var pgBuiltins = map[string]builtinDefinition{ return tree.DNull, nil } maybeTypmod := args[1] - typ, ok := types.OidToType[oid.Oid(int(oidArg.(*tree.DOid).DInt))] + oid := oid.Oid(int(oidArg.(*tree.DOid).DInt)) + typ, ok := types.OidToType[oid] if !ok { - return tree.NewDString(fmt.Sprintf("unknown (OID=%s)", oidArg)), nil + // If the type wasn't statically known, try looking it up as a user + // defined type. + var err error + typ, err = ctx.Planner.ResolveTypeByOID(ctx.Context, oid) + if err != nil { + // If the error is a descriptor does not exist error, then swallow it. + unknown := tree.NewDString(fmt.Sprintf("unknown (OID=%s)", oidArg)) + switch { + case errors.Is(err, catalog.ErrDescriptorNotFound): + return unknown, nil + case pgerror.GetPGCode(err) == pgcode.UndefinedObject: + return unknown, nil + default: + return nil, err + } + } } var hasTypmod bool var typmod int diff --git a/pkg/sql/sem/tree/eval.go b/pkg/sql/sem/tree/eval.go index 479ba7d14234..7d1531d210f8 100644 --- a/pkg/sql/sem/tree/eval.go +++ b/pkg/sql/sem/tree/eval.go @@ -2993,6 +2993,7 @@ type EvalDatabase interface { // EvalPlanner is a limited planner that can be used from EvalContext. type EvalPlanner interface { EvalDatabase + TypeReferenceResolver // ParseType parses a column type. ParseType(sql string) (*types.T, error)