diff --git a/pkg/server/server.go b/pkg/server/server.go index 062b682322d6..4a84684d58c5 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -638,7 +638,6 @@ func NewServer(cfg Config, stopper *stop.Stopper) (*Server, error) { sAdmin := newAdminServer(lateBoundServer, adminAuthzCheck, internalExecutor) sHTTP := newHTTPServer(cfg) sessionRegistry := sql.NewSessionRegistry() - contentionRegistry := contention.NewRegistry() flowScheduler := flowinfra.NewFlowScheduler(cfg.AmbientCtx, stopper, st) sStatus := newStatusServer( @@ -656,10 +655,11 @@ func NewServer(cfg Config, stopper *stop.Stopper) (*Server, error) { node.stores, stopper, sessionRegistry, - contentionRegistry, flowScheduler, internalExecutor, ) + + contentionRegistry := contention.NewRegistry() // TODO(tbg): don't pass all of Server into this to avoid this hack. sAuth := newAuthenticationServer(lateBoundServer) for i, gw := range []grpcGatewayServer{sAdmin, sStatus, sAuth, &sTS} { diff --git a/pkg/server/status.go b/pkg/server/status.go index 57ea7e7cc26f..3104a53e56dd 100644 --- a/pkg/server/status.go +++ b/pkg/server/status.go @@ -135,14 +135,13 @@ type baseStatusServer struct { serverpb.UnimplementedStatusServer log.AmbientContext - privilegeChecker *adminPrivilegeChecker - sessionRegistry *sql.SessionRegistry - contentionRegistry *contention.Registry - flowScheduler *flowinfra.FlowScheduler - st *cluster.Settings - sqlServer *SQLServer - rpcCtx *rpc.Context - stopper *stop.Stopper + privilegeChecker *adminPrivilegeChecker + sessionRegistry *sql.SessionRegistry + flowScheduler *flowinfra.FlowScheduler + st *cluster.Settings + sqlServer *SQLServer + rpcCtx *rpc.Context + stopper *stop.Stopper } // getLocalSessions returns a list of local sessions on this node. Note that the @@ -307,7 +306,7 @@ func (b *baseStatusServer) ListLocalContentionEvents( } return &serverpb.ListContentionEventsResponse{ - Events: b.contentionRegistry.Serialize(), + Events: b.sqlServer.execCfg.ContentionRegistry.Serialize(), }, nil } @@ -415,21 +414,19 @@ func newStatusServer( stores *kvserver.Stores, stopper *stop.Stopper, sessionRegistry *sql.SessionRegistry, - contentionRegistry *contention.Registry, flowScheduler *flowinfra.FlowScheduler, internalExecutor *sql.InternalExecutor, ) *statusServer { ambient.AddLogTag("status", nil) server := &statusServer{ baseStatusServer: &baseStatusServer{ - AmbientContext: ambient, - privilegeChecker: adminAuthzCheck, - sessionRegistry: sessionRegistry, - contentionRegistry: contentionRegistry, - flowScheduler: flowScheduler, - st: st, - rpcCtx: rpcCtx, - stopper: stopper, + AmbientContext: ambient, + privilegeChecker: adminAuthzCheck, + sessionRegistry: sessionRegistry, + flowScheduler: flowScheduler, + st: st, + rpcCtx: rpcCtx, + stopper: stopper, }, cfg: cfg, admin: adminServer, diff --git a/pkg/server/tenant.go b/pkg/server/tenant.go index 5505304a5698..4341fa7d8be0 100644 --- a/pkg/server/tenant.go +++ b/pkg/server/tenant.go @@ -165,9 +165,10 @@ func StartTenant( // the SQL server object. tenantStatusServer := newTenantStatusServer( baseCfg.AmbientCtx, &adminPrivilegeChecker{ie: args.circularInternalExecutor}, - args.sessionRegistry, args.contentionRegistry, args.flowScheduler, baseCfg.Settings, nil, + args.sessionRegistry, args.flowScheduler, baseCfg.Settings, nil, args.rpcContext, args.stopper, ) + args.contentionRegistry = contention.NewRegistry() args.sqlStatusServer = tenantStatusServer s, err := newSQLServer(ctx, args) tenantStatusServer.sqlServer = s @@ -499,7 +500,6 @@ func makeTenantSQLServerArgs( // writing): the blob service and DistSQL. dummyRPCServer := rpc.NewServer(rpcContext) sessionRegistry := sql.NewSessionRegistry() - contentionRegistry := contention.NewRegistry() flowScheduler := flowinfra.NewFlowScheduler(baseCfg.AmbientCtx, stopper, st) return sqlServerArgs{ sqlServerOptionalKVArgs: sqlServerOptionalKVArgs{ @@ -535,7 +535,6 @@ func makeTenantSQLServerArgs( registry: registry, recorder: recorder, sessionRegistry: sessionRegistry, - contentionRegistry: contentionRegistry, flowScheduler: flowScheduler, circularInternalExecutor: circularInternalExecutor, circularJobRegistry: circularJobRegistry, diff --git a/pkg/server/tenant_status.go b/pkg/server/tenant_status.go index 67a317536c37..b12043d82961 100644 --- a/pkg/server/tenant_status.go +++ b/pkg/server/tenant_status.go @@ -77,7 +77,6 @@ func newTenantStatusServer( ambient log.AmbientContext, privilegeChecker *adminPrivilegeChecker, sessionRegistry *sql.SessionRegistry, - contentionRegistry *contention.Registry, flowScheduler *flowinfra.FlowScheduler, st *cluster.Settings, sqlServer *SQLServer, @@ -87,15 +86,14 @@ func newTenantStatusServer( ambient.AddLogTag("tenant-status", nil) return &tenantStatusServer{ baseStatusServer: baseStatusServer{ - AmbientContext: ambient, - privilegeChecker: privilegeChecker, - sessionRegistry: sessionRegistry, - contentionRegistry: contentionRegistry, - flowScheduler: flowScheduler, - st: st, - sqlServer: sqlServer, - rpcCtx: rpcCtx, - stopper: stopper, + AmbientContext: ambient, + privilegeChecker: privilegeChecker, + sessionRegistry: sessionRegistry, + flowScheduler: flowScheduler, + st: st, + sqlServer: sqlServer, + rpcCtx: rpcCtx, + stopper: stopper, }, } } diff --git a/pkg/sql/types/types.go b/pkg/sql/types/types.go index 77731c5c9554..4730f087e731 100644 --- a/pkg/sql/types/types.go +++ b/pkg/sql/types/types.go @@ -843,6 +843,15 @@ func MakeChar(width int32) *T { Family: StringFamily, Oid: oid.T_bpchar, Width: width, Locale: &emptyLocale}} } +// oidCanBeCollatedString returns true if the given oid is can be a CollatedString. +func oidCanBeCollatedString(o oid.Oid) bool { + switch o { + case oid.T_text, oid.T_varchar, oid.T_bpchar, oid.T_char, oid.T_name: + return true + } + return false +} + // MakeCollatedString constructs a new instance of a CollatedStringFamily type // that is collated according to the given locale. The new type is based upon // the given string type, having the same oid and width values. For example: @@ -851,8 +860,7 @@ func MakeChar(width int32) *T { // VARCHAR(20) => VARCHAR(20) COLLATE EN // func MakeCollatedString(strType *T, locale string) *T { - switch strType.Oid() { - case oid.T_text, oid.T_varchar, oid.T_bpchar, oid.T_char, oid.T_name: + if oidCanBeCollatedString(strType.Oid()) { return &T{InternalType: InternalType{ Family: CollatedStringFamily, Oid: strType.Oid(), Width: strType.Width(), Locale: &locale}} } @@ -1278,50 +1286,20 @@ func (t *T) WithoutTypeModifiers() *T { return t } - switch t.Oid() { - case oid.T_bit: - return MakeBit(0) - case oid.T_bpchar, oid.T_char, oid.T_text, oid.T_varchar: - // For string-like types, we copy the type and set the width to 0 rather - // than returning typeBpChar, typeQChar, VarChar, or String so that - // we retain the locale value if the type is collated. + // For types that can be a collated string, we copy the type and set the width + // to 0 rather than returning the default OidToType type so that we retain the + // locale value if the type is collated. + if oidCanBeCollatedString(t.Oid()) { newT := *t newT.InternalType.Width = 0 return &newT - case oid.T_interval: - return Interval - case oid.T_numeric: - return Decimal - case oid.T_time: - return Time - case oid.T_timestamp: - return Timestamp - case oid.T_timestamptz: - return TimestampTZ - case oid.T_timetz: - return TimeTZ - case oid.T_varbit: - return VarBit - case oid.T_anyelement, - oid.T_bool, - oid.T_bytea, - oid.T_date, - oidext.T_box2d, - oid.T_float4, oid.T_float8, - oidext.T_geography, oidext.T_geometry, - oid.T_inet, - oid.T_int2, oid.T_int4, oid.T_int8, - oid.T_jsonb, - oid.T_name, - oid.T_oid, - oid.T_regclass, oid.T_regnamespace, oid.T_regproc, oid.T_regprocedure, oid.T_regrole, oid.T_regtype, - oid.T_unknown, - oid.T_uuid, - oid.T_void: - return t - default: + } + + t, ok := OidToType[t.Oid()] + if !ok { panic(errors.AssertionFailedf("unexpected OID: %d", t.Oid())) } + return t } // Scale is an alias method for Width, used for clarity for types in diff --git a/pkg/sql/types/types_test.go b/pkg/sql/types/types_test.go index 057497e27e62..3d5d38d8ba6a 100644 --- a/pkg/sql/types/types_test.go +++ b/pkg/sql/types/types_test.go @@ -1006,6 +1006,8 @@ func TestWithoutTypeModifiers(t *testing.T) { {MakeArray(MakeDecimal(5, 1)), DecimalArray}, {MakeTuple([]*T{MakeString(2), Time, MakeDecimal(5, 1)}), MakeTuple([]*T{String, Time, Decimal})}, + {MakeGeography(geopb.ShapeType_Point, 3857), Geography}, + {MakeGeometry(geopb.ShapeType_PointZ, 4326), Geometry}, // Types without modifiers. {Bool, Bool}, @@ -1026,8 +1028,10 @@ func TestWithoutTypeModifiers(t *testing.T) { } for _, tc := range testCases { - if actual := tc.t.WithoutTypeModifiers(); !actual.Identical(tc.expected) { - t.Errorf("expected <%v>, got <%v>", tc.expected.DebugString(), actual.DebugString()) - } + t.Run(tc.t.SQLString(), func(t *testing.T) { + if actual := tc.t.WithoutTypeModifiers(); !actual.Identical(tc.expected) { + t.Errorf("expected <%v>, got <%v>", tc.expected.DebugString(), actual.DebugString()) + } + }) } }