Skip to content

Commit

Permalink
Merge #77260
Browse files Browse the repository at this point in the history
77260: types: remove no longer used version gate r=yuzefovich a=yuzefovich

This is done to break the dependency of `sql/types` on `roachpb` (which
is a part of the effort to clean up the dependencies of `execgen`). This
commit brings the dependencies of `execgen` to a much more reasonable
set.

Although there is an ask in place to not remove this code even if it's
not used at the moment (so that we have the necessary infra in place
if/when the need arises), in order to keep the functionality we would
need to pull out `roachpb.Version` out of `roachpb`, and I don't want
to embark on that since we're in stability period.

Fixes: #77234.

Release note: None

Release justification: low risk change to clean up the dependencies.

Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
  • Loading branch information
craig[bot] and yuzefovich committed Mar 4, 2022
2 parents 0e1edf3 + f09a36d commit 3160bcf
Show file tree
Hide file tree
Showing 12 changed files with 70 additions and 131 deletions.
1 change: 1 addition & 0 deletions pkg/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ ALL_TESTS = [
"//pkg/sql/colexec/colexectestutils:colexectestutils_test",
"//pkg/sql/colexec/colexecutils:colexecutils_test",
"//pkg/sql/colexec/colexecwindow:colexecwindow_test",
"//pkg/sql/colexec/execgen/cmd/execgen:execgen_test",
"//pkg/sql/colexec/execgen:execgen_test",
"//pkg/sql/colexec:colexec_test",
"//pkg/sql/colexecerror:colexecerror_test",
Expand Down
4 changes: 0 additions & 4 deletions pkg/sql/add_column.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,6 @@ func (p *planner) addColumnImpl(
)
}

if err := checkTypeIsSupported(params.ctx, params.ExecCfg().Settings, toType); err != nil {
return err
}

var colOwnedSeqDesc *tabledesc.Mutable
newDef, seqPrefix, seqName, seqOpts, err := params.p.processSerialLikeInColumnDef(params.ctx, d, tn)
if err != nil {
Expand Down
4 changes: 0 additions & 4 deletions pkg/sql/alter_column_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,6 @@ func AlterColumnType(
return err
}

if err := checkTypeIsSupported(ctx, params.ExecCfg().Settings, typ); err != nil {
return err
}

// Special handling for STRING COLLATE xy to verify that we recognize the language.
if t.Collation != "" {
if types.IsStringType(typ) {
Expand Down
8 changes: 0 additions & 8 deletions pkg/sql/colexec/dep_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,4 @@ func TestNoLinkForbidden(t *testing.T) {
"github.com/cockroachdb/cockroach/pkg/sql/rowflow",
}, nil,
)
buildutil.VerifyNoImports(t,
"github.com/cockroachdb/cockroach/pkg/sql/colexec/execgen/cmd/execgen", true,
[]string{
"github.com/cockroachdb/cockroach/pkg/sql/catalog",
"github.com/cockroachdb/cockroach/pkg/sql/execinfrapb",
"github.com/cockroachdb/cockroach/pkg/sql/tree",
}, nil,
)
}
9 changes: 8 additions & 1 deletion pkg/sql/colexec/execgen/cmd/execgen/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library")
load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library", "go_test")

go_library(
name = "execgen_lib",
Expand Down Expand Up @@ -83,3 +83,10 @@ go_binary(
embed = [":execgen_lib"],
visibility = ["//visibility:public"],
)

go_test(
name = "execgen_test",
srcs = ["dep_test.go"],
embed = [":execgen_lib"],
deps = ["//pkg/testutils/buildutil"],
)
29 changes: 29 additions & 0 deletions pkg/sql/colexec/execgen/cmd/execgen/dep_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright 2022 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

package main

import (
"testing"

"github.com/cockroachdb/cockroach/pkg/testutils/buildutil"
)

func TestNoLinkForbidden(t *testing.T) {
buildutil.VerifyNoImports(t,
"github.com/cockroachdb/cockroach/pkg/sql/colexec/execgen/cmd/execgen", true,
[]string{
"github.com/cockroachdb/cockroach/pkg/roachpb",
"github.com/cockroachdb/cockroach/pkg/sql/catalog",
"github.com/cockroachdb/cockroach/pkg/sql/execinfrapb",
"github.com/cockroachdb/cockroach/pkg/sql/tree",
}, nil,
)
}
15 changes: 0 additions & 15 deletions pkg/sql/create_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -1533,9 +1533,6 @@ func NewTableDesc(
)
}
}
if err := checkTypeIsSupported(ctx, st, defType); err != nil {
return nil, err
}
if d.PrimaryKey.Sharded {
if n.PartitionByTable.ContainsPartitions() && !n.PartitionByTable.All {
return nil, pgerror.New(pgcode.FeatureNotSupported, "hash sharded indexes cannot be explicitly partitioned")
Expand Down Expand Up @@ -2777,18 +2774,6 @@ func regionalByRowDefaultColDef(
return c
}

func checkTypeIsSupported(ctx context.Context, settings *cluster.Settings, typ *types.T) error {
version := settings.Version.ActiveVersionOrEmpty(ctx)
if supported := types.IsTypeSupportedInVersion(version, typ); !supported {
return pgerror.Newf(
pgcode.FeatureNotSupported,
"type %s is not supported until version upgrade is finalized",
typ.SQLString(),
)
}
return nil
}

// setSequenceOwner adds sequence id to the sequence id list owned by a column
// and set ownership values of sequence options.
func setSequenceOwner(
Expand Down
16 changes: 3 additions & 13 deletions pkg/sql/schemachanger/scbuild/builder_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,9 @@
package scbuild

import (
"context"
"strings"

"github.com/cockroachdb/cockroach/pkg/keys"
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
"github.com/cockroachdb/cockroach/pkg/sql/catalog"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catconstants"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb"
Expand Down Expand Up @@ -298,18 +296,10 @@ func (b *builderState) ResolveTypeRef(ref tree.ResolvableTypeReference) scpb.Typ
if err != nil {
panic(err)
}
return newTypeT(b.ctx, b.clusterSettings, toType)
return newTypeT(toType)
}

func newTypeT(ctx context.Context, settings *cluster.Settings, t *types.T) scpb.TypeT {
version := settings.Version.ActiveVersionOrEmpty(ctx)
supported := types.IsTypeSupportedInVersion(version, t)
if !supported {
panic(pgerror.Newf(pgcode.FeatureNotSupported,
"type %s is not supported until version upgrade is finalized", t.SQLString(),
))
}

func newTypeT(t *types.T) scpb.TypeT {
m, err := typedesc.GetTypeDescriptorClosure(t)
if err != nil {
panic(err)
Expand Down Expand Up @@ -416,7 +406,7 @@ func (b *builderState) ComputedColumnExpression(
if err != nil {
panic(err)
}
return parsedExpr, newTypeT(b.ctx, b.clusterSettings, typ)
return parsedExpr, newTypeT(typ)
}

var _ scbuildstmt.ElementReferences = (*builderState)(nil)
Expand Down
8 changes: 2 additions & 6 deletions pkg/sql/types/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ go_library(
name = "types",
srcs = [
"alias.go",
"minimum_type_version.go",
"oid.go",
"testutils.go",
"types.go",
Expand All @@ -16,7 +15,6 @@ go_library(
importpath = "github.com/cockroachdb/cockroach/pkg/sql/types",
visibility = ["//visibility:public"],
deps = [
"//pkg/clusterversion",
"//pkg/geo/geopb",
"//pkg/sql/lex",
"//pkg/sql/oidext",
Expand All @@ -35,19 +33,17 @@ go_test(
name = "types_test",
size = "small",
srcs = [
"minimum_type_version_test.go",
"dep_test.go",
"types_test.go",
"types_text_marshal_test.go",
],
embed = [":types"],
deps = [
"//pkg/clusterversion",
"//pkg/geo/geopb",
"//pkg/sql/catalog/descpb",
"//pkg/sql/catalog/typedesc",
"//pkg/sql/oidext",
"//pkg/util/leaktest",
"//pkg/util/log",
"//pkg/testutils/buildutil",
"//pkg/util/protoutil",
"@com_github_lib_pq//oid",
"@com_github_stretchr_testify//assert",
Expand Down
27 changes: 27 additions & 0 deletions pkg/sql/types/dep_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright 2022 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

package types

import (
"testing"

"github.com/cockroachdb/cockroach/pkg/testutils/buildutil"
)

func TestNoLinkForbidden(t *testing.T) {
buildutil.VerifyNoImports(t,
"github.com/cockroachdb/cockroach/pkg/sql/types", true,
[]string{
"github.com/cockroachdb/cockroach/pkg/clusterversion",
"github.com/cockroachdb/cockroach/pkg/roachpb",
}, nil,
)
}
34 changes: 0 additions & 34 deletions pkg/sql/types/minimum_type_version.go

This file was deleted.

46 changes: 0 additions & 46 deletions pkg/sql/types/minimum_type_version_test.go

This file was deleted.

0 comments on commit 3160bcf

Please sign in to comment.