Skip to content

Commit

Permalink
Merge #60748
Browse files Browse the repository at this point in the history
60748: sql: remove experimental setting for virtual columns r=RaduBerinde a=RaduBerinde

Informs #57608.

Release note (sql change): CockroachDB now supports VIRTUAL computed
columns (as opposed to STORED). These are computed columns that are
not stored in the primary index and are recomputed as necessary.

Co-authored-by: Radu Berinde <[email protected]>
  • Loading branch information
craig[bot] and RaduBerinde committed Feb 19, 2021
2 parents 83e70ce + 0482ae2 commit 715a190
Show file tree
Hide file tree
Showing 17 changed files with 6 additions and 67 deletions.
4 changes: 0 additions & 4 deletions pkg/sql/add_column.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
"github.com/cockroachdb/cockroach/pkg/sql/sqlerrors"
"github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented"
"github.com/cockroachdb/errors"
)

Expand Down Expand Up @@ -128,9 +127,6 @@ func (p *planner) addColumnImpl(
}

if d.IsComputed() {
if d.IsVirtual() && !sessionData.VirtualColumnsEnabled {
return unimplemented.NewWithIssue(57608, "virtual computed columns")
}
computedColValidator := schemaexpr.MakeComputedColumnValidator(
params.ctx,
n.tableDesc,
Expand Down
3 changes: 0 additions & 3 deletions pkg/sql/create_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -1704,9 +1704,6 @@ func NewTableDesc(
columnDefaultExprs = append(columnDefaultExprs, nil)
}
if d.IsVirtual() {
if !sessionData.VirtualColumnsEnabled {
return nil, unimplemented.NewWithIssue(57608, "virtual computed columns")
}
if !evalCtx.Settings.Version.IsActive(ctx, clusterversion.VirtualComputedColumns) {
return nil, pgerror.Newf(pgcode.FeatureNotSupported,
"version %v must be finalized to use virtual columns",
Expand Down
5 changes: 0 additions & 5 deletions pkg/sql/exec_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -2276,11 +2276,6 @@ func (m *sessionDataMutator) SetAlterColumnTypeGeneral(val bool) {
m.data.AlterColumnTypeGeneralEnabled = val
}

// TODO(radu): remove this once the feature is stable.
func (m *sessionDataMutator) SetVirtualColumnsEnabled(val bool) {
m.data.VirtualColumnsEnabled = val
}

// TODO(rytaft): remove this once unique without index constraints are fully
// supported.
func (m *sessionDataMutator) SetUniqueWithoutIndexConstraints(val bool) {
Expand Down
1 change: 0 additions & 1 deletion pkg/sql/indexbackfiller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,6 @@ func TestIndexBackfillerComputedAndGeneratedColumns(t *testing.T) {
{
name: "virtual computed column in key",
setupSQL: `
SET experimental_enable_virtual_columns = true;
CREATE TABLE foo (i INT PRIMARY KEY, k INT, v INT AS (i*i + k) VIRTUAL);
INSERT INTO foo VALUES (1, 2), (2, 3), (3, 4);
`,
Expand Down
15 changes: 5 additions & 10 deletions pkg/sql/logictest/testdata/logic_test/computed
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
# Note: this file does not contain tests for virtual computed columns; those
# are in a separate test file.

statement ok
CREATE TABLE with_no_column_refs (
a INT,
Expand Down Expand Up @@ -267,27 +270,19 @@ DROP TABLE x
# statement ok
# DROP TABLE x

statement error use AS \( <expr> \) STORED
statement error use AS \( <expr> \) STORED or AS \( <expr> \) VIRTUAL
CREATE TABLE y (
a INT AS 3 STORED
)

statement error use AS \( <expr> \) STORED
statement error use AS \( <expr> \) STORED or AS \( <expr> \) VIRTUAL
CREATE TABLE y (
a INT AS (3)
)

statement error unimplemented: virtual computed columns
CREATE TABLE y (
a INT AS (3) VIRTUAL
)

statement ok
CREATE TABLE tmp (x INT)

statement error unimplemented: virtual computed columns
ALTER TABLE tmp ADD COLUMN y INT AS (x+1) VIRTUAL

statement ok
DROP TABLE tmp

Expand Down
1 change: 0 additions & 1 deletion pkg/sql/logictest/testdata/logic_test/information_schema
Original file line number Diff line number Diff line change
Expand Up @@ -3452,7 +3452,6 @@ experimental_enable_hash_sharded_indexes off
experimental_enable_implicit_column_partitioning off
experimental_enable_temp_tables off
experimental_enable_unique_without_index_constraints on
experimental_enable_virtual_columns off
experimental_use_new_schema_changer off
extra_float_digits 0
force_savepoint_restart off
Expand Down
3 changes: 0 additions & 3 deletions pkg/sql/logictest/testdata/logic_test/partial_index
Original file line number Diff line number Diff line change
Expand Up @@ -1345,9 +1345,6 @@ SELECT * FROM prune@idx WHERE c > 0
# columns.
subtest virtual

statement ok
SET experimental_enable_virtual_columns = true

statement ok
CREATE TABLE virt (
a INT PRIMARY KEY,
Expand Down
3 changes: 0 additions & 3 deletions pkg/sql/logictest/testdata/logic_test/pg_catalog
Original file line number Diff line number Diff line change
Expand Up @@ -2007,7 +2007,6 @@ experimental_enable_hash_sharded_indexes off NULL
experimental_enable_implicit_column_partitioning off NULL NULL NULL string
experimental_enable_temp_tables off NULL NULL NULL string
experimental_enable_unique_without_index_constraints on NULL NULL NULL string
experimental_enable_virtual_columns off NULL NULL NULL string
experimental_use_new_schema_changer off NULL NULL NULL string
extra_float_digits 0 NULL NULL NULL string
force_savepoint_restart off NULL NULL NULL string
Expand Down Expand Up @@ -2084,7 +2083,6 @@ experimental_enable_hash_sharded_indexes off NULL
experimental_enable_implicit_column_partitioning off NULL user NULL off off
experimental_enable_temp_tables off NULL user NULL off off
experimental_enable_unique_without_index_constraints on NULL user NULL off off
experimental_enable_virtual_columns off NULL user NULL off off
experimental_use_new_schema_changer off NULL user NULL off off
extra_float_digits 0 NULL user NULL 0 2
force_savepoint_restart off NULL user NULL off off
Expand Down Expand Up @@ -2157,7 +2155,6 @@ experimental_enable_hash_sharded_indexes NULL NULL NULL
experimental_enable_implicit_column_partitioning NULL NULL NULL NULL NULL
experimental_enable_temp_tables NULL NULL NULL NULL NULL
experimental_enable_unique_without_index_constraints NULL NULL NULL NULL NULL
experimental_enable_virtual_columns NULL NULL NULL NULL NULL
experimental_use_new_schema_changer NULL NULL NULL NULL NULL
extra_float_digits NULL NULL NULL NULL NULL
force_savepoint_restart NULL NULL NULL NULL NULL
Expand Down
1 change: 0 additions & 1 deletion pkg/sql/logictest/testdata/logic_test/show_source
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ experimental_enable_hash_sharded_indexes off
experimental_enable_implicit_column_partitioning off
experimental_enable_temp_tables off
experimental_enable_unique_without_index_constraints off
experimental_enable_virtual_columns off
experimental_use_new_schema_changer off
extra_float_digits 0
force_savepoint_restart off
Expand Down
3 changes: 0 additions & 3 deletions pkg/sql/logictest/testdata/logic_test/virtual_columns
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
statement ok
SET experimental_enable_virtual_columns = true

# Test that we don't allow FAMILY constraints with virtual columns.
statement error virtual computed column "v" cannot be part of a family
CREATE TABLE t (
Expand Down
3 changes: 0 additions & 3 deletions pkg/sql/opt/exec/execbuilder/testdata/virtual_columns
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
# LogicTest: local

statement ok
SET experimental_enable_virtual_columns = true

statement ok
CREATE TABLE t (
a INT PRIMARY KEY,
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/parser/sql.y
Original file line number Diff line number Diff line change
Expand Up @@ -6345,7 +6345,7 @@ col_qualification_elem:
}
| generated_as error
{
sqllex.Error("use AS ( <expr> ) STORED")
sqllex.Error("use AS ( <expr> ) STORED or AS ( <expr> ) VIRTUAL")
return 1
}

Expand Down
1 change: 0 additions & 1 deletion pkg/sql/schema_changer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4571,7 +4571,6 @@ func TestNoBackfillForVirtualColumn(t *testing.T) {
sqlDB.Exec(t, `CREATE DATABASE t`)
sqlDB.Exec(t, `CREATE TABLE t.test (a INT PRIMARY KEY)`)
sqlDB.Exec(t, `INSERT INTO t.test VALUES (1), (2), (3)`)
sqlDB.Exec(t, `SET experimental_enable_virtual_columns = true`)

sawBackfill = false
sqlDB.Exec(t, `ALTER TABLE t.test ADD COLUMN b INT AS (a + 5) VIRTUAL`)
Expand Down
1 change: 0 additions & 1 deletion pkg/sql/schemachanger/scbuild/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ go_library(
"//pkg/sql/sem/tree",
"//pkg/sql/sqlerrors",
"//pkg/sql/types",
"//pkg/util/errorutil/unimplemented",
"//pkg/util/protoutil",
"//pkg/util/sequence",
"@com_github_cockroachdb_errors//:errors",
Expand Down
5 changes: 0 additions & 5 deletions pkg/sql/schemachanger/scbuild/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sqlerrors"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented"
"github.com/cockroachdb/cockroach/pkg/util/protoutil"
"github.com/cockroachdb/cockroach/pkg/util/sequence"
"github.com/cockroachdb/errors"
Expand Down Expand Up @@ -234,10 +233,6 @@ func (b *Builder) alterTableAddColumn(
}

if d.IsComputed() {
if d.IsVirtual() {
return unimplemented.NewWithIssue(57608, "virtual computed columns")
}

// TODO (lucy): This is not going to work when the computed column
// references columns created in the same transaction.
computedColValidator := schemaexpr.MakeComputedColumnValidator(
Expand Down
5 changes: 0 additions & 5 deletions pkg/sql/sessiondata/session_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,11 +223,6 @@ type LocalOnlySessionData struct {
// EnableSeqScan is a dummy setting for the enable_seqscan var.
EnableSeqScan bool

// VirtualColumnsEnabled indicates whether we allow virtual (non-stored)
// computed columns.
// TODO(radu): remove this once the feature is stable.
VirtualColumnsEnabled bool

// EnableUniqueWithoutIndexConstraints indicates whether creating unique
// constraints without an index is allowed.
// TODO(rytaft): remove this once unique without index constraints are fully
Expand Down
17 changes: 0 additions & 17 deletions pkg/sql/vars.go
Original file line number Diff line number Diff line change
Expand Up @@ -1200,23 +1200,6 @@ var varGen = map[string]sessionVar{
},
},

// CockroachDB extension.
`experimental_enable_virtual_columns`: {
GetStringVal: makePostgresBoolGetStringValFn(`experimental_enable_virtual_columns`),
Set: func(_ context.Context, m *sessionDataMutator, s string) error {
b, err := paramparse.ParseBoolVar(`experimental_enable_virtual_columns`, s)
if err != nil {
return err
}
m.SetVirtualColumnsEnabled(b)
return nil
},
Get: func(evalCtx *extendedEvalContext) string {
return formatBoolAsPostgresSetting(evalCtx.SessionData.VirtualColumnsEnabled)
},
GlobalDefault: globalFalse,
},

// TODO(rytaft): remove this once unique without index constraints are fully
// supported.
`experimental_enable_unique_without_index_constraints`: {
Expand Down

0 comments on commit 715a190

Please sign in to comment.