From 4ec9a5bd511f01644beed8cf00ce952e6c14a163 Mon Sep 17 00:00:00 2001 From: Oliver Tan Date: Fri, 21 Jan 2022 14:59:20 +1100 Subject: [PATCH 1/2] paramparse: add pgcodes to certain error codes Release note (sql change): Added pgcodes to errors when an invalid storage parameter is passed. --- .../testdata/logic_test/create_statements | 2 +- .../testdata/logic_test/geospatial_index | 12 +++--- pkg/sql/paramparse/paramobserver.go | 37 +++++++++++++------ 3 files changed, 33 insertions(+), 18 deletions(-) diff --git a/pkg/sql/logictest/testdata/logic_test/create_statements b/pkg/sql/logictest/testdata/logic_test/create_statements index 197dcce505bc..60394ee23991 100644 --- a/pkg/sql/logictest/testdata/logic_test/create_statements +++ b/pkg/sql/logictest/testdata/logic_test/create_statements @@ -6562,7 +6562,7 @@ unlogged_tbl CREATE TABLE public.unlogged_tbl ( FAMILY "primary" (col) ) -statement error invalid storage parameter "foo" +statement error pgcode 22023 invalid storage parameter "foo" CREATE TABLE a (b INT) WITH (foo=100); statement error parameter "fillfactor" requires a float value diff --git a/pkg/sql/logictest/testdata/logic_test/geospatial_index b/pkg/sql/logictest/testdata/logic_test/geospatial_index index 6e694986bfc6..14d79e5614aa 100644 --- a/pkg/sql/logictest/testdata/logic_test/geospatial_index +++ b/pkg/sql/logictest/testdata/logic_test/geospatial_index @@ -8,22 +8,22 @@ CREATE TABLE geo_table( FAMILY fam_2_id (id) ) -statement error index setting "s2_max_cells" can only be set on GEOMETRY or GEOGRAPHY spatial indexes +statement error pgcode 22023 index setting "s2_max_cells" can only be set on GEOMETRY or GEOGRAPHY spatial indexes CREATE INDEX bad_idx ON geo_table(id) WITH (s2_max_cells=15) -statement error "s2_max_cells" value must be between 1 and 32 inclusive +statement error pgcode 22023 "s2_max_cells" value must be between 1 and 32 inclusive CREATE INDEX bad_idx ON geo_table USING GIST(geom) WITH (s2_max_cells=42) -statement error s2_max_level \(29\) must be divisible by s2_level_mod \(2\) +statement error pgcode 22023 s2_max_level \(29\) must be divisible by s2_level_mod \(2\) CREATE INDEX bad_idx ON geo_table USING GIST(geom) WITH (s2_max_level=29, s2_level_mod=2) -statement error "geometry_min_x" can only be applied to GEOMETRY spatial indexes +statement error pgcode 22023 "geometry_min_x" can only be applied to GEOMETRY spatial indexes CREATE INDEX bad_idx ON geo_table USING GIST(geog) WITH (geometry_min_x=0) -statement error geometry_max_x \(0\.000000\) must be greater than geometry_min_x \(10\.000000\) +statement error pgcode 22023 geometry_max_x \(0\.000000\) must be greater than geometry_min_x \(10\.000000\) CREATE INDEX bad_idx ON geo_table USING GIST(geom) WITH (geometry_min_x=10, geometry_max_x=0) -statement error geometry_max_y \(0\.000000\) must be greater than geometry_min_y \(10\.000000\) +statement error pgcode 22023 geometry_max_y \(0\.000000\) must be greater than geometry_min_y \(10\.000000\) CREATE INDEX bad_idx ON geo_table USING GIST(geom) WITH (geometry_min_y=10, geometry_max_y=0) statement ok diff --git a/pkg/sql/paramparse/paramobserver.go b/pkg/sql/paramparse/paramobserver.go index afe34e93b339..9b5b524bdc6d 100644 --- a/pkg/sql/paramparse/paramobserver.go +++ b/pkg/sql/paramparse/paramobserver.go @@ -15,6 +15,8 @@ import ( "github.com/cockroachdb/cockroach/pkg/geo/geoindex" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgnotice" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/types" @@ -34,7 +36,7 @@ func ApplyStorageParameters( for _, sp := range params { key := string(sp.Key) if sp.Value == nil { - return errors.Errorf("storage parameter %q requires a value", key) + return pgerror.Newf(pgcode.InvalidParameterValue, "storage parameter %q requires a value", key) } // Expressions may be an unresolved name. // Cast these as strings. @@ -78,7 +80,7 @@ func applyFillFactorStorageParam(evalCtx *tree.EvalContext, key string, datum tr return err } if val < 0 || val > 100 { - return errors.Newf("%q must be between 0 and 100", key) + return pgerror.Newf(pgcode.InvalidParameterValue, "%q must be between 0 and 100", key) } if evalCtx != nil { evalCtx.ClientNoticeSender.BufferClientNotice( @@ -151,7 +153,7 @@ func (a *TableStorageParamObserver) Apply( `user_catalog_table`: return unimplemented.NewWithIssuef(43299, "storage parameter %q", key) } - return errors.Errorf("invalid storage parameter %q", key) + return pgerror.Newf(pgcode.InvalidParameterValue, "invalid storage parameter %q", key) } // IndexStorageParamObserver observes storage parameters for indexes. @@ -177,7 +179,11 @@ func (a *IndexStorageParamObserver) applyS2ConfigSetting( ) error { s2Config := getS2ConfigFromIndex(a.IndexDesc) if s2Config == nil { - return errors.Newf("index setting %q can only be set on GEOMETRY or GEOGRAPHY spatial indexes", key) + return pgerror.Newf( + pgcode.InvalidParameterValue, + "index setting %q can only be set on GEOMETRY or GEOGRAPHY spatial indexes", + key, + ) } val, err := DatumAsInt(evalCtx, key, expr) @@ -185,7 +191,13 @@ func (a *IndexStorageParamObserver) applyS2ConfigSetting( return errors.Wrapf(err, "error decoding %q", key) } if val < min || val > max { - return errors.Newf("%q value must be between %d and %d inclusive", key, min, max) + return pgerror.Newf( + pgcode.InvalidParameterValue, + "%q value must be between %d and %d inclusive", + key, + min, + max, + ) } switch key { case `s2_max_level`: @@ -203,7 +215,7 @@ func (a *IndexStorageParamObserver) applyGeometryIndexSetting( evalCtx *tree.EvalContext, key string, expr tree.Datum, ) error { if a.IndexDesc.GeoConfig.S2Geometry == nil { - return errors.Newf("%q can only be applied to GEOMETRY spatial indexes", key) + return pgerror.Newf(pgcode.InvalidParameterValue, "%q can only be applied to GEOMETRY spatial indexes", key) } val, err := DatumAsFloat(evalCtx, key, expr) if err != nil { @@ -219,7 +231,7 @@ func (a *IndexStorageParamObserver) applyGeometryIndexSetting( case `geometry_max_y`: a.IndexDesc.GeoConfig.S2Geometry.MaxY = val default: - return errors.Newf("unknown key: %q", key) + return pgerror.Newf(pgcode.InvalidParameterValue, "unknown key: %q", key) } return nil } @@ -247,7 +259,7 @@ func (a *IndexStorageParamObserver) Apply( `autosummarize`: return unimplemented.NewWithIssuef(43299, "storage parameter %q", key) } - return errors.Errorf("invalid storage parameter %q", key) + return pgerror.Newf(pgcode.InvalidParameterValue, "invalid storage parameter %q", key) } // RunPostChecks implements the StorageParamObserver interface. @@ -255,7 +267,8 @@ func (a *IndexStorageParamObserver) RunPostChecks() error { s2Config := getS2ConfigFromIndex(a.IndexDesc) if s2Config != nil { if (s2Config.MaxLevel)%s2Config.LevelMod != 0 { - return errors.Newf( + return pgerror.Newf( + pgcode.InvalidParameterValue, "s2_max_level (%d) must be divisible by s2_level_mod (%d)", s2Config.MaxLevel, s2Config.LevelMod, @@ -265,14 +278,16 @@ func (a *IndexStorageParamObserver) RunPostChecks() error { if cfg := a.IndexDesc.GeoConfig.S2Geometry; cfg != nil { if cfg.MaxX <= cfg.MinX { - return errors.Newf( + return pgerror.Newf( + pgcode.InvalidParameterValue, "geometry_max_x (%f) must be greater than geometry_min_x (%f)", cfg.MaxX, cfg.MinX, ) } if cfg.MaxY <= cfg.MinY { - return errors.Newf( + return pgerror.Newf( + pgcode.InvalidParameterValue, "geometry_max_y (%f) must be greater than geometry_min_y (%f)", cfg.MaxY, cfg.MinY, From d38a3b74b8117473b0c5f63b0d59c78a812eba2c Mon Sep 17 00:00:00 2001 From: Oliver Tan Date: Mon, 24 Jan 2022 10:09:06 +1100 Subject: [PATCH 2/2] sql: implement the ALTER TABLE ... SET (...) syntax Release note (sql change): Implemented the `ALTER TABLE ... SET (...)` syntax. This mostly no-ops as we do not support any storage parameters (yet). --- docs/generated/sql/bnf/alter_table.bnf | 4 ++-- .../sql/bnf/alter_table_partition_by.bnf | 4 ++-- docs/generated/sql/bnf/stmt_block.bnf | 1 + pkg/sql/alter_table.go | 12 ++++++++++ .../logictest/testdata/logic_test/alter_table | 23 +++++++++++++++++++ pkg/sql/parser/sql.y | 7 ++++++ pkg/sql/parser/testdata/alter_table | 8 +++++++ pkg/sql/sem/tree/alter_table.go | 20 ++++++++++++++++ 8 files changed, 75 insertions(+), 4 deletions(-) diff --git a/docs/generated/sql/bnf/alter_table.bnf b/docs/generated/sql/bnf/alter_table.bnf index 303f85d9a7ff..c3d4dfcf7380 100644 --- a/docs/generated/sql/bnf/alter_table.bnf +++ b/docs/generated/sql/bnf/alter_table.bnf @@ -1,3 +1,3 @@ alter_onetable_stmt ::= - 'ALTER' 'TABLE' table_name ( ( ( 'RENAME' ( 'COLUMN' | ) column_name 'TO' column_name | 'RENAME' 'CONSTRAINT' column_name 'TO' column_name | 'ADD' ( column_name typename col_qual_list ) | 'ADD' 'IF' 'NOT' 'EXISTS' ( column_name typename col_qual_list ) | 'ADD' 'COLUMN' ( column_name typename col_qual_list ) | 'ADD' 'COLUMN' 'IF' 'NOT' 'EXISTS' ( column_name typename col_qual_list ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DEFAULT' a_expr | 'DROP' 'DEFAULT' ) | 'ALTER' ( 'COLUMN' | ) column_name alter_column_on_update | 'ALTER' ( 'COLUMN' | ) column_name alter_column_visible | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'NOT' 'NULL' | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'STORED' | 'ALTER' ( 'COLUMN' | ) column_name 'SET' 'NOT' 'NULL' | 'DROP' ( 'COLUMN' | ) 'IF' 'EXISTS' column_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' ( 'COLUMN' | ) column_name ( 'CASCADE' | 'RESTRICT' | ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DATA' | ) 'TYPE' typename ( 'COLLATE' collation_name | ) ( 'USING' a_expr | ) | 'ADD' ( 'CONSTRAINT' constraint_name constraint_elem | constraint_elem ) | 'ADD' 'CONSTRAINT' 'IF' 'NOT' 'EXISTS' constraint_name constraint_elem | 'ALTER' 'PRIMARY' 'KEY' 'USING' 'COLUMNS' '(' index_params ')' opt_hash_sharded | 'VALIDATE' 'CONSTRAINT' constraint_name | 'DROP' 'CONSTRAINT' 'IF' 'EXISTS' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' 'CONSTRAINT' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'EXPERIMENTAL_AUDIT' 'SET' audit_mode | partition_by_table ) ) ( ( ',' ( 'RENAME' ( 'COLUMN' | ) column_name 'TO' column_name | 'RENAME' 'CONSTRAINT' column_name 'TO' column_name | 'ADD' ( column_name typename col_qual_list ) | 'ADD' 'IF' 'NOT' 'EXISTS' ( column_name typename col_qual_list ) | 'ADD' 'COLUMN' ( column_name typename col_qual_list ) | 'ADD' 'COLUMN' 'IF' 'NOT' 'EXISTS' ( column_name typename col_qual_list ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DEFAULT' a_expr | 'DROP' 'DEFAULT' ) | 'ALTER' ( 'COLUMN' | ) column_name alter_column_on_update | 'ALTER' ( 'COLUMN' | ) column_name alter_column_visible | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'NOT' 'NULL' | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'STORED' | 'ALTER' ( 'COLUMN' | ) column_name 'SET' 'NOT' 'NULL' | 'DROP' ( 'COLUMN' | ) 'IF' 'EXISTS' column_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' ( 'COLUMN' | ) column_name ( 'CASCADE' | 'RESTRICT' | ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DATA' | ) 'TYPE' typename ( 'COLLATE' collation_name | ) ( 'USING' a_expr | ) | 'ADD' ( 'CONSTRAINT' constraint_name constraint_elem | constraint_elem ) | 'ADD' 'CONSTRAINT' 'IF' 'NOT' 'EXISTS' constraint_name constraint_elem | 'ALTER' 'PRIMARY' 'KEY' 'USING' 'COLUMNS' '(' index_params ')' opt_hash_sharded | 'VALIDATE' 'CONSTRAINT' constraint_name | 'DROP' 'CONSTRAINT' 'IF' 'EXISTS' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' 'CONSTRAINT' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'EXPERIMENTAL_AUDIT' 'SET' audit_mode | partition_by_table ) ) )* ) - | 'ALTER' 'TABLE' 'IF' 'EXISTS' table_name ( ( ( 'RENAME' ( 'COLUMN' | ) column_name 'TO' column_name | 'RENAME' 'CONSTRAINT' column_name 'TO' column_name | 'ADD' ( column_name typename col_qual_list ) | 'ADD' 'IF' 'NOT' 'EXISTS' ( column_name typename col_qual_list ) | 'ADD' 'COLUMN' ( column_name typename col_qual_list ) | 'ADD' 'COLUMN' 'IF' 'NOT' 'EXISTS' ( column_name typename col_qual_list ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DEFAULT' a_expr | 'DROP' 'DEFAULT' ) | 'ALTER' ( 'COLUMN' | ) column_name alter_column_on_update | 'ALTER' ( 'COLUMN' | ) column_name alter_column_visible | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'NOT' 'NULL' | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'STORED' | 'ALTER' ( 'COLUMN' | ) column_name 'SET' 'NOT' 'NULL' | 'DROP' ( 'COLUMN' | ) 'IF' 'EXISTS' column_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' ( 'COLUMN' | ) column_name ( 'CASCADE' | 'RESTRICT' | ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DATA' | ) 'TYPE' typename ( 'COLLATE' collation_name | ) ( 'USING' a_expr | ) | 'ADD' ( 'CONSTRAINT' constraint_name constraint_elem | constraint_elem ) | 'ADD' 'CONSTRAINT' 'IF' 'NOT' 'EXISTS' constraint_name constraint_elem | 'ALTER' 'PRIMARY' 'KEY' 'USING' 'COLUMNS' '(' index_params ')' opt_hash_sharded | 'VALIDATE' 'CONSTRAINT' constraint_name | 'DROP' 'CONSTRAINT' 'IF' 'EXISTS' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' 'CONSTRAINT' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'EXPERIMENTAL_AUDIT' 'SET' audit_mode | partition_by_table ) ) ( ( ',' ( 'RENAME' ( 'COLUMN' | ) column_name 'TO' column_name | 'RENAME' 'CONSTRAINT' column_name 'TO' column_name | 'ADD' ( column_name typename col_qual_list ) | 'ADD' 'IF' 'NOT' 'EXISTS' ( column_name typename col_qual_list ) | 'ADD' 'COLUMN' ( column_name typename col_qual_list ) | 'ADD' 'COLUMN' 'IF' 'NOT' 'EXISTS' ( column_name typename col_qual_list ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DEFAULT' a_expr | 'DROP' 'DEFAULT' ) | 'ALTER' ( 'COLUMN' | ) column_name alter_column_on_update | 'ALTER' ( 'COLUMN' | ) column_name alter_column_visible | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'NOT' 'NULL' | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'STORED' | 'ALTER' ( 'COLUMN' | ) column_name 'SET' 'NOT' 'NULL' | 'DROP' ( 'COLUMN' | ) 'IF' 'EXISTS' column_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' ( 'COLUMN' | ) column_name ( 'CASCADE' | 'RESTRICT' | ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DATA' | ) 'TYPE' typename ( 'COLLATE' collation_name | ) ( 'USING' a_expr | ) | 'ADD' ( 'CONSTRAINT' constraint_name constraint_elem | constraint_elem ) | 'ADD' 'CONSTRAINT' 'IF' 'NOT' 'EXISTS' constraint_name constraint_elem | 'ALTER' 'PRIMARY' 'KEY' 'USING' 'COLUMNS' '(' index_params ')' opt_hash_sharded | 'VALIDATE' 'CONSTRAINT' constraint_name | 'DROP' 'CONSTRAINT' 'IF' 'EXISTS' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' 'CONSTRAINT' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'EXPERIMENTAL_AUDIT' 'SET' audit_mode | partition_by_table ) ) )* ) + 'ALTER' 'TABLE' table_name ( ( ( 'RENAME' ( 'COLUMN' | ) column_name 'TO' column_name | 'RENAME' 'CONSTRAINT' column_name 'TO' column_name | 'ADD' ( column_name typename col_qual_list ) | 'ADD' 'IF' 'NOT' 'EXISTS' ( column_name typename col_qual_list ) | 'ADD' 'COLUMN' ( column_name typename col_qual_list ) | 'ADD' 'COLUMN' 'IF' 'NOT' 'EXISTS' ( column_name typename col_qual_list ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DEFAULT' a_expr | 'DROP' 'DEFAULT' ) | 'ALTER' ( 'COLUMN' | ) column_name alter_column_on_update | 'ALTER' ( 'COLUMN' | ) column_name alter_column_visible | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'NOT' 'NULL' | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'STORED' | 'ALTER' ( 'COLUMN' | ) column_name 'SET' 'NOT' 'NULL' | 'DROP' ( 'COLUMN' | ) 'IF' 'EXISTS' column_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' ( 'COLUMN' | ) column_name ( 'CASCADE' | 'RESTRICT' | ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DATA' | ) 'TYPE' typename ( 'COLLATE' collation_name | ) ( 'USING' a_expr | ) | 'ADD' ( 'CONSTRAINT' constraint_name constraint_elem | constraint_elem ) | 'ADD' 'CONSTRAINT' 'IF' 'NOT' 'EXISTS' constraint_name constraint_elem | 'ALTER' 'PRIMARY' 'KEY' 'USING' 'COLUMNS' '(' index_params ')' opt_hash_sharded | 'VALIDATE' 'CONSTRAINT' constraint_name | 'DROP' 'CONSTRAINT' 'IF' 'EXISTS' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' 'CONSTRAINT' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'EXPERIMENTAL_AUDIT' 'SET' audit_mode | partition_by_table | 'SET' '(' storage_parameter_list ')' ) ) ( ( ',' ( 'RENAME' ( 'COLUMN' | ) column_name 'TO' column_name | 'RENAME' 'CONSTRAINT' column_name 'TO' column_name | 'ADD' ( column_name typename col_qual_list ) | 'ADD' 'IF' 'NOT' 'EXISTS' ( column_name typename col_qual_list ) | 'ADD' 'COLUMN' ( column_name typename col_qual_list ) | 'ADD' 'COLUMN' 'IF' 'NOT' 'EXISTS' ( column_name typename col_qual_list ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DEFAULT' a_expr | 'DROP' 'DEFAULT' ) | 'ALTER' ( 'COLUMN' | ) column_name alter_column_on_update | 'ALTER' ( 'COLUMN' | ) column_name alter_column_visible | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'NOT' 'NULL' | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'STORED' | 'ALTER' ( 'COLUMN' | ) column_name 'SET' 'NOT' 'NULL' | 'DROP' ( 'COLUMN' | ) 'IF' 'EXISTS' column_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' ( 'COLUMN' | ) column_name ( 'CASCADE' | 'RESTRICT' | ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DATA' | ) 'TYPE' typename ( 'COLLATE' collation_name | ) ( 'USING' a_expr | ) | 'ADD' ( 'CONSTRAINT' constraint_name constraint_elem | constraint_elem ) | 'ADD' 'CONSTRAINT' 'IF' 'NOT' 'EXISTS' constraint_name constraint_elem | 'ALTER' 'PRIMARY' 'KEY' 'USING' 'COLUMNS' '(' index_params ')' opt_hash_sharded | 'VALIDATE' 'CONSTRAINT' constraint_name | 'DROP' 'CONSTRAINT' 'IF' 'EXISTS' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' 'CONSTRAINT' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'EXPERIMENTAL_AUDIT' 'SET' audit_mode | partition_by_table | 'SET' '(' storage_parameter_list ')' ) ) )* ) + | 'ALTER' 'TABLE' 'IF' 'EXISTS' table_name ( ( ( 'RENAME' ( 'COLUMN' | ) column_name 'TO' column_name | 'RENAME' 'CONSTRAINT' column_name 'TO' column_name | 'ADD' ( column_name typename col_qual_list ) | 'ADD' 'IF' 'NOT' 'EXISTS' ( column_name typename col_qual_list ) | 'ADD' 'COLUMN' ( column_name typename col_qual_list ) | 'ADD' 'COLUMN' 'IF' 'NOT' 'EXISTS' ( column_name typename col_qual_list ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DEFAULT' a_expr | 'DROP' 'DEFAULT' ) | 'ALTER' ( 'COLUMN' | ) column_name alter_column_on_update | 'ALTER' ( 'COLUMN' | ) column_name alter_column_visible | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'NOT' 'NULL' | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'STORED' | 'ALTER' ( 'COLUMN' | ) column_name 'SET' 'NOT' 'NULL' | 'DROP' ( 'COLUMN' | ) 'IF' 'EXISTS' column_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' ( 'COLUMN' | ) column_name ( 'CASCADE' | 'RESTRICT' | ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DATA' | ) 'TYPE' typename ( 'COLLATE' collation_name | ) ( 'USING' a_expr | ) | 'ADD' ( 'CONSTRAINT' constraint_name constraint_elem | constraint_elem ) | 'ADD' 'CONSTRAINT' 'IF' 'NOT' 'EXISTS' constraint_name constraint_elem | 'ALTER' 'PRIMARY' 'KEY' 'USING' 'COLUMNS' '(' index_params ')' opt_hash_sharded | 'VALIDATE' 'CONSTRAINT' constraint_name | 'DROP' 'CONSTRAINT' 'IF' 'EXISTS' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' 'CONSTRAINT' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'EXPERIMENTAL_AUDIT' 'SET' audit_mode | partition_by_table | 'SET' '(' storage_parameter_list ')' ) ) ( ( ',' ( 'RENAME' ( 'COLUMN' | ) column_name 'TO' column_name | 'RENAME' 'CONSTRAINT' column_name 'TO' column_name | 'ADD' ( column_name typename col_qual_list ) | 'ADD' 'IF' 'NOT' 'EXISTS' ( column_name typename col_qual_list ) | 'ADD' 'COLUMN' ( column_name typename col_qual_list ) | 'ADD' 'COLUMN' 'IF' 'NOT' 'EXISTS' ( column_name typename col_qual_list ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DEFAULT' a_expr | 'DROP' 'DEFAULT' ) | 'ALTER' ( 'COLUMN' | ) column_name alter_column_on_update | 'ALTER' ( 'COLUMN' | ) column_name alter_column_visible | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'NOT' 'NULL' | 'ALTER' ( 'COLUMN' | ) column_name 'DROP' 'STORED' | 'ALTER' ( 'COLUMN' | ) column_name 'SET' 'NOT' 'NULL' | 'DROP' ( 'COLUMN' | ) 'IF' 'EXISTS' column_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' ( 'COLUMN' | ) column_name ( 'CASCADE' | 'RESTRICT' | ) | 'ALTER' ( 'COLUMN' | ) column_name ( 'SET' 'DATA' | ) 'TYPE' typename ( 'COLLATE' collation_name | ) ( 'USING' a_expr | ) | 'ADD' ( 'CONSTRAINT' constraint_name constraint_elem | constraint_elem ) | 'ADD' 'CONSTRAINT' 'IF' 'NOT' 'EXISTS' constraint_name constraint_elem | 'ALTER' 'PRIMARY' 'KEY' 'USING' 'COLUMNS' '(' index_params ')' opt_hash_sharded | 'VALIDATE' 'CONSTRAINT' constraint_name | 'DROP' 'CONSTRAINT' 'IF' 'EXISTS' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'DROP' 'CONSTRAINT' constraint_name ( 'CASCADE' | 'RESTRICT' | ) | 'EXPERIMENTAL_AUDIT' 'SET' audit_mode | partition_by_table | 'SET' '(' storage_parameter_list ')' ) ) )* ) diff --git a/docs/generated/sql/bnf/alter_table_partition_by.bnf b/docs/generated/sql/bnf/alter_table_partition_by.bnf index 75d1c8f5450e..b844074b64f4 100644 --- a/docs/generated/sql/bnf/alter_table_partition_by.bnf +++ b/docs/generated/sql/bnf/alter_table_partition_by.bnf @@ -1,3 +1,3 @@ alter_onetable_stmt ::= - 'ALTER' 'TABLE' table_name 'PARTITION' 'ALL' 'BY' partition_by_inner ( ( ',' ( 'RENAME' opt_column column_name 'TO' column_name | 'RENAME' 'CONSTRAINT' column_name 'TO' column_name | 'ADD' column_def | 'ADD' 'IF' 'NOT' 'EXISTS' column_def | 'ADD' 'COLUMN' column_def | 'ADD' 'COLUMN' 'IF' 'NOT' 'EXISTS' column_def | 'ALTER' opt_column column_name alter_column_default | 'ALTER' opt_column column_name alter_column_on_update | 'ALTER' opt_column column_name alter_column_visible | 'ALTER' opt_column column_name 'DROP' 'NOT' 'NULL' | 'ALTER' opt_column column_name 'DROP' 'STORED' | 'ALTER' opt_column column_name 'SET' 'NOT' 'NULL' | 'DROP' opt_column 'IF' 'EXISTS' column_name opt_drop_behavior | 'DROP' opt_column column_name opt_drop_behavior | 'ALTER' opt_column column_name opt_set_data 'TYPE' typename opt_collate opt_alter_column_using | 'ADD' table_constraint opt_validate_behavior | 'ADD' 'CONSTRAINT' 'IF' 'NOT' 'EXISTS' constraint_name constraint_elem opt_validate_behavior | 'ALTER' 'PRIMARY' 'KEY' 'USING' 'COLUMNS' '(' index_params ')' opt_hash_sharded | 'VALIDATE' 'CONSTRAINT' constraint_name | 'DROP' 'CONSTRAINT' 'IF' 'EXISTS' constraint_name opt_drop_behavior | 'DROP' 'CONSTRAINT' constraint_name opt_drop_behavior | 'EXPERIMENTAL_AUDIT' 'SET' audit_mode | ( partition_by | 'PARTITION' 'ALL' 'BY' partition_by_inner ) ) ) )* - | 'ALTER' 'TABLE' 'IF' 'EXISTS' table_name 'PARTITION' 'ALL' 'BY' partition_by_inner ( ( ',' ( 'RENAME' opt_column column_name 'TO' column_name | 'RENAME' 'CONSTRAINT' column_name 'TO' column_name | 'ADD' column_def | 'ADD' 'IF' 'NOT' 'EXISTS' column_def | 'ADD' 'COLUMN' column_def | 'ADD' 'COLUMN' 'IF' 'NOT' 'EXISTS' column_def | 'ALTER' opt_column column_name alter_column_default | 'ALTER' opt_column column_name alter_column_on_update | 'ALTER' opt_column column_name alter_column_visible | 'ALTER' opt_column column_name 'DROP' 'NOT' 'NULL' | 'ALTER' opt_column column_name 'DROP' 'STORED' | 'ALTER' opt_column column_name 'SET' 'NOT' 'NULL' | 'DROP' opt_column 'IF' 'EXISTS' column_name opt_drop_behavior | 'DROP' opt_column column_name opt_drop_behavior | 'ALTER' opt_column column_name opt_set_data 'TYPE' typename opt_collate opt_alter_column_using | 'ADD' table_constraint opt_validate_behavior | 'ADD' 'CONSTRAINT' 'IF' 'NOT' 'EXISTS' constraint_name constraint_elem opt_validate_behavior | 'ALTER' 'PRIMARY' 'KEY' 'USING' 'COLUMNS' '(' index_params ')' opt_hash_sharded | 'VALIDATE' 'CONSTRAINT' constraint_name | 'DROP' 'CONSTRAINT' 'IF' 'EXISTS' constraint_name opt_drop_behavior | 'DROP' 'CONSTRAINT' constraint_name opt_drop_behavior | 'EXPERIMENTAL_AUDIT' 'SET' audit_mode | ( partition_by | 'PARTITION' 'ALL' 'BY' partition_by_inner ) ) ) )* + 'ALTER' 'TABLE' table_name 'PARTITION' 'ALL' 'BY' partition_by_inner ( ( ',' ( 'RENAME' opt_column column_name 'TO' column_name | 'RENAME' 'CONSTRAINT' column_name 'TO' column_name | 'ADD' column_def | 'ADD' 'IF' 'NOT' 'EXISTS' column_def | 'ADD' 'COLUMN' column_def | 'ADD' 'COLUMN' 'IF' 'NOT' 'EXISTS' column_def | 'ALTER' opt_column column_name alter_column_default | 'ALTER' opt_column column_name alter_column_on_update | 'ALTER' opt_column column_name alter_column_visible | 'ALTER' opt_column column_name 'DROP' 'NOT' 'NULL' | 'ALTER' opt_column column_name 'DROP' 'STORED' | 'ALTER' opt_column column_name 'SET' 'NOT' 'NULL' | 'DROP' opt_column 'IF' 'EXISTS' column_name opt_drop_behavior | 'DROP' opt_column column_name opt_drop_behavior | 'ALTER' opt_column column_name opt_set_data 'TYPE' typename opt_collate opt_alter_column_using | 'ADD' table_constraint opt_validate_behavior | 'ADD' 'CONSTRAINT' 'IF' 'NOT' 'EXISTS' constraint_name constraint_elem opt_validate_behavior | 'ALTER' 'PRIMARY' 'KEY' 'USING' 'COLUMNS' '(' index_params ')' opt_hash_sharded | 'VALIDATE' 'CONSTRAINT' constraint_name | 'DROP' 'CONSTRAINT' 'IF' 'EXISTS' constraint_name opt_drop_behavior | 'DROP' 'CONSTRAINT' constraint_name opt_drop_behavior | 'EXPERIMENTAL_AUDIT' 'SET' audit_mode | ( partition_by | 'PARTITION' 'ALL' 'BY' partition_by_inner ) | 'SET' '(' storage_parameter_list ')' ) ) )* + | 'ALTER' 'TABLE' 'IF' 'EXISTS' table_name 'PARTITION' 'ALL' 'BY' partition_by_inner ( ( ',' ( 'RENAME' opt_column column_name 'TO' column_name | 'RENAME' 'CONSTRAINT' column_name 'TO' column_name | 'ADD' column_def | 'ADD' 'IF' 'NOT' 'EXISTS' column_def | 'ADD' 'COLUMN' column_def | 'ADD' 'COLUMN' 'IF' 'NOT' 'EXISTS' column_def | 'ALTER' opt_column column_name alter_column_default | 'ALTER' opt_column column_name alter_column_on_update | 'ALTER' opt_column column_name alter_column_visible | 'ALTER' opt_column column_name 'DROP' 'NOT' 'NULL' | 'ALTER' opt_column column_name 'DROP' 'STORED' | 'ALTER' opt_column column_name 'SET' 'NOT' 'NULL' | 'DROP' opt_column 'IF' 'EXISTS' column_name opt_drop_behavior | 'DROP' opt_column column_name opt_drop_behavior | 'ALTER' opt_column column_name opt_set_data 'TYPE' typename opt_collate opt_alter_column_using | 'ADD' table_constraint opt_validate_behavior | 'ADD' 'CONSTRAINT' 'IF' 'NOT' 'EXISTS' constraint_name constraint_elem opt_validate_behavior | 'ALTER' 'PRIMARY' 'KEY' 'USING' 'COLUMNS' '(' index_params ')' opt_hash_sharded | 'VALIDATE' 'CONSTRAINT' constraint_name | 'DROP' 'CONSTRAINT' 'IF' 'EXISTS' constraint_name opt_drop_behavior | 'DROP' 'CONSTRAINT' constraint_name opt_drop_behavior | 'EXPERIMENTAL_AUDIT' 'SET' audit_mode | ( partition_by | 'PARTITION' 'ALL' 'BY' partition_by_inner ) | 'SET' '(' storage_parameter_list ')' ) ) )* diff --git a/docs/generated/sql/bnf/stmt_block.bnf b/docs/generated/sql/bnf/stmt_block.bnf index 0be91db8fd2b..786f6ace7910 100644 --- a/docs/generated/sql/bnf/stmt_block.bnf +++ b/docs/generated/sql/bnf/stmt_block.bnf @@ -2688,6 +2688,7 @@ alter_table_cmd ::= | 'DROP' 'CONSTRAINT' constraint_name opt_drop_behavior | 'EXPERIMENTAL_AUDIT' 'SET' audit_mode | partition_by_table + | 'SET' '(' storage_parameter_list ')' var_set_list ::= ( var_name '=' 'COPY' 'FROM' 'PARENT' | var_name '=' var_value ) ( ( ',' var_name '=' var_value | ',' var_name '=' 'COPY' 'FROM' 'PARENT' ) )* diff --git a/pkg/sql/alter_table.go b/pkg/sql/alter_table.go index 235bbdf11c03..282043afc5d3 100644 --- a/pkg/sql/alter_table.go +++ b/pkg/sql/alter_table.go @@ -27,6 +27,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/catalog/resolver" "github.com/cockroachdb/cockroach/pkg/sql/catalog/schemaexpr" "github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc" + "github.com/cockroachdb/cockroach/pkg/sql/paramparse" "github.com/cockroachdb/cockroach/pkg/sql/parser" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" @@ -966,6 +967,17 @@ func (n *alterTableNode) startExec(params runParams) error { return err } + case *tree.AlterTableSetStorageParams: + if err := paramparse.ApplyStorageParameters( + params.ctx, + params.p.SemaCtx(), + params.EvalContext(), + t.StorageParams, + ¶mparse.TableStorageParamObserver{}, + ); err != nil { + return err + } + case *tree.AlterTableRenameColumn: descChanged, err := params.p.renameColumn(params.ctx, n.tableDesc, t.Column, t.NewName) if err != nil { diff --git a/pkg/sql/logictest/testdata/logic_test/alter_table b/pkg/sql/logictest/testdata/logic_test/alter_table index 9cda7c58009f..894bdcc43a01 100644 --- a/pkg/sql/logictest/testdata/logic_test/alter_table +++ b/pkg/sql/logictest/testdata/logic_test/alter_table @@ -2148,3 +2148,26 @@ SELECT * FROM multipleinstmt ORDER BY id ASC; 1 a a false NULL true NULL 2 b b false NULL true NULL 3 c c false NULL true NULL + +subtest storage_params + +statement ok +CREATE TABLE storage_param_table() + +statement error pgcode 22023 invalid storage parameter "foo" +ALTER TABLE storage_param_table SET (foo=100) + +statement error parameter "fillfactor" requires a float value +ALTER TABLE storage_param_table SET (fillfactor=true) + +statement error unimplemented: storage parameter "toast_tuple_target" +ALTER TABLE storage_param_table SET (toast_tuple_target=100) + +query T noticetrace +ALTER TABLE storage_param_table SET (fillfactor=99.9, autovacuum_enabled = off) +---- +NOTICE: storage parameter "fillfactor" is ignored +NOTICE: storage parameter "autovacuum_enabled = 'off'" is ignored + +statement error parameter "autovacuum_enabled" requires a Boolean value +ALTER TABLE storage_param_table SET (autovacuum_enabled='11') diff --git a/pkg/sql/parser/sql.y b/pkg/sql/parser/sql.y index 6bf38b20d8ed..70dc92efe33f 100644 --- a/pkg/sql/parser/sql.y +++ b/pkg/sql/parser/sql.y @@ -1552,6 +1552,7 @@ alter_ddl_stmt: // ALTER TABLE ... RENAME TO // ALTER TABLE ... RENAME [COLUMN] TO // ALTER TABLE ... VALIDATE CONSTRAINT +// ALTER TABLE ... SET (storage_param = value, ...) // ALTER TABLE ... SPLIT AT [WITH EXPIRATION ] // ALTER TABLE ... UNSPLIT AT // ALTER TABLE ... UNSPLIT ALL @@ -2365,6 +2366,12 @@ alter_table_cmd: Stats: $3.expr(), } } +| SET '(' storage_parameter_list ')' + { + $$.val = &tree.AlterTableSetStorageParams{ + StorageParams: $3.storageParams(), + } + } audit_mode: READ WRITE { $$.val = tree.AuditModeReadWrite } diff --git a/pkg/sql/parser/testdata/alter_table b/pkg/sql/parser/testdata/alter_table index 908bcb2f3159..935df43de29d 100644 --- a/pkg/sql/parser/testdata/alter_table +++ b/pkg/sql/parser/testdata/alter_table @@ -1106,6 +1106,14 @@ ALTER TABLE t EXPERIMENTAL_AUDIT SET OFF -- fully parenthesized ALTER TABLE t EXPERIMENTAL_AUDIT SET OFF -- literals removed ALTER TABLE _ EXPERIMENTAL_AUDIT SET OFF -- identifiers removed +parse +ALTER TABLE t SET (fillfactor = 100, autovacuum_enabled = false) +---- +ALTER TABLE t SET (fillfactor = 100, autovacuum_enabled = false) +ALTER TABLE t SET (fillfactor = (100), autovacuum_enabled = (false)) -- fully parenthesized +ALTER TABLE t SET (fillfactor = _, autovacuum_enabled = _) -- literals removed +ALTER TABLE _ SET (_ = 100, _ = false) -- identifiers removed + error ALTER PARTITION p OF TABLE tbl@idx CONFIGURE ZONE USING num_replicas = 1 ---- diff --git a/pkg/sql/sem/tree/alter_table.go b/pkg/sql/sem/tree/alter_table.go index f08b9ad6ac41..45d1493ff119 100644 --- a/pkg/sql/sem/tree/alter_table.go +++ b/pkg/sql/sem/tree/alter_table.go @@ -77,6 +77,7 @@ func (*AlterTableSetVisible) alterTableCmd() {} func (*AlterTableValidateConstraint) alterTableCmd() {} func (*AlterTablePartitionByTable) alterTableCmd() {} func (*AlterTableInjectStats) alterTableCmd() {} +func (*AlterTableSetStorageParams) alterTableCmd() {} var _ AlterTableCmd = &AlterTableAddColumn{} var _ AlterTableCmd = &AlterTableAddConstraint{} @@ -95,6 +96,7 @@ var _ AlterTableCmd = &AlterTableSetVisible{} var _ AlterTableCmd = &AlterTableValidateConstraint{} var _ AlterTableCmd = &AlterTablePartitionByTable{} var _ AlterTableCmd = &AlterTableInjectStats{} +var _ AlterTableCmd = &AlterTableSetStorageParams{} // ColumnMutationCmd is the subset of AlterTableCmds that modify an // existing column. @@ -604,6 +606,24 @@ func (node *AlterTableInjectStats) Format(ctx *FmtCtx) { ctx.FormatNode(node.Stats) } +// AlterTableSetStorageParams represents a ALTER TABLE SET command. +type AlterTableSetStorageParams struct { + StorageParams StorageParams +} + +// TelemetryCounter returns the telemetry counter to increment +// when this command is used. +func (node *AlterTableSetStorageParams) TelemetryCounter() telemetry.Counter { + return sqltelemetry.SchemaChangeAlterCounter("set_storage_param") +} + +// Format implements the NodeFormatter interface. +func (node *AlterTableSetStorageParams) Format(ctx *FmtCtx) { + ctx.WriteString(" SET (") + ctx.FormatNode(&node.StorageParams) + ctx.WriteString(")") +} + // AlterTableLocality represents an ALTER TABLE LOCALITY command. type AlterTableLocality struct { Name *UnresolvedObjectName