From 0482ae2b73b954f2e6a5fcf54aa518b0d845d269 Mon Sep 17 00:00:00 2001 From: Radu Berinde Date: Thu, 18 Feb 2021 14:00:37 -0500 Subject: [PATCH] sql: remove experimental setting for virtual columns 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. --- pkg/sql/add_column.go | 4 ---- pkg/sql/create_table.go | 3 --- pkg/sql/exec_util.go | 5 ----- pkg/sql/indexbackfiller_test.go | 1 - pkg/sql/logictest/testdata/logic_test/computed | 15 +++++---------- .../testdata/logic_test/information_schema | 1 - .../logictest/testdata/logic_test/partial_index | 3 --- .../logictest/testdata/logic_test/pg_catalog | 3 --- .../logictest/testdata/logic_test/show_source | 1 - .../testdata/logic_test/virtual_columns | 3 --- .../exec/execbuilder/testdata/virtual_columns | 3 --- pkg/sql/parser/sql.y | 2 +- pkg/sql/schema_changer_test.go | 1 - pkg/sql/schemachanger/scbuild/BUILD.bazel | 1 - pkg/sql/schemachanger/scbuild/builder.go | 5 ----- pkg/sql/sessiondata/session_data.go | 5 ----- pkg/sql/vars.go | 17 ----------------- 17 files changed, 6 insertions(+), 67 deletions(-) diff --git a/pkg/sql/add_column.go b/pkg/sql/add_column.go index 9a6e9bf676c5..6588ec81ed0a 100644 --- a/pkg/sql/add_column.go +++ b/pkg/sql/add_column.go @@ -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" ) @@ -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, diff --git a/pkg/sql/create_table.go b/pkg/sql/create_table.go index 199e787027ce..f91275b64ea3 100644 --- a/pkg/sql/create_table.go +++ b/pkg/sql/create_table.go @@ -1699,9 +1699,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", diff --git a/pkg/sql/exec_util.go b/pkg/sql/exec_util.go index 68fefa406d48..f9d29f924cb3 100644 --- a/pkg/sql/exec_util.go +++ b/pkg/sql/exec_util.go @@ -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) { diff --git a/pkg/sql/indexbackfiller_test.go b/pkg/sql/indexbackfiller_test.go index b444659b8007..640abe66433e 100644 --- a/pkg/sql/indexbackfiller_test.go +++ b/pkg/sql/indexbackfiller_test.go @@ -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); `, diff --git a/pkg/sql/logictest/testdata/logic_test/computed b/pkg/sql/logictest/testdata/logic_test/computed index ee9ec5c2ce82..9f581a0c2b87 100644 --- a/pkg/sql/logictest/testdata/logic_test/computed +++ b/pkg/sql/logictest/testdata/logic_test/computed @@ -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, @@ -267,27 +270,19 @@ DROP TABLE x # statement ok # DROP TABLE x -statement error use AS \( \) STORED +statement error use AS \( \) STORED or AS \( \) VIRTUAL CREATE TABLE y ( a INT AS 3 STORED ) -statement error use AS \( \) STORED +statement error use AS \( \) STORED or AS \( \) 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 diff --git a/pkg/sql/logictest/testdata/logic_test/information_schema b/pkg/sql/logictest/testdata/logic_test/information_schema index e74bf5812088..1bc29f9e298b 100755 --- a/pkg/sql/logictest/testdata/logic_test/information_schema +++ b/pkg/sql/logictest/testdata/logic_test/information_schema @@ -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 diff --git a/pkg/sql/logictest/testdata/logic_test/partial_index b/pkg/sql/logictest/testdata/logic_test/partial_index index 605ebdffdd1d..1d2e7c06f0d5 100644 --- a/pkg/sql/logictest/testdata/logic_test/partial_index +++ b/pkg/sql/logictest/testdata/logic_test/partial_index @@ -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, diff --git a/pkg/sql/logictest/testdata/logic_test/pg_catalog b/pkg/sql/logictest/testdata/logic_test/pg_catalog index e060a20f9352..78164890ef65 100644 --- a/pkg/sql/logictest/testdata/logic_test/pg_catalog +++ b/pkg/sql/logictest/testdata/logic_test/pg_catalog @@ -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 @@ -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 @@ -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 diff --git a/pkg/sql/logictest/testdata/logic_test/show_source b/pkg/sql/logictest/testdata/logic_test/show_source index 2b4ca2dde365..0a70b29df13f 100644 --- a/pkg/sql/logictest/testdata/logic_test/show_source +++ b/pkg/sql/logictest/testdata/logic_test/show_source @@ -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 diff --git a/pkg/sql/logictest/testdata/logic_test/virtual_columns b/pkg/sql/logictest/testdata/logic_test/virtual_columns index 21da631c6411..6a293114fa61 100644 --- a/pkg/sql/logictest/testdata/logic_test/virtual_columns +++ b/pkg/sql/logictest/testdata/logic_test/virtual_columns @@ -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 ( diff --git a/pkg/sql/opt/exec/execbuilder/testdata/virtual_columns b/pkg/sql/opt/exec/execbuilder/testdata/virtual_columns index 69ca0af3a1ba..04ad2bce416b 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/virtual_columns +++ b/pkg/sql/opt/exec/execbuilder/testdata/virtual_columns @@ -1,8 +1,5 @@ # LogicTest: local -statement ok -SET experimental_enable_virtual_columns = true - statement ok CREATE TABLE t ( a INT PRIMARY KEY, diff --git a/pkg/sql/parser/sql.y b/pkg/sql/parser/sql.y index 58b048f3d7ee..9400ae80b3d8 100644 --- a/pkg/sql/parser/sql.y +++ b/pkg/sql/parser/sql.y @@ -6323,7 +6323,7 @@ col_qualification_elem: } | generated_as error { - sqllex.Error("use AS ( ) STORED") + sqllex.Error("use AS ( ) STORED or AS ( ) VIRTUAL") return 1 } diff --git a/pkg/sql/schema_changer_test.go b/pkg/sql/schema_changer_test.go index c75c668e9dd9..67d1d538b2c4 100644 --- a/pkg/sql/schema_changer_test.go +++ b/pkg/sql/schema_changer_test.go @@ -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`) diff --git a/pkg/sql/schemachanger/scbuild/BUILD.bazel b/pkg/sql/schemachanger/scbuild/BUILD.bazel index ebbb998c86c2..9628eec90c45 100644 --- a/pkg/sql/schemachanger/scbuild/BUILD.bazel +++ b/pkg/sql/schemachanger/scbuild/BUILD.bazel @@ -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", diff --git a/pkg/sql/schemachanger/scbuild/builder.go b/pkg/sql/schemachanger/scbuild/builder.go index b7543d384694..bb8688a39a7d 100644 --- a/pkg/sql/schemachanger/scbuild/builder.go +++ b/pkg/sql/schemachanger/scbuild/builder.go @@ -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" @@ -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( diff --git a/pkg/sql/sessiondata/session_data.go b/pkg/sql/sessiondata/session_data.go index 815b2c9ad475..f7d80ae6df47 100644 --- a/pkg/sql/sessiondata/session_data.go +++ b/pkg/sql/sessiondata/session_data.go @@ -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 diff --git a/pkg/sql/vars.go b/pkg/sql/vars.go index 1925e7ed3a24..e6c38beb4324 100644 --- a/pkg/sql/vars.go +++ b/pkg/sql/vars.go @@ -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`: {