From ab5b9bbfc3351b2e542786ca4bab89fb83f45b0c Mon Sep 17 00:00:00 2001 From: Andrew Werner Date: Mon, 6 Jun 2022 19:33:24 -0400 Subject: [PATCH] sql/storageparam: break builtins dep on tabledesc The TableStorageParamObserver meant that paramparse and transitively builtins depended on tabledesc. This was unfortunate because we want seqexpr is depended on by builtins and we want to use seqexpr in tabledesc, so we have to make sure that builtins does not depend on tabledesc. This commit achieves that goal by splitting out paramparse into three new packages from paramparse: * sql/storageparam: defines the interface for the Setter, contains functions to drive forward the setting and resetting of params, and has some shared functionality. * sql/storageparam/indexstorageparam: implementation of storageparam.Setter for the `descpb.IndexDescriptor`. * sql/storageparam/tablestorageparam: implementation of storageparam.Setter for the `*tabledesc.Mutable`. This allows the `builtins` package to use the `indexstorageparam` package cleanly without depending on `*tabledesc.Mutable`. It also recognizes that lots of utility methods in `paramparse` aren't about `storageparam`s. Release note: None --- pkg/sql/BUILD.bazel | 3 + pkg/sql/alter_table.go | 11 +- pkg/sql/create_index.go | 7 +- pkg/sql/create_table.go | 11 +- pkg/sql/paramparse/BUILD.bazel | 12 - pkg/sql/paramparse/paramparse.go | 3 +- pkg/sql/sem/builtins/BUILD.bazel | 7 +- pkg/sql/sem/builtins/geo_builtins.go | 7 +- pkg/sql/storageparam/BUILD.bazel | 20 + .../indexstorageparam/BUILD.bazel | 20 + .../indexstorageparam/index_storage_param.go | 182 ++++++++ pkg/sql/storageparam/storage_param.go | 114 +++++ .../tablestorageparam/BUILD.bazel | 23 + .../tablestorageparam/table_storage_param.go} | 393 ++++-------------- 14 files changed, 464 insertions(+), 349 deletions(-) create mode 100644 pkg/sql/storageparam/BUILD.bazel create mode 100644 pkg/sql/storageparam/indexstorageparam/BUILD.bazel create mode 100644 pkg/sql/storageparam/indexstorageparam/index_storage_param.go create mode 100644 pkg/sql/storageparam/storage_param.go create mode 100644 pkg/sql/storageparam/tablestorageparam/BUILD.bazel rename pkg/sql/{paramparse/paramobserver.go => storageparam/tablestorageparam/table_storage_param.go} (50%) diff --git a/pkg/sql/BUILD.bazel b/pkg/sql/BUILD.bazel index 9081571fe766..dd593a2f2336 100644 --- a/pkg/sql/BUILD.bazel +++ b/pkg/sql/BUILD.bazel @@ -416,6 +416,9 @@ go_library( "//pkg/sql/sqlutil", "//pkg/sql/stats", "//pkg/sql/stmtdiagnostics", + "//pkg/sql/storageparam", + "//pkg/sql/storageparam/indexstorageparam", + "//pkg/sql/storageparam/tablestorageparam", "//pkg/sql/types", "//pkg/sql/vtable", "//pkg/storage/enginepb", diff --git a/pkg/sql/alter_table.go b/pkg/sql/alter_table.go index cbe3f09acb99..5be85d892a35 100644 --- a/pkg/sql/alter_table.go +++ b/pkg/sql/alter_table.go @@ -30,7 +30,6 @@ 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" @@ -43,6 +42,8 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/sqlerrors" "github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry" "github.com/cockroachdb/cockroach/pkg/sql/stats" + "github.com/cockroachdb/cockroach/pkg/sql/storageparam" + "github.com/cockroachdb/cockroach/pkg/sql/storageparam/tablestorageparam" "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented" "github.com/cockroachdb/cockroach/pkg/util/log/eventpb" @@ -727,12 +728,12 @@ func (n *alterTableNode) startExec(params runParams) error { if ttl := n.tableDesc.GetRowLevelTTL(); ttl != nil { ttlBefore = protoutil.Clone(ttl).(*catpb.RowLevelTTL) } - if err := paramparse.SetStorageParameters( + if err := storageparam.Set( params.ctx, params.p.SemaCtx(), params.EvalContext(), t.StorageParams, - paramparse.NewTableStorageParamObserver(n.tableDesc), + tablestorageparam.NewSetter(n.tableDesc), ); err != nil { return err } @@ -761,11 +762,11 @@ func (n *alterTableNode) startExec(params runParams) error { if ttl := n.tableDesc.GetRowLevelTTL(); ttl != nil { ttlBefore = protoutil.Clone(ttl).(*catpb.RowLevelTTL) } - if err := paramparse.ResetStorageParameters( + if err := storageparam.Reset( params.ctx, params.EvalContext(), t.Params, - paramparse.NewTableStorageParamObserver(n.tableDesc), + tablestorageparam.NewSetter(n.tableDesc), ); err != nil { return err } diff --git a/pkg/sql/create_index.go b/pkg/sql/create_index.go index b1781104ae01..7fc78a51642d 100644 --- a/pkg/sql/create_index.go +++ b/pkg/sql/create_index.go @@ -23,7 +23,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" "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/pgwire/pgcode" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgnotice" @@ -32,6 +31,8 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sqlerrors" "github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry" + "github.com/cockroachdb/cockroach/pkg/sql/storageparam" + "github.com/cockroachdb/cockroach/pkg/sql/storageparam/indexstorageparam" "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/cockroach/pkg/util/log/eventpb" "github.com/cockroachdb/errors" @@ -267,12 +268,12 @@ func makeIndexDescriptor( return nil, err } - if err := paramparse.SetStorageParameters( + if err := storageparam.Set( params.ctx, params.p.SemaCtx(), params.EvalContext(), n.StorageParams, - ¶mparse.IndexStorageParamObserver{IndexDesc: &indexDesc}, + &indexstorageparam.Setter{IndexDesc: &indexDesc}, ); err != nil { return nil, err } diff --git a/pkg/sql/create_table.go b/pkg/sql/create_table.go index ad4169d6cf3f..a7c4a0e82e40 100644 --- a/pkg/sql/create_table.go +++ b/pkg/sql/create_table.go @@ -55,6 +55,9 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/sessiondata" "github.com/cockroachdb/cockroach/pkg/sql/sqlerrors" "github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry" + "github.com/cockroachdb/cockroach/pkg/sql/storageparam" + "github.com/cockroachdb/cockroach/pkg/sql/storageparam/indexstorageparam" + "github.com/cockroachdb/cockroach/pkg/sql/storageparam/tablestorageparam" "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented" "github.com/cockroachdb/cockroach/pkg/util/hlc" @@ -1313,12 +1316,12 @@ func NewTableDesc( id, dbID, sc.GetID(), n.Table.Table(), creationTime, privileges, persistence, ) - if err := paramparse.SetStorageParameters( + if err := storageparam.Set( ctx, semaCtx, evalCtx, n.StorageParams, - paramparse.NewTableStorageParamObserver(&desc), + tablestorageparam.NewSetter(&desc), ); err != nil { return nil, err } @@ -1854,12 +1857,12 @@ func NewTableDesc( } idx.Predicate = expr } - if err := paramparse.SetStorageParameters( + if err := storageparam.Set( ctx, semaCtx, evalCtx, d.StorageParams, - ¶mparse.IndexStorageParamObserver{IndexDesc: &idx}, + &indexstorageparam.Setter{IndexDesc: &idx}, ); err != nil { return nil, err } diff --git a/pkg/sql/paramparse/BUILD.bazel b/pkg/sql/paramparse/BUILD.bazel index 0941534f405c..d39528151f9f 100644 --- a/pkg/sql/paramparse/BUILD.bazel +++ b/pkg/sql/paramparse/BUILD.bazel @@ -3,29 +3,17 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( name = "paramparse", srcs = [ - "paramobserver.go", "paramparse.go", "validation.go", ], importpath = "github.com/cockroachdb/cockroach/pkg/sql/paramparse", visibility = ["//visibility:public"], deps = [ - "//pkg/geo/geoindex", - "//pkg/server/telemetry", - "//pkg/settings", - "//pkg/sql/catalog/catpb", - "//pkg/sql/catalog/descpb", - "//pkg/sql/catalog/tabledesc", "//pkg/sql/pgwire/pgcode", "//pkg/sql/pgwire/pgerror", - "//pkg/sql/pgwire/pgnotice", "//pkg/sql/sem/eval", - "//pkg/sql/sem/normalize", "//pkg/sql/sem/tree", - "//pkg/sql/sqltelemetry", - "//pkg/sql/types", "//pkg/util/duration", - "//pkg/util/errorutil/unimplemented", "@com_github_cockroachdb_errors//:errors", ], ) diff --git a/pkg/sql/paramparse/paramparse.go b/pkg/sql/paramparse/paramparse.go index fff9c751c011..82c2efbdcde6 100644 --- a/pkg/sql/paramparse/paramparse.go +++ b/pkg/sql/paramparse/paramparse.go @@ -8,8 +8,7 @@ // by the Apache License, Version 2.0, included in the file // licenses/APL.txt. -// Package paramparse parses parameters that are set in param lists -// or session vars. +// Package paramparse provides utilities for parsing storage paramaters. package paramparse import ( diff --git a/pkg/sql/sem/builtins/BUILD.bazel b/pkg/sql/sem/builtins/BUILD.bazel index 836b095abaf4..2806254fdeb4 100644 --- a/pkg/sql/sem/builtins/BUILD.bazel +++ b/pkg/sql/sem/builtins/BUILD.bazel @@ -56,7 +56,6 @@ go_library( "//pkg/sql/lex", "//pkg/sql/lexbase", "//pkg/sql/memsize", - "//pkg/sql/paramparse", "//pkg/sql/parser", "//pkg/sql/pgwire/pgcode", "//pkg/sql/pgwire/pgerror", @@ -78,6 +77,8 @@ go_library( "//pkg/sql/sqlliveness", "//pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil", "//pkg/sql/sqltelemetry", + "//pkg/sql/storageparam", + "//pkg/sql/storageparam/indexstorageparam", "//pkg/sql/types", "//pkg/streaming", "//pkg/util", @@ -174,5 +175,9 @@ disallowed_imports_test( src = "builtins", disallowed_list = [ "//pkg/sql/catalog/descs", + "//pkg/sql/catalog/tabledesc", + "//pkg/sql/catalog/schemadesc", + "//pkg/sql/catalog/dbdesc", + "//pkg/sql/catalog/typedesc", ], ) diff --git a/pkg/sql/sem/builtins/geo_builtins.go b/pkg/sql/sem/builtins/geo_builtins.go index 9745566221cc..f000ac0b4b61 100644 --- a/pkg/sql/sem/builtins/geo_builtins.go +++ b/pkg/sql/sem/builtins/geo_builtins.go @@ -27,13 +27,14 @@ import ( "github.com/cockroachdb/cockroach/pkg/geo/twkb" "github.com/cockroachdb/cockroach/pkg/kv" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" - "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" "github.com/cockroachdb/cockroach/pkg/sql/sem/eval" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sem/volatility" + "github.com/cockroachdb/cockroach/pkg/sql/storageparam" + "github.com/cockroachdb/cockroach/pkg/sql/storageparam/indexstorageparam" "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/cockroach/pkg/util" "github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented" @@ -7671,12 +7672,12 @@ func applyGeoindexConfigStorageParams( return geoindex.Config{}, errors.Newf("invalid storage parameters specified: %s", params) } semaCtx := tree.MakeSemaContext() - if err := paramparse.SetStorageParameters( + if err := storageparam.Set( evalCtx.Context, &semaCtx, evalCtx, stmt.AST.(*tree.CreateIndex).StorageParams, - ¶mparse.IndexStorageParamObserver{IndexDesc: indexDesc}, + &indexstorageparam.Setter{IndexDesc: indexDesc}, ); err != nil { return geoindex.Config{}, err } diff --git a/pkg/sql/storageparam/BUILD.bazel b/pkg/sql/storageparam/BUILD.bazel new file mode 100644 index 000000000000..e610e307735e --- /dev/null +++ b/pkg/sql/storageparam/BUILD.bazel @@ -0,0 +1,20 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +go_library( + name = "storageparam", + srcs = ["storage_param.go"], + importpath = "github.com/cockroachdb/cockroach/pkg/sql/storageparam", + visibility = ["//visibility:public"], + deps = [ + "//pkg/server/telemetry", + "//pkg/sql/paramparse", + "//pkg/sql/pgwire/pgcode", + "//pkg/sql/pgwire/pgerror", + "//pkg/sql/pgwire/pgnotice", + "//pkg/sql/sem/eval", + "//pkg/sql/sem/normalize", + "//pkg/sql/sem/tree", + "//pkg/sql/sqltelemetry", + "//pkg/sql/types", + ], +) diff --git a/pkg/sql/storageparam/indexstorageparam/BUILD.bazel b/pkg/sql/storageparam/indexstorageparam/BUILD.bazel new file mode 100644 index 000000000000..44ceeb4b73b2 --- /dev/null +++ b/pkg/sql/storageparam/indexstorageparam/BUILD.bazel @@ -0,0 +1,20 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +go_library( + name = "indexstorageparam", + srcs = ["index_storage_param.go"], + importpath = "github.com/cockroachdb/cockroach/pkg/sql/storageparam/indexstorageparam", + visibility = ["//visibility:public"], + deps = [ + "//pkg/geo/geoindex", + "//pkg/sql/catalog/descpb", + "//pkg/sql/paramparse", + "//pkg/sql/pgwire/pgcode", + "//pkg/sql/pgwire/pgerror", + "//pkg/sql/sem/eval", + "//pkg/sql/sem/tree", + "//pkg/sql/storageparam", + "//pkg/util/errorutil/unimplemented", + "@com_github_cockroachdb_errors//:errors", + ], +) diff --git a/pkg/sql/storageparam/indexstorageparam/index_storage_param.go b/pkg/sql/storageparam/indexstorageparam/index_storage_param.go new file mode 100644 index 000000000000..8ed0448eb9ba --- /dev/null +++ b/pkg/sql/storageparam/indexstorageparam/index_storage_param.go @@ -0,0 +1,182 @@ +// 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 indexstorageparam implements storageparam.Setter for a +// descpb.IndexDescriptor. +package indexstorageparam + +import ( + "context" + + "github.com/cockroachdb/cockroach/pkg/geo/geoindex" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" + "github.com/cockroachdb/cockroach/pkg/sql/paramparse" + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" + "github.com/cockroachdb/cockroach/pkg/sql/sem/eval" + "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" + "github.com/cockroachdb/cockroach/pkg/sql/storageparam" + "github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented" + "github.com/cockroachdb/errors" +) + +// Setter observes storage parameters for indexes. +type Setter struct { + IndexDesc *descpb.IndexDescriptor +} + +var _ storageparam.Setter = (*Setter)(nil) + +func getS2ConfigFromIndex(indexDesc *descpb.IndexDescriptor) *geoindex.S2Config { + var s2Config *geoindex.S2Config + if indexDesc.GeoConfig.S2Geometry != nil { + s2Config = indexDesc.GeoConfig.S2Geometry.S2Config + } + if indexDesc.GeoConfig.S2Geography != nil { + s2Config = indexDesc.GeoConfig.S2Geography.S2Config + } + return s2Config +} + +func (po *Setter) applyS2ConfigSetting( + evalCtx *eval.Context, key string, expr tree.Datum, min int64, max int64, +) error { + s2Config := getS2ConfigFromIndex(po.IndexDesc) + if s2Config == nil { + return pgerror.Newf( + pgcode.InvalidParameterValue, + "index setting %q can only be set on GEOMETRY or GEOGRAPHY spatial indexes", + key, + ) + } + + val, err := paramparse.DatumAsInt(evalCtx, key, expr) + if err != nil { + return errors.Wrapf(err, "error decoding %q", key) + } + if val < min || val > max { + return pgerror.Newf( + pgcode.InvalidParameterValue, + "%q value must be between %d and %d inclusive", + key, + min, + max, + ) + } + switch key { + case `s2_max_level`: + s2Config.MaxLevel = int32(val) + case `s2_level_mod`: + s2Config.LevelMod = int32(val) + case `s2_max_cells`: + s2Config.MaxCells = int32(val) + } + + return nil +} + +func (po *Setter) applyGeometryIndexSetting( + evalCtx *eval.Context, key string, expr tree.Datum, +) error { + if po.IndexDesc.GeoConfig.S2Geometry == nil { + return pgerror.Newf(pgcode.InvalidParameterValue, "%q can only be applied to GEOMETRY spatial indexes", key) + } + val, err := paramparse.DatumAsFloat(evalCtx, key, expr) + if err != nil { + return errors.Wrapf(err, "error decoding %q", key) + } + switch key { + case `geometry_min_x`: + po.IndexDesc.GeoConfig.S2Geometry.MinX = val + case `geometry_max_x`: + po.IndexDesc.GeoConfig.S2Geometry.MaxX = val + case `geometry_min_y`: + po.IndexDesc.GeoConfig.S2Geometry.MinY = val + case `geometry_max_y`: + po.IndexDesc.GeoConfig.S2Geometry.MaxY = val + default: + return pgerror.Newf(pgcode.InvalidParameterValue, "unknown key: %q", key) + } + return nil +} + +// Set implements the Setter interface. +func (po *Setter) Set( + ctx context.Context, + semaCtx *tree.SemaContext, + evalCtx *eval.Context, + key string, + expr tree.Datum, +) error { + switch key { + case `fillfactor`: + return storageparam.SetFillFactor(evalCtx, key, expr) + case `s2_max_level`: + return po.applyS2ConfigSetting(evalCtx, key, expr, 0, 30) + case `s2_level_mod`: + return po.applyS2ConfigSetting(evalCtx, key, expr, 1, 3) + case `s2_max_cells`: + return po.applyS2ConfigSetting(evalCtx, key, expr, 1, 32) + case `geometry_min_x`, `geometry_max_x`, `geometry_min_y`, `geometry_max_y`: + return po.applyGeometryIndexSetting(evalCtx, key, expr) + // `bucket_count` is handled in schema changer when creating hash sharded + // indexes. + case `bucket_count`: + return nil + case `vacuum_cleanup_index_scale_factor`, + `buffering`, + `fastupdate`, + `gin_pending_list_limit`, + `pages_per_range`, + `autosummarize`: + return unimplemented.NewWithIssuef(43299, "storage parameter %q", key) + } + return pgerror.Newf(pgcode.InvalidParameterValue, "invalid storage parameter %q", key) +} + +// Reset implements the StorageParameterObserver interface. +func (po *Setter) Reset(evalCtx *eval.Context, key string) error { + return errors.AssertionFailedf("non-implemented codepath") +} + +// RunPostChecks implements the Setter interface. +func (po *Setter) RunPostChecks() error { + s2Config := getS2ConfigFromIndex(po.IndexDesc) + if s2Config != nil { + if (s2Config.MaxLevel)%s2Config.LevelMod != 0 { + return pgerror.Newf( + pgcode.InvalidParameterValue, + "s2_max_level (%d) must be divisible by s2_level_mod (%d)", + s2Config.MaxLevel, + s2Config.LevelMod, + ) + } + } + + if cfg := po.IndexDesc.GeoConfig.S2Geometry; cfg != nil { + if cfg.MaxX <= cfg.MinX { + 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 pgerror.Newf( + pgcode.InvalidParameterValue, + "geometry_max_y (%f) must be greater than geometry_min_y (%f)", + cfg.MaxY, + cfg.MinY, + ) + } + } + return nil +} diff --git a/pkg/sql/storageparam/storage_param.go b/pkg/sql/storageparam/storage_param.go new file mode 100644 index 000000000000..423f8b094073 --- /dev/null +++ b/pkg/sql/storageparam/storage_param.go @@ -0,0 +1,114 @@ +// Copyright 2020 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 storageparam defines interfaces and functions for setting and +// resetting storage parameters. +package storageparam + +import ( + "context" + + "github.com/cockroachdb/cockroach/pkg/server/telemetry" + "github.com/cockroachdb/cockroach/pkg/sql/paramparse" + "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/eval" + "github.com/cockroachdb/cockroach/pkg/sql/sem/normalize" + "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" + "github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry" + "github.com/cockroachdb/cockroach/pkg/sql/types" +) + +// Setter applies a storage parameter to an underlying item. +type Setter interface { + // Set is called during CREATE [TABLE | INDEX] ... WITH (...) or + // ALTER [TABLE | INDEX] ... WITH (...). + Set(ctx context.Context, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error + // Reset is called during ALTER [TABLE | INDEX] ... RESET (...) + Reset(evalCtx *eval.Context, key string) error + // RunPostChecks is called after all storage parameters have been set. + // This allows checking whether multiple storage parameters together + // form a valid configuration. + RunPostChecks() error +} + +// Set sets the given storage parameters using the +// given observer. +func Set( + ctx context.Context, + semaCtx *tree.SemaContext, + evalCtx *eval.Context, + params tree.StorageParams, + setter Setter, +) error { + for _, sp := range params { + key := string(sp.Key) + if sp.Value == nil { + return pgerror.Newf(pgcode.InvalidParameterValue, "storage parameter %q requires a value", key) + } + telemetry.Inc(sqltelemetry.SetTableStorageParameter(key)) + + // Expressions may be an unresolved name. + // Cast these as strings. + expr := paramparse.UnresolvedNameToStrVal(sp.Value) + + // Convert the expressions to a datum. + typedExpr, err := tree.TypeCheck(ctx, expr, semaCtx, types.Any) + if err != nil { + return err + } + if typedExpr, err = normalize.Expr(evalCtx, typedExpr); err != nil { + return err + } + datum, err := eval.Expr(evalCtx, typedExpr) + if err != nil { + return err + } + + if err := setter.Set(ctx, semaCtx, evalCtx, key, datum); err != nil { + return err + } + } + return setter.RunPostChecks() +} + +// Reset sets the given storage parameters using the +// given observer. +func Reset( + ctx context.Context, evalCtx *eval.Context, params tree.NameList, paramObserver Setter, +) error { + for _, p := range params { + telemetry.Inc(sqltelemetry.ResetTableStorageParameter(string(p))) + if err := paramObserver.Reset(evalCtx, string(p)); err != nil { + return err + } + } + return paramObserver.RunPostChecks() +} + +// SetFillFactor validates the fill_factor storage param and then issues a +// notice. +func SetFillFactor(evalCtx *eval.Context, key string, datum tree.Datum) error { + val, err := paramparse.DatumAsFloat(evalCtx, key, datum) + if err != nil { + return err + } + if val < 0 || val > 100 { + return pgerror.Newf(pgcode.InvalidParameterValue, "%q must be between 0 and 100", key) + } + if evalCtx != nil { + evalCtx.ClientNoticeSender.BufferClientNotice( + evalCtx.Context, + pgnotice.Newf("storage parameter %q is ignored", key), + ) + } + return nil +} diff --git a/pkg/sql/storageparam/tablestorageparam/BUILD.bazel b/pkg/sql/storageparam/tablestorageparam/BUILD.bazel new file mode 100644 index 000000000000..8970d6033e83 --- /dev/null +++ b/pkg/sql/storageparam/tablestorageparam/BUILD.bazel @@ -0,0 +1,23 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +go_library( + name = "tablestorageparam", + srcs = ["table_storage_param.go"], + importpath = "github.com/cockroachdb/cockroach/pkg/sql/storageparam/tablestorageparam", + visibility = ["//visibility:public"], + deps = [ + "//pkg/settings", + "//pkg/sql/catalog/catpb", + "//pkg/sql/catalog/tabledesc", + "//pkg/sql/paramparse", + "//pkg/sql/pgwire/pgcode", + "//pkg/sql/pgwire/pgerror", + "//pkg/sql/pgwire/pgnotice", + "//pkg/sql/sem/eval", + "//pkg/sql/sem/tree", + "//pkg/sql/storageparam", + "//pkg/util/duration", + "//pkg/util/errorutil/unimplemented", + "@com_github_cockroachdb_errors//:errors", + ], +) diff --git a/pkg/sql/paramparse/paramobserver.go b/pkg/sql/storageparam/tablestorageparam/table_storage_param.go similarity index 50% rename from pkg/sql/paramparse/paramobserver.go rename to pkg/sql/storageparam/tablestorageparam/table_storage_param.go index 9efa5d9a8a95..d3a05c1a342f 100644 --- a/pkg/sql/paramparse/paramobserver.go +++ b/pkg/sql/storageparam/tablestorageparam/table_storage_param.go @@ -1,4 +1,4 @@ -// Copyright 2020 The Cockroach Authors. +// Copyright 2022 The Cockroach Authors. // // Use of this software is governed by the Business Source License // included in the file licenses/BSL.txt. @@ -8,116 +8,44 @@ // by the Apache License, Version 2.0, included in the file // licenses/APL.txt. -package paramparse +// Package tablestorageparam implements storageparam.Setter for +// tabledesc.Mutable. +package tablestorageparam import ( "context" "strings" - "github.com/cockroachdb/cockroach/pkg/geo/geoindex" - "github.com/cockroachdb/cockroach/pkg/server/telemetry" "github.com/cockroachdb/cockroach/pkg/settings" "github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb" - "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc" + "github.com/cockroachdb/cockroach/pkg/sql/paramparse" "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/eval" - "github.com/cockroachdb/cockroach/pkg/sql/sem/normalize" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" - "github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry" - "github.com/cockroachdb/cockroach/pkg/sql/types" + "github.com/cockroachdb/cockroach/pkg/sql/storageparam" "github.com/cockroachdb/cockroach/pkg/util/duration" "github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented" "github.com/cockroachdb/errors" ) -// SetStorageParameters sets the given storage parameters using the -// given observer. -func SetStorageParameters( - ctx context.Context, - semaCtx *tree.SemaContext, - evalCtx *eval.Context, - params tree.StorageParams, - paramObserver StorageParamObserver, -) error { - for _, sp := range params { - key := string(sp.Key) - if sp.Value == nil { - return pgerror.Newf(pgcode.InvalidParameterValue, "storage parameter %q requires a value", key) - } - telemetry.Inc(sqltelemetry.SetTableStorageParameter(key)) - - // Expressions may be an unresolved name. - // Cast these as strings. - expr := UnresolvedNameToStrVal(sp.Value) - - // Convert the expressions to a datum. - typedExpr, err := tree.TypeCheck(ctx, expr, semaCtx, types.Any) - if err != nil { - return err - } - if typedExpr, err = normalize.Expr(evalCtx, typedExpr); err != nil { - return err - } - datum, err := eval.Expr(evalCtx, typedExpr) - if err != nil { - return err - } - - if err := paramObserver.onSet(ctx, semaCtx, evalCtx, key, datum); err != nil { - return err - } - } - return paramObserver.runPostChecks() -} - -// ResetStorageParameters sets the given storage parameters using the -// given observer. -func ResetStorageParameters( - ctx context.Context, - evalCtx *eval.Context, - params tree.NameList, - paramObserver StorageParamObserver, -) error { - for _, p := range params { - telemetry.Inc(sqltelemetry.ResetTableStorageParameter(string(p))) - if err := paramObserver.onReset(evalCtx, string(p)); err != nil { - return err - } - } - return paramObserver.runPostChecks() -} - -// StorageParamObserver applies a storage parameter to an underlying item. -type StorageParamObserver interface { - // onSet is called during CREATE [TABLE | INDEX] ... WITH (...) or - // ALTER [TABLE | INDEX] ... WITH (...). - onSet(ctx context.Context, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error - // onReset is called during ALTER [TABLE | INDEX] ... RESET (...) - onReset(evalCtx *eval.Context, key string) error - // runPostChecks is called after all storage parameters have been set. - // This allows checking whether multiple storage parameters together - // form a valid configuration. - runPostChecks() error -} - -// TableStorageParamObserver observes storage parameters for tables. -type TableStorageParamObserver struct { +// Setter observes storage parameters for tables. +type Setter struct { tableDesc *tabledesc.Mutable setAutomaticColumn bool } -var _ StorageParamObserver = (*TableStorageParamObserver)(nil) +var _ storageparam.Setter = (*Setter)(nil) -// NewTableStorageParamObserver returns a new TableStorageParamObserver. -func NewTableStorageParamObserver(tableDesc *tabledesc.Mutable) *TableStorageParamObserver { - return &TableStorageParamObserver{tableDesc: tableDesc} +// NewSetter returns a new Setter. +func NewSetter(tableDesc *tabledesc.Mutable) *Setter { + return &Setter{tableDesc: tableDesc} } -// runPostChecks implements the StorageParamObserver interface. -func (po *TableStorageParamObserver) runPostChecks() error { +// RunPostChecks implements the Setter interface. +func (po *Setter) RunPostChecks() error { ttl := po.tableDesc.GetRowLevelTTL() if po.setAutomaticColumn && (ttl == nil || ttl.DurationExpr == "") { return pgerror.Newf( @@ -132,10 +60,10 @@ func (po *TableStorageParamObserver) runPostChecks() error { } func boolFromDatum(evalCtx *eval.Context, key string, datum tree.Datum) (bool, error) { - if stringVal, err := DatumAsString(evalCtx, key, datum); err == nil { - return ParseBoolVar(key, stringVal) + if stringVal, err := paramparse.DatumAsString(evalCtx, key, datum); err == nil { + return paramparse.ParseBoolVar(key, stringVal) } - s, err := GetSingleBool(key, datum) + s, err := paramparse.GetSingleBool(key, datum) if err != nil { return false, err } @@ -144,12 +72,12 @@ func boolFromDatum(evalCtx *eval.Context, key string, datum tree.Datum) (bool, e func intFromDatum(evalCtx *eval.Context, key string, datum tree.Datum) (int64, error) { intDatum := datum - if stringVal, err := DatumAsString(evalCtx, key, datum); err == nil { + if stringVal, err := paramparse.DatumAsString(evalCtx, key, datum); err == nil { if intDatum, err = tree.ParseDInt(stringVal); err != nil { return 0, errors.Wrapf(err, "invalid integer value for %s", key) } } - s, err := DatumAsInt(evalCtx, key, intDatum) + s, err := paramparse.DatumAsInt(evalCtx, key, intDatum) if err != nil { return 0, err } @@ -158,12 +86,12 @@ func intFromDatum(evalCtx *eval.Context, key string, datum tree.Datum) (int64, e func floatFromDatum(evalCtx *eval.Context, key string, datum tree.Datum) (float64, error) { floatDatum := datum - if stringVal, err := DatumAsString(evalCtx, key, datum); err == nil { + if stringVal, err := paramparse.DatumAsString(evalCtx, key, datum); err == nil { if floatDatum, err = tree.ParseDFloat(stringVal); err != nil { return 0, errors.Wrapf(err, "invalid float value for %s", key) } } - s, err := DatumAsFloat(evalCtx, key, floatDatum) + s, err := paramparse.DatumAsFloat(evalCtx, key, floatDatum) if err != nil { return 0, err } @@ -171,30 +99,30 @@ func floatFromDatum(evalCtx *eval.Context, key string, datum tree.Datum) (float6 } type tableParam struct { - onSet func(ctx context.Context, po *TableStorageParamObserver, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error - onReset func(po *TableStorageParamObserver, evalCtx *eval.Context, key string) error + onSet func(ctx context.Context, po *Setter, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error + onReset func(po *Setter, evalCtx *eval.Context, key string) error } var tableParams = map[string]tableParam{ `fillfactor`: { - onSet: func(ctx context.Context, po *TableStorageParamObserver, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error { - return setFillFactorStorageParam(evalCtx, key, datum) + onSet: func(ctx context.Context, po *Setter, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error { + return storageparam.SetFillFactor(evalCtx, key, datum) }, - onReset: func(po *TableStorageParamObserver, evalCtx *eval.Context, key string) error { + onReset: func(po *Setter, evalCtx *eval.Context, key string) error { // Operation is a no-op so do nothing. return nil }, }, `autovacuum_enabled`: { - onSet: func(ctx context.Context, po *TableStorageParamObserver, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error { + onSet: func(ctx context.Context, po *Setter, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error { var boolVal bool - if stringVal, err := DatumAsString(evalCtx, key, datum); err == nil { - boolVal, err = ParseBoolVar(key, stringVal) + if stringVal, err := paramparse.DatumAsString(evalCtx, key, datum); err == nil { + boolVal, err = paramparse.ParseBoolVar(key, stringVal) if err != nil { return err } } else { - s, err := GetSingleBool(key, datum) + s, err := paramparse.GetSingleBool(key, datum) if err != nil { return err } @@ -208,13 +136,13 @@ var tableParams = map[string]tableParam{ } return nil }, - onReset: func(po *TableStorageParamObserver, evalCtx *eval.Context, key string) error { + onReset: func(po *Setter, evalCtx *eval.Context, key string) error { // Operation is a no-op so do nothing. return nil }, }, `ttl`: { - onSet: func(ctx context.Context, po *TableStorageParamObserver, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error { + onSet: func(ctx context.Context, po *Setter, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error { setTrue, err := boolFromDatum(evalCtx, key, datum) if err != nil { return err @@ -229,13 +157,13 @@ var tableParams = map[string]tableParam{ } return nil }, - onReset: func(po *TableStorageParamObserver, evalCtx *eval.Context, key string) error { + onReset: func(po *Setter, evalCtx *eval.Context, key string) error { po.tableDesc.RowLevelTTL = nil return nil }, }, `ttl_automatic_column`: { - onSet: func(ctx context.Context, po *TableStorageParamObserver, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error { + onSet: func(ctx context.Context, po *Setter, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error { setTrue, err := boolFromDatum(evalCtx, key, datum) if err != nil { return err @@ -248,14 +176,14 @@ var tableParams = map[string]tableParam{ } return nil }, - onReset: func(po *TableStorageParamObserver, evalCtx *eval.Context, key string) error { + onReset: func(po *Setter, evalCtx *eval.Context, key string) error { return unimplemented.NewWithIssue(76916, "unsetting TTL automatic column not yet implemented") }, }, `ttl_expire_after`: { - onSet: func(ctx context.Context, po *TableStorageParamObserver, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error { + onSet: func(ctx context.Context, po *Setter, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error { var d *tree.DInterval - if stringVal, err := DatumAsString(evalCtx, key, datum); err == nil { + if stringVal, err := paramparse.DatumAsString(evalCtx, key, datum); err == nil { d, err = tree.ParseDInterval(evalCtx.SessionData().GetIntervalStyle(), stringVal) if err != nil || d == nil { return pgerror.Newf( @@ -288,7 +216,7 @@ var tableParams = map[string]tableParam{ po.tableDesc.RowLevelTTL.DurationExpr = catpb.Expression(tree.Serialize(d)) return nil }, - onReset: func(po *TableStorageParamObserver, evalCtx *eval.Context, key string) error { + onReset: func(po *Setter, evalCtx *eval.Context, key string) error { return errors.WithHintf( pgerror.Newf( pgcode.InvalidParameterValue, @@ -299,11 +227,11 @@ var tableParams = map[string]tableParam{ }, }, `ttl_select_batch_size`: { - onSet: func(ctx context.Context, po *TableStorageParamObserver, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error { + onSet: func(ctx context.Context, po *Setter, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error { if po.tableDesc.RowLevelTTL == nil { po.tableDesc.RowLevelTTL = &catpb.RowLevelTTL{} } - val, err := DatumAsInt(evalCtx, key, datum) + val, err := paramparse.DatumAsInt(evalCtx, key, datum) if err != nil { return err } @@ -313,7 +241,7 @@ var tableParams = map[string]tableParam{ po.tableDesc.RowLevelTTL.SelectBatchSize = val return nil }, - onReset: func(po *TableStorageParamObserver, evalCtx *eval.Context, key string) error { + onReset: func(po *Setter, evalCtx *eval.Context, key string) error { if po.tableDesc.RowLevelTTL != nil { po.tableDesc.RowLevelTTL.SelectBatchSize = 0 } @@ -321,11 +249,11 @@ var tableParams = map[string]tableParam{ }, }, `ttl_delete_batch_size`: { - onSet: func(ctx context.Context, po *TableStorageParamObserver, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error { + onSet: func(ctx context.Context, po *Setter, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error { if po.tableDesc.RowLevelTTL == nil { po.tableDesc.RowLevelTTL = &catpb.RowLevelTTL{} } - val, err := DatumAsInt(evalCtx, key, datum) + val, err := paramparse.DatumAsInt(evalCtx, key, datum) if err != nil { return err } @@ -335,7 +263,7 @@ var tableParams = map[string]tableParam{ po.tableDesc.RowLevelTTL.DeleteBatchSize = val return nil }, - onReset: func(po *TableStorageParamObserver, evalCtx *eval.Context, key string) error { + onReset: func(po *Setter, evalCtx *eval.Context, key string) error { if po.tableDesc.RowLevelTTL != nil { po.tableDesc.RowLevelTTL.DeleteBatchSize = 0 } @@ -343,11 +271,11 @@ var tableParams = map[string]tableParam{ }, }, `ttl_range_concurrency`: { - onSet: func(ctx context.Context, po *TableStorageParamObserver, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error { + onSet: func(ctx context.Context, po *Setter, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error { if po.tableDesc.RowLevelTTL == nil { po.tableDesc.RowLevelTTL = &catpb.RowLevelTTL{} } - val, err := DatumAsInt(evalCtx, key, datum) + val, err := paramparse.DatumAsInt(evalCtx, key, datum) if err != nil { return err } @@ -357,7 +285,7 @@ var tableParams = map[string]tableParam{ po.tableDesc.RowLevelTTL.RangeConcurrency = val return nil }, - onReset: func(po *TableStorageParamObserver, evalCtx *eval.Context, key string) error { + onReset: func(po *Setter, evalCtx *eval.Context, key string) error { if po.tableDesc.RowLevelTTL != nil { po.tableDesc.RowLevelTTL.RangeConcurrency = 0 } @@ -365,11 +293,11 @@ var tableParams = map[string]tableParam{ }, }, `ttl_delete_rate_limit`: { - onSet: func(ctx context.Context, po *TableStorageParamObserver, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error { + onSet: func(ctx context.Context, po *Setter, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error { if po.tableDesc.RowLevelTTL == nil { po.tableDesc.RowLevelTTL = &catpb.RowLevelTTL{} } - val, err := DatumAsInt(evalCtx, key, datum) + val, err := paramparse.DatumAsInt(evalCtx, key, datum) if err != nil { return err } @@ -379,7 +307,7 @@ var tableParams = map[string]tableParam{ po.tableDesc.RowLevelTTL.DeleteRateLimit = val return nil }, - onReset: func(po *TableStorageParamObserver, evalCtx *eval.Context, key string) error { + onReset: func(po *Setter, evalCtx *eval.Context, key string) error { if po.tableDesc.RowLevelTTL != nil { po.tableDesc.RowLevelTTL.DeleteRateLimit = 0 } @@ -387,7 +315,7 @@ var tableParams = map[string]tableParam{ }, }, `ttl_label_metrics`: { - onSet: func(ctx context.Context, po *TableStorageParamObserver, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error { + onSet: func(ctx context.Context, po *Setter, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error { if po.tableDesc.RowLevelTTL == nil { po.tableDesc.RowLevelTTL = &catpb.RowLevelTTL{} } @@ -398,17 +326,17 @@ var tableParams = map[string]tableParam{ po.tableDesc.RowLevelTTL.LabelMetrics = val return nil }, - onReset: func(po *TableStorageParamObserver, evalCtx *eval.Context, key string) error { + onReset: func(po *Setter, evalCtx *eval.Context, key string) error { po.tableDesc.RowLevelTTL.LabelMetrics = false return nil }, }, `ttl_job_cron`: { - onSet: func(ctx context.Context, po *TableStorageParamObserver, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error { + onSet: func(ctx context.Context, po *Setter, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error { if po.tableDesc.RowLevelTTL == nil { po.tableDesc.RowLevelTTL = &catpb.RowLevelTTL{} } - str, err := DatumAsString(evalCtx, key, datum) + str, err := paramparse.DatumAsString(evalCtx, key, datum) if err != nil { return err } @@ -418,7 +346,7 @@ var tableParams = map[string]tableParam{ po.tableDesc.RowLevelTTL.DeletionCron = str return nil }, - onReset: func(po *TableStorageParamObserver, evalCtx *eval.Context, key string) error { + onReset: func(po *Setter, evalCtx *eval.Context, key string) error { if po.tableDesc.RowLevelTTL != nil { po.tableDesc.RowLevelTTL.DeletionCron = "" } @@ -426,7 +354,7 @@ var tableParams = map[string]tableParam{ }, }, `ttl_pause`: { - onSet: func(ctx context.Context, po *TableStorageParamObserver, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error { + onSet: func(ctx context.Context, po *Setter, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error { b, err := boolFromDatum(evalCtx, key, datum) if err != nil { return err @@ -437,14 +365,14 @@ var tableParams = map[string]tableParam{ po.tableDesc.RowLevelTTL.Pause = b return nil }, - onReset: func(po *TableStorageParamObserver, evalCtx *eval.Context, key string) error { + onReset: func(po *Setter, evalCtx *eval.Context, key string) error { po.tableDesc.RowLevelTTL.Pause = false return nil }, }, `ttl_row_stats_poll_interval`: { - onSet: func(ctx context.Context, po *TableStorageParamObserver, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error { - d, err := DatumAsDuration(evalCtx, key, datum) + onSet: func(ctx context.Context, po *Setter, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error { + d, err := paramparse.DatumAsDuration(evalCtx, key, datum) if err != nil { return err } @@ -457,13 +385,13 @@ var tableParams = map[string]tableParam{ po.tableDesc.RowLevelTTL.RowStatsPollInterval = d return nil }, - onReset: func(po *TableStorageParamObserver, evalCtx *eval.Context, key string) error { + onReset: func(po *Setter, evalCtx *eval.Context, key string) error { po.tableDesc.RowLevelTTL.RowStatsPollInterval = 0 return nil }, }, `exclude_data_from_backup`: { - onSet: func(ctx context.Context, po *TableStorageParamObserver, semaCtx *tree.SemaContext, + onSet: func(ctx context.Context, po *Setter, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error { if po.tableDesc.Temporary { return pgerror.Newf(pgcode.FeatureNotSupported, @@ -491,7 +419,7 @@ var tableParams = map[string]tableParam{ po.tableDesc.ExcludeDataFromBackup = excludeDataFromBackup return nil }, - onReset: func(po *TableStorageParamObserver, evalCtx *eval.Context, key string) error { + onReset: func(po *Setter, evalCtx *eval.Context, key string) error { po.tableDesc.ExcludeDataFromBackup = false return nil }, @@ -541,10 +469,10 @@ func init() { `user_catalog_table`, } { tableParams[param] = tableParam{ - onSet: func(ctx context.Context, po *TableStorageParamObserver, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error { + onSet: func(ctx context.Context, po *Setter, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error { return unimplemented.NewWithIssuef(43299, "storage parameter %q", key) }, - onReset: func(po *TableStorageParamObserver, evalCtx *eval.Context, key string) error { + onReset: func(po *Setter, evalCtx *eval.Context, key string) error { return nil }, } @@ -553,7 +481,7 @@ func init() { func autoStatsEnabledSettingFunc( ctx context.Context, - po *TableStorageParamObserver, + po *Setter, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, @@ -572,9 +500,9 @@ func autoStatsEnabledSettingFunc( func autoStatsMinStaleRowsSettingFunc( validateFunc func(v int64) error, -) func(ctx context.Context, po *TableStorageParamObserver, semaCtx *tree.SemaContext, +) func(ctx context.Context, po *Setter, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error { - return func(ctx context.Context, po *TableStorageParamObserver, semaCtx *tree.SemaContext, + return func(ctx context.Context, po *Setter, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error { intVal, err := intFromDatum(evalCtx, key, datum) if err != nil { @@ -593,9 +521,9 @@ func autoStatsMinStaleRowsSettingFunc( func autoStatsFractionStaleRowsSettingFunc( validateFunc func(v float64) error, -) func(ctx context.Context, po *TableStorageParamObserver, semaCtx *tree.SemaContext, +) func(ctx context.Context, po *Setter, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error { - return func(ctx context.Context, po *TableStorageParamObserver, semaCtx *tree.SemaContext, + return func(ctx context.Context, po *Setter, semaCtx *tree.SemaContext, evalCtx *eval.Context, key string, datum tree.Datum) error { floatVal, err := floatFromDatum(evalCtx, key, datum) if err != nil { @@ -612,9 +540,7 @@ func autoStatsFractionStaleRowsSettingFunc( } } -func autoStatsTableSettingResetFunc( - po *TableStorageParamObserver, evalCtx *eval.Context, key string, -) error { +func autoStatsTableSettingResetFunc(po *Setter, evalCtx *eval.Context, key string) error { if po.tableDesc.AutoStatsSettings == nil { return nil } @@ -633,8 +559,8 @@ func autoStatsTableSettingResetFunc( return errors.Newf("unable to reset table setting %s", key) } -// onSet implements the StorageParamObserver interface. -func (po *TableStorageParamObserver) onSet( +// Set implements the Setter interface. +func (po *Setter) Set( ctx context.Context, semaCtx *tree.SemaContext, evalCtx *eval.Context, @@ -653,8 +579,8 @@ func (po *TableStorageParamObserver) onSet( return pgerror.Newf(pgcode.InvalidParameterValue, "invalid storage parameter %q", key) } -// onReset implements the StorageParamObserver interface. -func (po *TableStorageParamObserver) onReset(evalCtx *eval.Context, key string) error { +// Reset implements the Setter interface. +func (po *Setter) Reset(evalCtx *eval.Context, key string) error { if strings.HasPrefix(key, "ttl_") && len(po.tableDesc.AllMutations()) > 0 { return pgerror.Newf( pgcode.FeatureNotSupported, @@ -666,174 +592,3 @@ func (po *TableStorageParamObserver) onReset(evalCtx *eval.Context, key string) } return pgerror.Newf(pgcode.InvalidParameterValue, "invalid storage parameter %q", key) } - -func setFillFactorStorageParam(evalCtx *eval.Context, key string, datum tree.Datum) error { - val, err := DatumAsFloat(evalCtx, key, datum) - if err != nil { - return err - } - if val < 0 || val > 100 { - return pgerror.Newf(pgcode.InvalidParameterValue, "%q must be between 0 and 100", key) - } - if evalCtx != nil { - evalCtx.ClientNoticeSender.BufferClientNotice( - evalCtx.Context, - pgnotice.Newf("storage parameter %q is ignored", key), - ) - } - return nil -} - -// IndexStorageParamObserver observes storage parameters for indexes. -type IndexStorageParamObserver struct { - IndexDesc *descpb.IndexDescriptor -} - -var _ StorageParamObserver = (*IndexStorageParamObserver)(nil) - -func getS2ConfigFromIndex(indexDesc *descpb.IndexDescriptor) *geoindex.S2Config { - var s2Config *geoindex.S2Config - if indexDesc.GeoConfig.S2Geometry != nil { - s2Config = indexDesc.GeoConfig.S2Geometry.S2Config - } - if indexDesc.GeoConfig.S2Geography != nil { - s2Config = indexDesc.GeoConfig.S2Geography.S2Config - } - return s2Config -} - -func (po *IndexStorageParamObserver) applyS2ConfigSetting( - evalCtx *eval.Context, key string, expr tree.Datum, min int64, max int64, -) error { - s2Config := getS2ConfigFromIndex(po.IndexDesc) - if s2Config == nil { - 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) - if err != nil { - return errors.Wrapf(err, "error decoding %q", key) - } - if val < min || val > max { - return pgerror.Newf( - pgcode.InvalidParameterValue, - "%q value must be between %d and %d inclusive", - key, - min, - max, - ) - } - switch key { - case `s2_max_level`: - s2Config.MaxLevel = int32(val) - case `s2_level_mod`: - s2Config.LevelMod = int32(val) - case `s2_max_cells`: - s2Config.MaxCells = int32(val) - } - - return nil -} - -func (po *IndexStorageParamObserver) applyGeometryIndexSetting( - evalCtx *eval.Context, key string, expr tree.Datum, -) error { - if po.IndexDesc.GeoConfig.S2Geometry == nil { - return pgerror.Newf(pgcode.InvalidParameterValue, "%q can only be applied to GEOMETRY spatial indexes", key) - } - val, err := DatumAsFloat(evalCtx, key, expr) - if err != nil { - return errors.Wrapf(err, "error decoding %q", key) - } - switch key { - case `geometry_min_x`: - po.IndexDesc.GeoConfig.S2Geometry.MinX = val - case `geometry_max_x`: - po.IndexDesc.GeoConfig.S2Geometry.MaxX = val - case `geometry_min_y`: - po.IndexDesc.GeoConfig.S2Geometry.MinY = val - case `geometry_max_y`: - po.IndexDesc.GeoConfig.S2Geometry.MaxY = val - default: - return pgerror.Newf(pgcode.InvalidParameterValue, "unknown key: %q", key) - } - return nil -} - -// onSet implements the StorageParamObserver interface. -func (po *IndexStorageParamObserver) onSet( - ctx context.Context, - semaCtx *tree.SemaContext, - evalCtx *eval.Context, - key string, - expr tree.Datum, -) error { - switch key { - case `fillfactor`: - return setFillFactorStorageParam(evalCtx, key, expr) - case `s2_max_level`: - return po.applyS2ConfigSetting(evalCtx, key, expr, 0, 30) - case `s2_level_mod`: - return po.applyS2ConfigSetting(evalCtx, key, expr, 1, 3) - case `s2_max_cells`: - return po.applyS2ConfigSetting(evalCtx, key, expr, 1, 32) - case `geometry_min_x`, `geometry_max_x`, `geometry_min_y`, `geometry_max_y`: - return po.applyGeometryIndexSetting(evalCtx, key, expr) - // `bucket_count` is handled in schema changer when creating hash sharded - // indexes. - case `bucket_count`: - return nil - case `vacuum_cleanup_index_scale_factor`, - `buffering`, - `fastupdate`, - `gin_pending_list_limit`, - `pages_per_range`, - `autosummarize`: - return unimplemented.NewWithIssuef(43299, "storage parameter %q", key) - } - return pgerror.Newf(pgcode.InvalidParameterValue, "invalid storage parameter %q", key) -} - -// onReset implements the StorageParameterObserver interface. -func (po *IndexStorageParamObserver) onReset(evalCtx *eval.Context, key string) error { - return errors.AssertionFailedf("non-implemented codepath") -} - -// runPostChecks implements the StorageParamObserver interface. -func (po *IndexStorageParamObserver) runPostChecks() error { - s2Config := getS2ConfigFromIndex(po.IndexDesc) - if s2Config != nil { - if (s2Config.MaxLevel)%s2Config.LevelMod != 0 { - return pgerror.Newf( - pgcode.InvalidParameterValue, - "s2_max_level (%d) must be divisible by s2_level_mod (%d)", - s2Config.MaxLevel, - s2Config.LevelMod, - ) - } - } - - if cfg := po.IndexDesc.GeoConfig.S2Geometry; cfg != nil { - if cfg.MaxX <= cfg.MinX { - 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 pgerror.Newf( - pgcode.InvalidParameterValue, - "geometry_max_y (%f) must be greater than geometry_min_y (%f)", - cfg.MaxY, - cfg.MinY, - ) - } - } - return nil -}