diff --git a/docs/generated/sql/bnf/stmt_block.bnf b/docs/generated/sql/bnf/stmt_block.bnf index f3c3c1d89929..6c9884f503db 100644 --- a/docs/generated/sql/bnf/stmt_block.bnf +++ b/docs/generated/sql/bnf/stmt_block.bnf @@ -1587,6 +1587,7 @@ case_expr ::= simple_typename ::= general_type_name + | '@' iconst32 | complex_type_name | const_typename | bit_with_length @@ -1925,6 +1926,9 @@ case_default ::= general_type_name ::= type_function_name_no_crdb_extra +iconst32 ::= + 'ICONST' + complex_type_name ::= general_type_name '.' unrestricted_name | general_type_name '.' unrestricted_name '.' unrestricted_name @@ -2163,9 +2167,6 @@ opt_interval_qualifier ::= interval_qualifier | -iconst32 ::= - 'ICONST' - func_application ::= func_name '(' ')' | func_name '(' expr_list opt_sort_clause ')' diff --git a/pkg/ccl/backupccl/backup_planning.go b/pkg/ccl/backupccl/backup_planning.go index 3b4b192c81d1..828ce8b529e2 100644 --- a/pkg/ccl/backupccl/backup_planning.go +++ b/pkg/ccl/backupccl/backup_planning.go @@ -34,6 +34,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/cockroach/pkg/storage/cloud" "github.com/cockroachdb/cockroach/pkg/util/ctxgroup" + "github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented" "github.com/cockroachdb/cockroach/pkg/util/hlc" "github.com/cockroachdb/cockroach/pkg/util/interval" "github.com/cockroachdb/cockroach/pkg/util/log" @@ -356,6 +357,13 @@ func backupPlanHook( } tables = append(tables, tableDesc) + // If the table has any user defined types, error out. + for _, col := range tableDesc.Columns { + if col.Type.UserDefined() { + return unimplemented.NewWithIssue(48689, "user defined types in backup") + } + } + // Collect all the table stats for this table. tableStatisticsAcc, err := statsCache.GetTableStats(ctx, tableDesc.GetID()) if err != nil { diff --git a/pkg/ccl/logictestccl/testdata/logic_test/restore b/pkg/ccl/logictestccl/testdata/logic_test/restore index 858256bd74dc..119411d9ff95 100644 --- a/pkg/ccl/logictestccl/testdata/logic_test/restore +++ b/pkg/ccl/logictestccl/testdata/logic_test/restore @@ -3,3 +3,11 @@ # Check that we get through parsing and license check. statement error pq: failed to open backup storage location: unsupported storage scheme: "" RESTORE foo FROM "bar" + +# Check that user defined types are disallowed in backups. +statement ok +CREATE TYPE t AS ENUM ('hello'); +CREATE TABLE tt (x t) + +statement error pq: unimplemented: user defined types in backup +BACKUP TABLE tt TO "" diff --git a/pkg/server/server_sql.go b/pkg/server/server_sql.go index 6476a16b00c1..c78a49605e1e 100644 --- a/pkg/server/server_sql.go +++ b/pkg/server/server_sql.go @@ -412,6 +412,7 @@ func newSQLServer(ctx context.Context, cfg sqlServerArgs) (*sqlServer, error) { cfg.gossip, cfg.db, cfg.circularInternalExecutor, + codec, ), // Note: don't forget to add the secondary loggers as closers diff --git a/pkg/sql/create_stats.go b/pkg/sql/create_stats.go index 95499a79b387..eeb10b54474f 100644 --- a/pkg/sql/create_stats.go +++ b/pkg/sql/create_stats.go @@ -397,6 +397,10 @@ func (r *createStatsResumer) Resume( dsp := p.DistSQLPlanner() if err := p.ExecCfg().DB.Txn(ctx, func(ctx context.Context, txn *kv.Txn) error { + // Set the transaction on the EvalContext to this txn. This allows for + // use of the txn during processor setup during the execution of the flow. + evalCtx.Txn = txn + if details.AsOf != nil { p.semaCtx.AsOfTimestamp = details.AsOf p.extendedEvalCtx.SetTxnTimestamp(details.AsOf.GoTime()) diff --git a/pkg/sql/execinfra/expr.go b/pkg/sql/execinfra/expr.go index ecbffc6c36d3..e4fedf68436f 100644 --- a/pkg/sql/execinfra/expr.go +++ b/pkg/sql/execinfra/expr.go @@ -158,6 +158,7 @@ func (eh *ExprHelper) Init( } var err error semaContext := tree.MakeSemaContext() + semaContext.TypeResolver = evalCtx.DistSQLTypeResolver eh.Expr, err = processExpression(expr, evalCtx, &semaContext, &eh.Vars) if err != nil { return err diff --git a/pkg/sql/execinfra/flow_context.go b/pkg/sql/execinfra/flow_context.go index 12cdab362d38..d0e15f2acd04 100644 --- a/pkg/sql/execinfra/flow_context.go +++ b/pkg/sql/execinfra/flow_context.go @@ -70,7 +70,9 @@ type FlowCtx struct { // them at runtime to ensure expressions are evaluated with the correct indexed // var context. func (ctx *FlowCtx) NewEvalCtx() *tree.EvalContext { - return ctx.EvalCtx.Copy() + evalCopy := ctx.EvalCtx.Copy() + evalCopy.DistSQLTypeResolver = &execinfrapb.DistSQLTypeResolver{EvalContext: evalCopy} + return evalCopy } // TestingKnobs returns the distsql testing knobs for this flow context. diff --git a/pkg/sql/execinfra/processorsbase.go b/pkg/sql/execinfra/processorsbase.go index fcaef4ebb2c1..178fb9cd34a3 100644 --- a/pkg/sql/execinfra/processorsbase.go +++ b/pkg/sql/execinfra/processorsbase.go @@ -796,6 +796,12 @@ func (pb *ProcessorBase) InitWithEvalCtx( pb.MemMonitor = memMonitor pb.trailingMetaCallback = opts.TrailingMetaCallback pb.inputsToDrain = opts.InputsToDrain + + // Hydrate all types used in the processor. + if err := execinfrapb.HydrateTypeSlice(evalCtx, types); err != nil { + return err + } + return pb.Out.Init(post, types, pb.EvalCtx, output) } diff --git a/pkg/sql/execinfrapb/data.go b/pkg/sql/execinfrapb/data.go index f092dad52fc3..bb797524fde5 100644 --- a/pkg/sql/execinfrapb/data.go +++ b/pkg/sql/execinfrapb/data.go @@ -18,6 +18,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sqlbase" + "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/cockroach/pkg/util/encoding" "github.com/cockroachdb/cockroach/pkg/util/tracing" "github.com/cockroachdb/errors" @@ -71,11 +72,73 @@ func ConvertToMappedSpecOrdering( return specOrdering } +// DistSQLTypeResolver implements tree.ResolvableTypeReference for accessing +// type information during DistSQL query evaluation. +type DistSQLTypeResolver struct { + EvalContext *tree.EvalContext + // TODO (rohany): This struct should locally cache id -> types.T here + // so that repeated lookups do not incur additional KV operations. +} + +// ResolveType implements tree.ResolvableTypeReference. +func (tr *DistSQLTypeResolver) ResolveType(name *tree.UnresolvedObjectName) (*types.T, error) { + return nil, errors.AssertionFailedf("cannot resolve types in DistSQL by name") +} + +// ResolveTypeByID implements tree.ResolvableTypeReference. +func (tr *DistSQLTypeResolver) ResolveTypeByID(id uint32) (*types.T, error) { + // TODO (rohany): This should eventually look into the set of cached type + // descriptors before attempting to access it here. + typDesc, err := sqlbase.GetTypeDescFromID( + tr.EvalContext.Context, + tr.EvalContext.Txn, + tr.EvalContext.Codec, + sqlbase.ID(id), + ) + if err != nil { + return nil, err + } + var typ *types.T + switch t := typDesc.Kind; t { + case sqlbase.TypeDescriptor_ENUM: + typ = types.MakeEnum(id) + default: + return nil, errors.AssertionFailedf("unknown type kind %s", t) + } + if err := typDesc.HydrateTypeInfo(typ); err != nil { + return nil, err + } + return typ, nil +} + +// HydrateTypeSlice hydrates all user defined types in an input slice of types. +func HydrateTypeSlice(evalCtx *tree.EvalContext, typs []*types.T) error { + for _, t := range typs { + if t.UserDefined() { + // TODO (rohany): This should eventually look into the set of cached type + // descriptors before attempting to access it here. + typDesc, err := sqlbase.GetTypeDescFromID( + evalCtx.Context, + evalCtx.Txn, + evalCtx.Codec, + sqlbase.ID(t.StableTypeID()), + ) + if err != nil { + return err + } + if err := typDesc.HydrateTypeInfo(t); err != nil { + return err + } + } + } + return nil +} + // ExprFmtCtxBase produces a FmtCtx used for serializing expressions; a proper // IndexedVar formatting function needs to be added on. It replaces placeholders // with their values. func ExprFmtCtxBase(evalCtx *tree.EvalContext) *tree.FmtCtx { - fmtCtx := tree.NewFmtCtx(tree.FmtCheckEquivalence) + fmtCtx := tree.NewFmtCtx(tree.FmtDistSQLSerialization) fmtCtx.SetPlaceholderFormat( func(fmtCtx *tree.FmtCtx, p *tree.Placeholder) { d, err := p.Eval(evalCtx) @@ -115,7 +178,7 @@ func (e *Expression) Empty() bool { // String implements the Stringer interface. func (e Expression) String() string { if e.LocalExpr != nil { - ctx := tree.NewFmtCtx(tree.FmtCheckEquivalence) + ctx := tree.NewFmtCtx(tree.FmtDistSQLSerialization) ctx.FormatNode(e.LocalExpr) return ctx.CloseAndGetString() } diff --git a/pkg/sql/logictest/testdata/logic_test/enums b/pkg/sql/logictest/testdata/logic_test/enums index 5433627fe558..d42f84d023a3 100644 --- a/pkg/sql/logictest/testdata/logic_test/enums +++ b/pkg/sql/logictest/testdata/logic_test/enums @@ -145,9 +145,6 @@ true true statement error pq: invalid input value for enum greeting: "notagreeting" SELECT 'hello'::greeting = 'notagreeting' -statement error pq: value type greeting cannot be used for table columns -CREATE TABLE bad (x greeting) - statement error pq: unimplemented: ALTER TYPE ADD VALUE unsupported ALTER TYPE greeting ADD VALUE 'hola' AFTER 'hello' @@ -199,3 +196,166 @@ SELECT enum_range(NULL::dbs, NULL::dbs) query error pq: enum_range\(\): mismatched types SELECT enum_range('cockroach'::dbs, 'hello'::greeting) + +# Test inserting and reading enum data from tables. +statement ok +CREATE TABLE greeting_table (x1 greeting, x2 greeting) + +statement error pq: invalid input value for enum greeting: "bye" +INSERT INTO greeting_table VALUES ('bye', 'hi') + +statement ok +INSERT INTO greeting_table VALUES ('hi', 'hello') + +query TT +SELECT * FROM greeting_table +---- +hi hello + +query TT +SELECT 'hello'::greeting, x1 FROM greeting_table +---- +hello hi + +query TB +SELECT x1, x1 < 'hello' FROM greeting_table +---- +hi false + +query TT +SELECT x1, enum_first(x1) FROM greeting_table +---- +hi hello + +statement ok +CREATE TABLE t1 (x greeting, INDEX i (x)); +CREATE TABLE t2 (x greeting, INDEX i (x)); +INSERT INTO t1 VALUES ('hello'); +INSERT INTO t2 VALUES ('hello') + +query TT +SELECT * FROM t1 INNER LOOKUP JOIN t2 ON t1.x = t2.x +---- +hello hello + +query TT +SELECT * FROM t1 INNER HASH JOIN t2 ON t1.x = t2.x +---- +hello hello + +query TT +SELECT * FROM t1 INNER MERGE JOIN t2 ON t1.x = t2.x +---- +hello hello + +statement ok +INSERT INTO t2 VALUES ('hello'), ('hello'), ('howdy'), ('hi') + +query T rowsort +SELECT DISTINCT x FROM t2 +---- +hello +howdy +hi + +query T +SELECT DISTINCT x FROM t2 ORDER BY x DESC +---- +hi +howdy +hello + +# Test out some subqueries. +query T rowsort +SELECT x FROM t2 WHERE x > (SELECT x FROM t1 ORDER BY x LIMIT 1) +---- +hi +howdy + +# Test ordinality. +query TI +SELECT * FROM t2 WITH ORDINALITY ORDER BY x +---- +hello 1 +hello 2 +hello 3 +howdy 4 +hi 5 + +# Test ordering with and without limits. +statement ok +INSERT INTO t1 VALUES ('hi'), ('hello'), ('howdy'), ('howdy'), ('howdy'), ('hello') + +query T +SELECT x FROM t1 ORDER BY x DESC +---- +hi +howdy +howdy +howdy +hello +hello +hello + +query T +SELECT x FROM t1 ORDER BY x ASC +---- +hello +hello +hello +howdy +howdy +howdy +hi + +query T +SELECT x FROM t1 ORDER BY x ASC LIMIT 3 +---- +hello +hello +hello + +query T +SELECT x FROM t1 ORDER BY x DESC LIMIT 3 +---- +hi +howdy +howdy + +# Test we can group on enums. +query T rowsort +(SELECT * FROM t1) UNION (SELECT * FROM t2) +---- +hello +howdy +hi + +statement ok +CREATE TABLE enum_agg (x greeting, y INT); +INSERT INTO enum_agg VALUES + ('hello', 1), + ('hello', 3), + ('howdy', 5), + ('howdy', 0), + ('howdy', 1), + ('hi', 10) + +query TIRI rowsort +SELECT x, max(y), sum(y), min(y) FROM enum_agg GROUP BY x +---- +hello 3 4 1 +howdy 5 6 0 +hi 10 10 10 + + +# Regression to ensure that statistics jobs can be run on tables +# with user defined types. +statement ok +CREATE TABLE greeting_stats (x greeting PRIMARY KEY); +INSERT INTO greeting_stats VALUES ('hi'); +CREATE STATISTICS s FROM greeting_stats + +query T +SELECT x FROM greeting_stats +---- +hi diff --git a/pkg/sql/opt/exec/execbuilder/testdata/enums b/pkg/sql/opt/exec/execbuilder/testdata/enums new file mode 100644 index 000000000000..516b996193d7 --- /dev/null +++ b/pkg/sql/opt/exec/execbuilder/testdata/enums @@ -0,0 +1,54 @@ +# LogicTest: local + +# Note that we use EXPLAIN (opt) in these tests because the standard explain +# prints spans after they have been converted into keys. Once converted into +# keys, enum datums are not human readable. EXPLAIN (OPT) prints these enums +# as datums, so we can more clearly see what spans are being generated. + +statement ok +CREATE TYPE greeting AS ENUM ('hello', 'howdy', 'hi'); +CREATE TABLE t (x greeting PRIMARY KEY, y greeting, INDEX i (y), FAMILY (x, y)); +INSERT INTO t VALUES ('hello', 'howdy'), ('howdy', 'hi') + +query T +EXPLAIN (OPT) SELECT * FROM t WHERE x = 'hello' +---- +scan t + └── constraint: /1: [/'hello' - /'hello'] + +query T +EXPLAIN (OPT) SELECT * FROM t WHERE x = 'hello' OR x = 'hi' +---- +scan t + └── constraint: /1 + ├── [/'hello' - /'hello'] + └── [/'hi' - /'hi'] + +query T +EXPLAIN (OPT) SELECT * FROM t WHERE x > 'hello' +---- +scan t + └── constraint: /1: [/'howdy' - ] + +# Test that we can perform constrained scans using secondary indexes too. +query T +EXPLAIN (OPT) SELECT * FROM t WHERE y = 'hello' +---- +scan t@i + └── constraint: /2/1: [/'hello' - /'hello'] + +query T +EXPLAIN (OPT) SELECT * FROM t WHERE y > 'hello' AND y < 'hi' +---- +scan t@i + └── constraint: /2/1: [/'howdy' - /'howdy'] + +query T +EXPLAIN (opt) SELECT * FROM t WHERE x IN ('hello', 'hi') +---- +scan t + └── constraint: /1 + ├── [/'hello' - /'hello'] + └── [/'hi' - /'hi'] + + diff --git a/pkg/sql/parser/sql.y b/pkg/sql/parser/sql.y index ee20adb29c81..5c0bf3eb6e24 100644 --- a/pkg/sql/parser/sql.y +++ b/pkg/sql/parser/sql.y @@ -7628,6 +7628,11 @@ simple_typename: } } } +| '@' iconst32 + { + id := $2.int32() + $$.val = &tree.IDTypeReference{ID: uint32(id)} + } | complex_type_name { $$.val = $1.typeReference() diff --git a/pkg/sql/resolver.go b/pkg/sql/resolver.go index 04d0719641ad..3feb4de5a7b8 100644 --- a/pkg/sql/resolver.go +++ b/pkg/sql/resolver.go @@ -148,7 +148,7 @@ func (p *planner) ResolveType(name *tree.UnresolvedObjectName) (*types.T, error) } // TODO (rohany): The ResolveAnyDescType argument doesn't do anything here // if we are looking for a type. This should be cleaned up. - desc, prefix, err := resolveExistingObjectImpl(context.Background(), p, name, lookupFlags, ResolveAnyDescType) + desc, prefix, err := resolveExistingObjectImpl(p.EvalContext().Context, p, name, lookupFlags, ResolveAnyDescType) if err != nil { return nil, err } @@ -162,6 +162,7 @@ func (p *planner) ResolveType(name *tree.UnresolvedObjectName) (*types.T, error) if err := tdesc.HydrateTypeInfo(typ); err != nil { return nil, err } + // Override the hydrated name with the fully resolved type name. typ.TypeMeta.Name = &tn return typ, nil default: @@ -169,6 +170,75 @@ func (p *planner) ResolveType(name *tree.UnresolvedObjectName) (*types.T, error) } } +// TODO (rohany): Once we start to cache type descriptors, this needs to +// look into the set of leased copies. +// TODO (rohany): Once we lease types, this should be pushed down into the +// leased object collection. +func (p *planner) getTypeDescByID(ctx context.Context, id sqlbase.ID) (*TypeDescriptor, error) { + rawDesc, err := getDescriptorByID(ctx, p.txn, p.ExecCfg().Codec, id) + if err != nil { + return nil, err + } + typDesc, ok := rawDesc.(*TypeDescriptor) + if !ok { + return nil, errors.AssertionFailedf("%s was not a type descriptor", rawDesc) + } + return typDesc, nil +} + +// ResolveTypeByID implements the tree.TypeResolver interface. We disallow +// accessing types directly by their ID in standard SQL contexts, so error +// out nicely here. +// TODO (rohany): Is there a need to disable this in the general case? +func (p *planner) ResolveTypeByID(id uint32) (*types.T, error) { + return nil, errors.Newf("type id reference @%d not allowed in this context", id) +} + +// maybeHydrateTypesInDescriptor hydrates any types.T's in the input descriptor. +// TODO (rohany): Once we lease types, this should be pushed down into the +// leased object collection. +func (p *planner) maybeHydrateTypesInDescriptor( + ctx context.Context, objDesc tree.NameResolutionResult, +) error { + // Helper method to hydrate the types within a TableDescriptor. + hydrateDesc := func(desc *TableDescriptor) error { + for i := range desc.Columns { + col := &desc.Columns[i] + if col.Type.UserDefined() { + // Look up its type descriptor. + typDesc, err := p.getTypeDescByID(ctx, sqlbase.ID(col.Type.StableTypeID())) + if err != nil { + return err + } + // TODO (rohany): This should be a noop if the hydrated type + // information present in the descriptor has the same version as + // the resolved type descriptor we found here. + // TODO (rohany): Once types are leased we need to create a new + // ImmutableTableDescriptor when a type lease expires rather than + // overwriting the types information in the shared descriptor. + if err := typDesc.HydrateTypeInfo(col.Type); err != nil { + return err + } + } + } + return nil + } + + // As of now, only {Mutable,Immutable}TableDescriptor have types.T that + // need to be hydrated. + switch desc := objDesc.(type) { + case *sqlbase.MutableTableDescriptor: + if err := hydrateDesc(desc.TableDesc()); err != nil { + return err + } + case *sqlbase.ImmutableTableDescriptor: + if err := hydrateDesc(desc.TableDesc()); err != nil { + return err + } + } + return nil +} + func resolveExistingObjectImpl( ctx context.Context, sc SchemaResolver, @@ -354,6 +424,14 @@ func (p *planner) LookupObject( sc := p.LogicalSchemaAccessor() lookupFlags.CommonLookupFlags = p.CommonLookupFlags(false /* required */) objDesc, err := sc.GetObjectDesc(ctx, p.txn, p.ExecCfg().Settings, p.ExecCfg().Codec, dbName, scName, tbName, lookupFlags) + + // The returned object may contain types.T that need hydrating. + if objDesc != nil { + if err := p.maybeHydrateTypesInDescriptor(ctx, objDesc); err != nil { + return false, nil, err + } + } + return objDesc != nil, objDesc, err } diff --git a/pkg/sql/rowexec/sample_aggregator.go b/pkg/sql/rowexec/sample_aggregator.go index a4ffd7d172d8..0e90c4591760 100644 --- a/pkg/sql/rowexec/sample_aggregator.go +++ b/pkg/sql/rowexec/sample_aggregator.go @@ -133,7 +133,7 @@ func newSampleAggregator( s.sr.Init(int(spec.SampleSize), input.OutputTypes()[:rankCol], &s.memAcc, sampleCols) if err := s.Init( - nil, post, []*types.T{}, flowCtx, processorID, output, memMonitor, + nil, post, input.OutputTypes(), flowCtx, processorID, output, memMonitor, execinfra.ProcStateOpts{ TrailingMetaCallback: func(context.Context) []execinfrapb.ProducerMetadata { s.close() diff --git a/pkg/sql/rowexec/windower_test.go b/pkg/sql/rowexec/windower_test.go index bd49c1cb52b2..93589ea6232f 100644 --- a/pkg/sql/rowexec/windower_test.go +++ b/pkg/sql/rowexec/windower_test.go @@ -74,7 +74,7 @@ func TestWindowerAccountingForResults(t *testing.T) { Func: execinfrapb.WindowerSpec_Func{AggregateFunc: &aggSpec}, ArgsIdxs: []uint32{0}, Ordering: execinfrapb.Ordering{Columns: []execinfrapb.Ordering_Column{{ColIdx: 0}}}, - OutputColIdx: 0, + OutputColIdx: 1, FilterColIdx: noFilterIdx, Frame: &execinfrapb.WindowerSpec_Frame{ Mode: execinfrapb.WindowerSpec_Frame_ROWS, @@ -239,6 +239,7 @@ func BenchmarkWindower(b *testing.B) { runName = runName + "ORDER BY" } runName = runName + ")" + spec.WindowFns[0].OutputColIdx = 3 b.Run(runName, func(b *testing.B) { post := &execinfrapb.PostProcessSpec{} diff --git a/pkg/sql/sem/tree/eval.go b/pkg/sql/sem/tree/eval.go index ae06b3a2e93b..ef3e45885e22 100644 --- a/pkg/sql/sem/tree/eval.go +++ b/pkg/sql/sem/tree/eval.go @@ -2909,6 +2909,12 @@ type EvalContext struct { Tenant TenantOperator + // DistSQLTypeResolver is a type resolver used during execution of DistSQL + // flows. It is limited to only provide access to types via ID, meaning that + // it cannot perform resolution of qualified names into types. It will be nil + // when not in the context of a DistSQL flow. + DistSQLTypeResolver TypeReferenceResolver + // The transaction in which the statement is executing. Txn *kv.Txn // A handle to the database. diff --git a/pkg/sql/sem/tree/expr.go b/pkg/sql/sem/tree/expr.go index caa3b9988873..bae0fd686077 100644 --- a/pkg/sql/sem/tree/expr.go +++ b/pkg/sql/sem/tree/expr.go @@ -692,7 +692,7 @@ func (node *IsOfTypeExpr) Format(ctx *FmtCtx) { if i > 0 { ctx.WriteString(", ") } - ctx.Buffer.WriteString(t.SQLString()) + ctx.FormatTypeReference(t) } ctx.WriteByte(')') } @@ -1533,7 +1533,7 @@ func (node *CastExpr) Format(ctx *FmtCtx) { // with string constants; if the underlying expression was changed, we fall // back to the short syntax. if _, ok := node.Expr.(*StrVal); ok { - ctx.WriteString(node.Type.SQLString()) + ctx.FormatTypeReference(node.Type) ctx.WriteByte(' ') ctx.FormatNode(node.Expr) break @@ -1542,7 +1542,7 @@ func (node *CastExpr) Format(ctx *FmtCtx) { case CastShort: exprFmtWithParen(ctx, node.Expr) ctx.WriteString("::") - ctx.WriteString(node.Type.SQLString()) + ctx.FormatTypeReference(node.Type) default: ctx.WriteString("CAST(") ctx.FormatNode(node.Expr) @@ -1561,7 +1561,7 @@ func (node *CastExpr) Format(ctx *FmtCtx) { ctx.WriteString(") COLLATE ") lex.EncodeLocaleName(&ctx.Buffer, typ.Locale()) } else { - ctx.WriteString(node.Type.SQLString()) + ctx.FormatTypeReference(node.Type) ctx.WriteByte(')') } } @@ -1707,7 +1707,7 @@ func (node *AnnotateTypeExpr) Format(ctx *FmtCtx) { case types.StringFamily, types.CollatedStringFamily: // Postgres formats strings using a cast afterward. Let's do the same. ctx.WriteString("::") - ctx.WriteString(node.Type.SQLString()) + ctx.WriteString(typ.SQLString()) } } return @@ -1716,13 +1716,13 @@ func (node *AnnotateTypeExpr) Format(ctx *FmtCtx) { case AnnotateShort: exprFmtWithParen(ctx, node.Expr) ctx.WriteString(":::") - ctx.WriteString(node.Type.SQLString()) + ctx.FormatTypeReference(node.Type) default: ctx.WriteString("ANNOTATE_TYPE(") ctx.FormatNode(node.Expr) ctx.WriteString(", ") - ctx.WriteString(node.Type.SQLString()) + ctx.FormatTypeReference(node.Type) ctx.WriteByte(')') } } diff --git a/pkg/sql/sem/tree/format.go b/pkg/sql/sem/tree/format.go index b5b0ad4b067b..9d81eb813b09 100644 --- a/pkg/sql/sem/tree/format.go +++ b/pkg/sql/sem/tree/format.go @@ -126,6 +126,11 @@ const ( // FmtPGIndexDef is used to produce CREATE INDEX statements that are // compatible with pg_get_indexdef. FmtPGIndexDef + + // If set, user defined types will be printed as '@id', where id is the + // stable type ID for the user defined type. This is used in DistSQL flows + // where we don't want to perform name resolution of types again. + fmtFormatUserDefinedTypesAsIDs ) // Composite/derived flag definitions follow. @@ -150,6 +155,12 @@ const ( // DDecimal 1 and the DInt 1). FmtCheckEquivalence FmtFlags = fmtSymbolicVars | fmtDisambiguateDatumTypes | FmtParsableNumerics + // FmtDistSQLSerialization is just like FmtCheckEquivalence, but it can be + // used to serialize expressions for query distribution. In particular, it + // includes the flag fmtFormatUserDefinedTypesAsIDs which serializes user + // defined types in a way that avoids name resolution for DistSQL evaluation. + FmtDistSQLSerialization FmtFlags = FmtCheckEquivalence | fmtFormatUserDefinedTypesAsIDs + // FmtArrayToString is a special composite flag suitable // for the output of array_to_string(). This de-quotes // the strings enclosed in the array and skips the normal escaping @@ -370,7 +381,7 @@ func (ctx *FmtCtx) FormatNode(n NodeFormatter) { } if typ != nil { ctx.WriteString(":::") - ctx.WriteString(typ.SQLString()) + ctx.FormatTypeReference(typ) } } } diff --git a/pkg/sql/sem/tree/type_name.go b/pkg/sql/sem/tree/type_name.go index 3ebc97ef5e5c..a2d648ac2d70 100644 --- a/pkg/sql/sem/tree/type_name.go +++ b/pkg/sql/sem/tree/type_name.go @@ -11,6 +11,8 @@ package tree import ( + "fmt" + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" "github.com/cockroachdb/cockroach/pkg/sql/types" @@ -92,15 +94,16 @@ func MakeTypeNameFromPrefix(prefix ObjectNamePrefix, object Name) TypeName { // TypeReferenceResolver is the interface that will provide the ability // to actually look up type metadata and transform references into -// *types.T's. In practice, this will probably be implemented by -// the planner, but for now it is a dummy interface. +// *types.T's. type TypeReferenceResolver interface { - // In the future this will take a context. ResolveType(name *UnresolvedObjectName) (*types.T, error) + ResolveTypeByID(id uint32) (*types.T, error) } // ResolvableTypeReference represents a type that is possibly unknown // until type-checking/type name resolution is performed. +// N.B. ResolvableTypeReferences in expressions must be formatted with +// FormatTypeReference instead of SQLString. type ResolvableTypeReference interface { SQLString() string } @@ -108,6 +111,7 @@ type ResolvableTypeReference interface { var _ ResolvableTypeReference = &UnresolvedObjectName{} var _ ResolvableTypeReference = &ArrayTypeReference{} var _ ResolvableTypeReference = &types.T{} +var _ ResolvableTypeReference = &IDTypeReference{} // ResolveType converts a ResolvableTypeReference into a *types.T. func ResolveType(ref ResolvableTypeReference, resolver TypeReferenceResolver) (*types.T, error) { @@ -127,11 +131,31 @@ func ResolveType(ref ResolvableTypeReference, resolver TypeReferenceResolver) (* return nil, pgerror.Newf(pgcode.UndefinedObject, "type %q does not exist", t) } return resolver.ResolveType(t) + case *IDTypeReference: + if resolver == nil { + return nil, pgerror.Newf(pgcode.UndefinedObject, "type id %d does not exist", t.ID) + } + return resolver.ResolveTypeByID(t.ID) default: return nil, errors.AssertionFailedf("unknown resolvable type reference type %s", t) } } +// FormatTypeReference formats a ResolvableTypeReference. +func (ctx *FmtCtx) FormatTypeReference(ref ResolvableTypeReference) { + if ctx.HasFlags(fmtFormatUserDefinedTypesAsIDs) { + switch t := ref.(type) { + case *types.T: + if t.UserDefined() { + idRef := IDTypeReference{ID: t.StableTypeID()} + ctx.WriteString(idRef.SQLString()) + return + } + } + } + ctx.WriteString(ref.SQLString()) +} + // GetStaticallyKnownType possibly promotes a ResolvableTypeReference into a // *types.T if the reference is a statically known type. It is only safe to // access the returned type if ok is true. @@ -151,6 +175,16 @@ func MustBeStaticallyKnownType(ref ResolvableTypeReference) *types.T { panic(errors.AssertionFailedf("type reference was not a statically known type")) } +// IDTypeReference is a reference to a type directly by its stable ID. +type IDTypeReference struct { + ID uint32 +} + +// SQLString implements the ResolvableTypeReference interface. +func (node *IDTypeReference) SQLString() string { + return fmt.Sprintf("@%d", node.ID) +} + // ArrayTypeReference represents an array of possibly unknown type references. type ArrayTypeReference struct { ElementType ResolvableTypeReference @@ -196,6 +230,11 @@ func (dtr *TestingMapTypeResolver) ResolveType(name *UnresolvedObjectName) (*typ return typ, nil } +// ResolveTypeByID implements the TypeReferenceResolver interface. +func (dtr *TestingMapTypeResolver) ResolveTypeByID(uint32) (*types.T, error) { + return nil, errors.AssertionFailedf("unimplemented") +} + // MakeTestingMapTypeResolver creates a TestingMapTypeResolver from a map. func MakeTestingMapTypeResolver(typeMap map[string]*types.T) TypeReferenceResolver { return &TestingMapTypeResolver{typeMap: typeMap} diff --git a/pkg/sql/sqlbase/column_type_encoding.go b/pkg/sql/sqlbase/column_type_encoding.go index 2f89b93f48a8..23987fc6b9d5 100644 --- a/pkg/sql/sqlbase/column_type_encoding.go +++ b/pkg/sql/sqlbase/column_type_encoding.go @@ -160,6 +160,11 @@ func EncodeTableKey(b []byte, val tree.Datum, dir encoding.Direction) ([]byte, e return encoding.EncodeVarintAscending(b, int64(t.DInt)), nil } return encoding.EncodeVarintDescending(b, int64(t.DInt)), nil + case *tree.DEnum: + if dir == encoding.Ascending { + return encoding.EncodeBytesAscending(b, t.PhysicalRep), nil + } + return encoding.EncodeBytesDescending(b, t.PhysicalRep), nil } return nil, errors.Errorf("unable to encode table key: %T", val) } @@ -348,6 +353,17 @@ func DecodeTableKey( rkey, i, err = encoding.DecodeVarintDescending(key) } return a.NewDOid(tree.MakeDOid(tree.DInt(i))), rkey, err + case types.EnumFamily: + var r []byte + if dir == encoding.Ascending { + rkey, r, err = encoding.DecodeBytesAscending(key, nil) + } else { + rkey, r, err = encoding.DecodeBytesDescending(key, nil) + } + if err != nil { + return nil, nil, err + } + return tree.MakeDEnumFromPhysicalRepresentation(valType, r), rkey, nil default: return nil, nil, errors.Errorf("unable to decode table key: %s", valType) } @@ -725,6 +741,11 @@ func MarshalColumnValue(col *ColumnDescriptor, val tree.Datum) (roachpb.Value, e r.SetInt(int64(v.DInt)) return r, nil } + case types.EnumFamily: + if v, ok := val.(*tree.DEnum); ok { + r.SetBytes(v.PhysicalRep) + return r, nil + } default: return r, errors.AssertionFailedf("unsupported column type: %s", col.Type.Family()) } @@ -887,6 +908,12 @@ func UnmarshalColumnValue(a *DatumAlloc, typ *types.T, value roachpb.Value) (tre return nil, err } return tree.NewDJSON(jsonDatum), nil + case types.EnumFamily: + v, err := value.GetBytes() + if err != nil { + return nil, err + } + return tree.MakeDEnumFromPhysicalRepresentation(typ, v), nil default: return nil, errors.Errorf("unsupported column type: %s", typ.Family()) } diff --git a/pkg/sql/sqlbase/structured.go b/pkg/sql/sqlbase/structured.go index c3973f5d858b..c8b0fb4ce35a 100644 --- a/pkg/sql/sqlbase/structured.go +++ b/pkg/sql/sqlbase/structured.go @@ -355,6 +355,25 @@ func GetDatabaseDescFromID( return db, nil } +// GetTypeDescFromID retrieves the type descriptor for the type ID passed +// in using an existing proto getter. It returns an error if the descriptor +// doesn't exist or if it exists and is not a type descriptor. +func GetTypeDescFromID( + ctx context.Context, protoGetter protoGetter, codec keys.SQLCodec, id ID, +) (*TypeDescriptor, error) { + descKey := MakeDescMetadataKey(codec, id) + desc := &Descriptor{} + _, err := protoGetter.GetProtoTs(ctx, descKey, desc) + if err != nil { + return nil, err + } + typ := desc.GetType() + if typ == nil { + return nil, ErrDescriptorNotFound + } + return typ, nil +} + // GetTableDescFromID retrieves the table descriptor for the table // ID passed in using an existing proto getter. Returns an error if the // descriptor doesn't exist or if it exists and is not a table. @@ -4138,6 +4157,7 @@ func (desc *TypeDescriptor) SetName(name string) { // ImmutableTypeDescriptor so that pointers to the cached info // can be shared among callers. func (desc *TypeDescriptor) HydrateTypeInfo(typ *types.T) error { + typ.TypeMeta.Name = tree.NewUnqualifiedTypeName(tree.Name(desc.Name)) switch desc.Kind { case TypeDescriptor_ENUM: if typ.Family() != types.EnumFamily { diff --git a/pkg/sql/sqlbase/table.go b/pkg/sql/sqlbase/table.go index 0b35d1e43c8e..a41883d22d31 100644 --- a/pkg/sql/sqlbase/table.go +++ b/pkg/sql/sqlbase/table.go @@ -104,7 +104,7 @@ func ValidateColumnDefType(t *types.T) error { case types.BitFamily, types.IntFamily, types.FloatFamily, types.BoolFamily, types.BytesFamily, types.DateFamily, types.INetFamily, types.IntervalFamily, types.JsonFamily, types.OidFamily, types.TimeFamily, types.TimestampFamily, types.TimestampTZFamily, types.UuidFamily, types.TimeTZFamily, - types.GeographyFamily, types.GeometryFamily: + types.GeographyFamily, types.GeometryFamily, types.EnumFamily: // These types are OK. default: diff --git a/pkg/sql/stats/automatic_stats_test.go b/pkg/sql/stats/automatic_stats_test.go index df909b0b39bc..89aa55edef5a 100644 --- a/pkg/sql/stats/automatic_stats_test.go +++ b/pkg/sql/stats/automatic_stats_test.go @@ -60,6 +60,7 @@ func TestMaybeRefreshStats(t *testing.T) { gossip.MakeExposedGossip(s.GossipI().(*gossip.Gossip)), kvDB, executor, + keys.SystemSQLCodec, ) refresher := MakeRefresher(st, executor, cache, time.Microsecond /* asOfTime */) @@ -135,6 +136,7 @@ func TestAverageRefreshTime(t *testing.T) { gossip.MakeExposedGossip(s.GossipI().(*gossip.Gossip)), kvDB, executor, + keys.SystemSQLCodec, ) refresher := MakeRefresher(st, executor, cache, time.Microsecond /* asOfTime */) @@ -365,6 +367,7 @@ func TestAutoStatsReadOnlyTables(t *testing.T) { gossip.MakeExposedGossip(s.GossipI().(*gossip.Gossip)), kvDB, executor, + keys.SystemSQLCodec, ) refresher := MakeRefresher(st, executor, cache, time.Microsecond /* asOfTime */) @@ -401,6 +404,7 @@ func TestNoRetryOnFailure(t *testing.T) { gossip.MakeExposedGossip(s.GossipI().(*gossip.Gossip)), kvDB, executor, + keys.SystemSQLCodec, ) r := MakeRefresher(st, executor, cache, time.Microsecond /* asOfTime */) diff --git a/pkg/sql/stats/delete_stats_test.go b/pkg/sql/stats/delete_stats_test.go index ef396afbb74e..4fb1b3147d67 100644 --- a/pkg/sql/stats/delete_stats_test.go +++ b/pkg/sql/stats/delete_stats_test.go @@ -19,6 +19,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/base" "github.com/cockroachdb/cockroach/pkg/gossip" + "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/kv" "github.com/cockroachdb/cockroach/pkg/sql/sqlbase" "github.com/cockroachdb/cockroach/pkg/sql/sqlutil" @@ -39,6 +40,7 @@ func TestDeleteOldStatsForColumns(t *testing.T) { gossip.MakeExposedGossip(s.GossipI().(*gossip.Gossip)), db, ex, + keys.SystemSQLCodec, ) // The test data must be ordered by CreatedAt DESC so the calculated set of diff --git a/pkg/sql/stats/gossip_invalidation_test.go b/pkg/sql/stats/gossip_invalidation_test.go index dc225d748d9e..e17aa5e9b9d8 100644 --- a/pkg/sql/stats/gossip_invalidation_test.go +++ b/pkg/sql/stats/gossip_invalidation_test.go @@ -41,6 +41,7 @@ func TestGossipInvalidation(t *testing.T) { gossip.MakeExposedGossip(tc.Server(0).GossipI().(*gossip.Gossip)), tc.Server(0).DB(), tc.Server(0).InternalExecutor().(sqlutil.InternalExecutor), + keys.SystemSQLCodec, ) sr0 := sqlutils.MakeSQLRunner(tc.ServerConn(0)) diff --git a/pkg/sql/stats/stats_cache.go b/pkg/sql/stats/stats_cache.go index 6843da69e102..60146b272e46 100644 --- a/pkg/sql/stats/stats_cache.go +++ b/pkg/sql/stats/stats_cache.go @@ -15,6 +15,7 @@ import ( "sync" "github.com/cockroachdb/cockroach/pkg/gossip" + "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/kv" "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/sql/opt/cat" @@ -57,6 +58,7 @@ type TableStatisticsCache struct { } ClientDB *kv.DB SQLExecutor sqlutil.InternalExecutor + Codec keys.SQLCodec } // The cache stores *cacheEntry objects. The fields are protected by the @@ -76,11 +78,16 @@ type cacheEntry struct { // NewTableStatisticsCache creates a new TableStatisticsCache that can hold // statistics for tables. func NewTableStatisticsCache( - cacheSize int, gw gossip.DeprecatedGossip, db *kv.DB, sqlExecutor sqlutil.InternalExecutor, + cacheSize int, + gw gossip.DeprecatedGossip, + db *kv.DB, + sqlExecutor sqlutil.InternalExecutor, + codec keys.SQLCodec, ) *TableStatisticsCache { tableStatsCache := &TableStatisticsCache{ ClientDB: db, SQLExecutor: sqlExecutor, + Codec: codec, } tableStatsCache.mu.cache = cache.NewUnorderedCache(cache.Config{ Policy: cache.CacheLRU, @@ -236,8 +243,11 @@ const ( statsLen ) -// parseStats converts the given datums to a TableStatistic object. -func parseStats(datums tree.Datums) (*TableStatistic, error) { +// parseStats converts the given datums to a TableStatistic object. It might +// need to run a query to get user defined type metadata. +func parseStats( + ctx context.Context, db *kv.DB, codec keys.SQLCodec, datums tree.Datums, +) (*TableStatistic, error) { if datums == nil || datums.Len() == 0 { return nil, nil } @@ -303,6 +313,29 @@ func parseStats(datums tree.Datums) (*TableStatistic, error) { // Decode the histogram data so that it's usable by the opt catalog. res.Histogram = make([]cat.HistogramBucket, len(res.HistogramData.Buckets)) typ := res.HistogramData.ColumnType + // Hydrate the type in case any user defined types are present. + // There are cases where typ is nil, so don't do anything if so. + if typ != nil && typ.UserDefined() { + // TODO (rohany): This should instead query a leased copy of the type. + // TODO (rohany): If we are caching data about types here, then this + // cache needs to be invalidated as well when type metadata changes. + // TODO (rohany): It might be better to store the type metadata used when + // collecting the stats in the HistogramData object itself, and avoid + // this query and caching/leasing problem. + // The metadata accessed here is never older than the metadata used when + // collecting the stats. Changes to types are backwards compatible across + // versions, so using a newer version of the type metadata here is safe. + err := db.Txn(ctx, func(ctx context.Context, txn *kv.Txn) error { + typDesc, err := sqlbase.GetTypeDescFromID(ctx, txn, codec, sqlbase.ID(typ.StableTypeID())) + if err != nil { + return err + } + return typDesc.HydrateTypeInfo(typ) + }) + if err != nil { + return nil, err + } + } var a sqlbase.DatumAlloc for i := range res.Histogram { bucket := &res.HistogramData.Buckets[i] @@ -351,7 +384,7 @@ ORDER BY "createdAt" DESC var statsList []*TableStatistic for _, row := range rows { - stats, err := parseStats(row) + stats, err := parseStats(ctx, sc.ClientDB, sc.Codec, row) if err != nil { return nil, err } diff --git a/pkg/sql/stats/stats_cache_test.go b/pkg/sql/stats/stats_cache_test.go index 9a099e3fa5f5..33be0a5ec99a 100644 --- a/pkg/sql/stats/stats_cache_test.go +++ b/pkg/sql/stats/stats_cache_test.go @@ -21,6 +21,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/base" "github.com/cockroachdb/cockroach/pkg/gossip" + "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/kv" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sqlbase" @@ -230,6 +231,7 @@ func TestCacheBasic(t *testing.T) { gossip.MakeExposedGossip(s.GossipI().(*gossip.Gossip)), db, ex, + keys.SystemSQLCodec, ) for _, tableID := range tableIDs { if err := checkStatsForTable(ctx, sc, expectedStats[tableID], tableID); err != nil { @@ -261,6 +263,40 @@ func TestCacheBasic(t *testing.T) { } } +func TestCacheUserDefinedTypes(t *testing.T) { + defer leaktest.AfterTest(t)() + ctx := context.Background() + s, sqlDB, kvDB := serverutils.StartServer(t, base.TestServerArgs{}) + defer s.Stopper().Stop(ctx) + + if _, err := sqlDB.Exec(` +CREATE DATABASE t; +USE t; +CREATE TYPE t AS ENUM ('hello'); +CREATE TABLE tt (x t PRIMARY KEY); +INSERT INTO tt VALUES ('hello'); +CREATE STATISTICS s FROM tt; +`); err != nil { + t.Fatal(err) + } + _ = kvDB + // Make a stats cache. + sc := NewTableStatisticsCache( + 1, + gossip.MakeExposedGossip(s.GossipI().(*gossip.Gossip)), + kvDB, + s.InternalExecutor().(sqlutil.InternalExecutor), + keys.SystemSQLCodec, + ) + tbl := sqlbase.GetTableDescriptor(kvDB, keys.SystemSQLCodec, "t", "tt") + // Get stats for our table. We are ensuring here that the access to the stats + // for tt properly hydrates the user defined type t before access. + _, err := sc.GetTableStats(ctx, tbl.ID) + if err != nil { + t.Fatal(err) + } +} + // TestCacheWait verifies that when a table gets invalidated, we only retrieve // the stats one time, even if there are multiple callers asking for them. func TestCacheWait(t *testing.T) { @@ -288,6 +324,7 @@ func TestCacheWait(t *testing.T) { gossip.MakeExposedGossip(s.GossipI().(*gossip.Gossip)), db, ex, + keys.SystemSQLCodec, ) for _, tableID := range tableIDs { if err := checkStatsForTable(ctx, sc, expectedStats[tableID], tableID); err != nil { diff --git a/pkg/sql/types/types.go b/pkg/sql/types/types.go index 06d65aa0a18e..4fa5378b4f15 100644 --- a/pkg/sql/types/types.go +++ b/pkg/sql/types/types.go @@ -1572,6 +1572,9 @@ func (t *T) SQLString() string { } return t.ArrayContents().SQLString() + "[]" case EnumFamily: + if t.Oid() == oid.T_anyenum { + return "anyenum" + } return t.TypeMeta.Name.FQName() } return strings.ToUpper(t.Name())