From 22d968dde8c8242040dc863c896d78f3fe1edb2b Mon Sep 17 00:00:00 2001 From: Pavel Kalinnikov Date: Wed, 25 Jan 2023 10:51:35 +0000 Subject: [PATCH 01/11] kvserver: add TenantsStorageMetrics release test This test verifies that TenantsStorageMetrics is updated correctly when tenant IDs are released. Release note: none Epic: none --- pkg/kv/kvserver/metrics_test.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/pkg/kv/kvserver/metrics_test.go b/pkg/kv/kvserver/metrics_test.go index 0d44c5fa67a3..5f2156e0b71f 100644 --- a/pkg/kv/kvserver/metrics_test.go +++ b/pkg/kv/kvserver/metrics_test.go @@ -21,8 +21,31 @@ import ( "github.com/cockroachdb/cockroach/pkg/storage/enginepb" "github.com/cockroachdb/cockroach/pkg/util/leaktest" "github.com/cockroachdb/cockroach/pkg/util/log" + "github.com/stretchr/testify/require" ) +func TestTenantsStorageMetricsRelease(t *testing.T) { + defer leaktest.AfterTest(t)() + m := newTenantsStorageMetrics() + var refs []*tenantMetricsRef + const tenants = 7 + for i := 0; i < tenants; i++ { + id := roachpb.MustMakeTenantID(roachpb.MinTenantID.InternalValue + uint64(i)) + ref := m.acquireTenant(id) + tm := m.getTenant(context.Background(), ref) + tm.SysBytes.Update(1023) + tm.KeyCount.Inc(123) + refs = append(refs, ref) + } + for i, ref := range refs { + require.Equal(t, int64(1023*(tenants-i)), m.SysBytes.Value(), i) + require.Equal(t, int64(123*(tenants-i)), m.KeyCount.Value(), i) + m.releaseTenant(context.Background(), ref) + } + require.Zero(t, m.SysBytes.Value()) + require.Zero(t, m.KeyCount.Value()) +} + // TestTenantsStorageMetricsConcurrency exercises the concurrency logic of the // TenantsStorageMetrics and ensures that none of the assertions are hit. // The test doesn't meaningfully exercise the logic which is tested elsewhere. From f7170c5db253eb642a69612e3a5724635039abf3 Mon Sep 17 00:00:00 2001 From: Ryan Kuo Date: Thu, 26 Jan 2023 16:15:09 -0500 Subject: [PATCH 02/11] docgen: remove redundant SQL diagrams --- docs/generated/sql/bnf/BUILD.bazel | 4 ---- docs/generated/sql/bnf/alter_database_stmt.bnf | 17 ----------------- docs/generated/sql/bnf/alter_index_stmt.bnf | 8 -------- docs/generated/sql/bnf/alter_range_stmt.bnf | 3 --- docs/generated/sql/bnf/alter_table_stmt.bnf | 10 ---------- pkg/cmd/docgen/diagrams.go | 12 ------------ pkg/gen/bnf.bzl | 4 ---- pkg/gen/docs.bzl | 4 ---- 8 files changed, 62 deletions(-) delete mode 100644 docs/generated/sql/bnf/alter_database_stmt.bnf delete mode 100644 docs/generated/sql/bnf/alter_index_stmt.bnf delete mode 100644 docs/generated/sql/bnf/alter_range_stmt.bnf delete mode 100644 docs/generated/sql/bnf/alter_table_stmt.bnf diff --git a/docs/generated/sql/bnf/BUILD.bazel b/docs/generated/sql/bnf/BUILD.bazel index 82999088d812..4f409b7ff8d1 100644 --- a/docs/generated/sql/bnf/BUILD.bazel +++ b/docs/generated/sql/bnf/BUILD.bazel @@ -21,7 +21,6 @@ FILES = [ "alter_database_primary_region", "alter_database_set_zone_config_extension_stmt", "alter_database_set_stmt", - "alter_database_stmt", "alter_database_survival_goal_stmt", "alter_database_to_schema_stmt", "alter_ddl_stmt", @@ -34,13 +33,11 @@ FILES = [ "alter_func_dep_extension_stmt", "alter_index_partition_by", "alter_index", - "alter_index_stmt", "alter_index_visible_stmt", "alter_partition_stmt", "alter_primary_key", "alter_range_relocate_stmt", "alter_range", - "alter_range_stmt", "alter_rename_view_stmt", "alter_role_stmt", "alter_scatter_index_stmt", @@ -59,7 +56,6 @@ FILES = [ "alter_table_reset_storage_param", "alter_table_set_schema_stmt", "alter_table_set_storage_param", - "alter_table_stmt", "alter_type", "alter_view", "alter_view_owner_stmt", diff --git a/docs/generated/sql/bnf/alter_database_stmt.bnf b/docs/generated/sql/bnf/alter_database_stmt.bnf deleted file mode 100644 index d71b379d8860..000000000000 --- a/docs/generated/sql/bnf/alter_database_stmt.bnf +++ /dev/null @@ -1,17 +0,0 @@ -alter_database_stmt ::= - alter_rename_database_stmt - | alter_zone_database_stmt - | alter_database_owner - | alter_database_to_schema_stmt - | alter_database_add_region_stmt - | alter_database_drop_region_stmt - | alter_database_survival_goal_stmt - | alter_database_primary_region_stmt - | alter_database_placement_stmt - | alter_database_set_stmt - | alter_database_add_super_region - | alter_database_alter_super_region - | alter_database_drop_super_region - | alter_database_set_secondary_region_stmt - | alter_database_drop_secondary_region - | alter_database_set_zone_config_extension_stmt diff --git a/docs/generated/sql/bnf/alter_index_stmt.bnf b/docs/generated/sql/bnf/alter_index_stmt.bnf deleted file mode 100644 index 0c2827a3ac8e..000000000000 --- a/docs/generated/sql/bnf/alter_index_stmt.bnf +++ /dev/null @@ -1,8 +0,0 @@ -alter_index_stmt ::= - alter_oneindex_stmt - | alter_split_index_stmt - | alter_unsplit_index_stmt - | alter_scatter_index_stmt - | alter_rename_index_stmt - | alter_zone_index_stmt - | alter_index_visible_stmt diff --git a/docs/generated/sql/bnf/alter_range_stmt.bnf b/docs/generated/sql/bnf/alter_range_stmt.bnf deleted file mode 100644 index f8a65e7db6c0..000000000000 --- a/docs/generated/sql/bnf/alter_range_stmt.bnf +++ /dev/null @@ -1,3 +0,0 @@ -alter_range_stmt ::= - alter_zone_range_stmt - | alter_range_relocate_stmt diff --git a/docs/generated/sql/bnf/alter_table_stmt.bnf b/docs/generated/sql/bnf/alter_table_stmt.bnf deleted file mode 100644 index 35225ecf5a2d..000000000000 --- a/docs/generated/sql/bnf/alter_table_stmt.bnf +++ /dev/null @@ -1,10 +0,0 @@ -alter_table_stmt ::= - alter_onetable_stmt - | alter_split_stmt - | alter_unsplit_stmt - | alter_scatter_stmt - | alter_zone_table_stmt - | alter_rename_table_stmt - | alter_table_set_schema_stmt - | alter_table_locality_stmt - | alter_table_owner_stmt diff --git a/pkg/cmd/docgen/diagrams.go b/pkg/cmd/docgen/diagrams.go index 759536df3ef0..65e5d2bcd1eb 100644 --- a/pkg/cmd/docgen/diagrams.go +++ b/pkg/cmd/docgen/diagrams.go @@ -406,9 +406,6 @@ var specs = []stmtSpec{ replace: map[string]string{"'RENAME' 'TO' database_name": "'RENAME' 'TO' database_new_name", "'SUPER' 'REGION' name": "'SUPER' 'REGION' region_name", "'VALUES' name_list": "'VALUES' region_name_list", "var_name": "variable", "var_value": "value"}, unlink: []string{"database_new_name", "region_name_list", "variable", "value"}, }, - { - name: "alter_database_stmt", - }, { name: "alter_database_primary_region", stmt: "alter_database_primary_region_stmt", @@ -431,9 +428,6 @@ var specs = []stmtSpec{ replace: map[string]string{"standalone_index_name": "index_name", "var_name": "variable", "var_value": "value", "'RENAME' 'TO' index_name": "'RENAME' 'TO' index_new_name"}, unlink: []string{"index_new_name", "variable", "value"}, }, - { - name: "alter_index_stmt", - }, { name: "alter_range", stmt: "alter_range_stmt", @@ -445,9 +439,6 @@ var specs = []stmtSpec{ replace: map[string]string{"'RANGE' a_expr": "'RANGE' range_id"}, unlink: []string{"range_id"}, }, - { - name: "alter_range_stmt", - }, { name: "alter_table_reset_storage_param", stmt: "alter_onetable_stmt", @@ -557,9 +548,6 @@ var specs = []stmtSpec{ replace: map[string]string{"relation_expr": "table_name", "'RENAME' 'TO' table_name": "'RENAME' 'TO' table_new_name", "var_name": "variable", "var_value": "value"}, unlink: []string{"table_name", "table_new_name", "variable", "value"}, }, - { - name: "alter_table_stmt", - }, { name: "alter_table_cmds", inline: []string{"alter_table_cmd", "column_table_def", "col_qual_list", "opt_column", "opt_validate_behavior", "table_constraint", "alter_column_default", "alter_column_visible", "opt_set_data", "opt_collate", "opt_alter_column_using", "alter_column_on_update", "opt_hash_sharded", "opt_with_storage_parameter_list", "opt_drop_behavior", "audit_mode", "partition_by", "partition_by_table", "partition_by_inner", "storage_parameter_list", "storage_parameter", "storage_parameter_key_list"}, diff --git a/pkg/gen/bnf.bzl b/pkg/gen/bnf.bzl index f54c79118a32..b0da0c037c09 100644 --- a/pkg/gen/bnf.bzl +++ b/pkg/gen/bnf.bzl @@ -21,7 +21,6 @@ BNF_SRCS = [ "//docs/generated/sql/bnf:alter_database_set_secondary_region_stmt.bnf", "//docs/generated/sql/bnf:alter_database_set_stmt.bnf", "//docs/generated/sql/bnf:alter_database_set_zone_config_extension_stmt.bnf", - "//docs/generated/sql/bnf:alter_database_stmt.bnf", "//docs/generated/sql/bnf:alter_database_survival_goal_stmt.bnf", "//docs/generated/sql/bnf:alter_database_to_schema_stmt.bnf", "//docs/generated/sql/bnf:alter_ddl_stmt.bnf", @@ -34,13 +33,11 @@ BNF_SRCS = [ "//docs/generated/sql/bnf:alter_func_stmt.bnf", "//docs/generated/sql/bnf:alter_index.bnf", "//docs/generated/sql/bnf:alter_index_partition_by.bnf", - "//docs/generated/sql/bnf:alter_index_stmt.bnf", "//docs/generated/sql/bnf:alter_index_visible_stmt.bnf", "//docs/generated/sql/bnf:alter_partition_stmt.bnf", "//docs/generated/sql/bnf:alter_primary_key.bnf", "//docs/generated/sql/bnf:alter_range.bnf", "//docs/generated/sql/bnf:alter_range_relocate_stmt.bnf", - "//docs/generated/sql/bnf:alter_range_stmt.bnf", "//docs/generated/sql/bnf:alter_rename_view_stmt.bnf", "//docs/generated/sql/bnf:alter_role_stmt.bnf", "//docs/generated/sql/bnf:alter_scatter_index_stmt.bnf", @@ -59,7 +56,6 @@ BNF_SRCS = [ "//docs/generated/sql/bnf:alter_table_reset_storage_param.bnf", "//docs/generated/sql/bnf:alter_table_set_schema_stmt.bnf", "//docs/generated/sql/bnf:alter_table_set_storage_param.bnf", - "//docs/generated/sql/bnf:alter_table_stmt.bnf", "//docs/generated/sql/bnf:alter_type.bnf", "//docs/generated/sql/bnf:alter_view.bnf", "//docs/generated/sql/bnf:alter_view_owner_stmt.bnf", diff --git a/pkg/gen/docs.bzl b/pkg/gen/docs.bzl index e98b7429fcbe..dd83384a27f0 100644 --- a/pkg/gen/docs.bzl +++ b/pkg/gen/docs.bzl @@ -33,7 +33,6 @@ DOCS_SRCS = [ "//docs/generated/sql/bnf:alter_database_set_secondary_region_stmt.bnf", "//docs/generated/sql/bnf:alter_database_set_stmt.bnf", "//docs/generated/sql/bnf:alter_database_set_zone_config_extension_stmt.bnf", - "//docs/generated/sql/bnf:alter_database_stmt.bnf", "//docs/generated/sql/bnf:alter_database_survival_goal_stmt.bnf", "//docs/generated/sql/bnf:alter_database_to_schema_stmt.bnf", "//docs/generated/sql/bnf:alter_ddl_stmt.bnf", @@ -46,13 +45,11 @@ DOCS_SRCS = [ "//docs/generated/sql/bnf:alter_func_stmt.bnf", "//docs/generated/sql/bnf:alter_index.bnf", "//docs/generated/sql/bnf:alter_index_partition_by.bnf", - "//docs/generated/sql/bnf:alter_index_stmt.bnf", "//docs/generated/sql/bnf:alter_index_visible_stmt.bnf", "//docs/generated/sql/bnf:alter_partition_stmt.bnf", "//docs/generated/sql/bnf:alter_primary_key.bnf", "//docs/generated/sql/bnf:alter_range.bnf", "//docs/generated/sql/bnf:alter_range_relocate_stmt.bnf", - "//docs/generated/sql/bnf:alter_range_stmt.bnf", "//docs/generated/sql/bnf:alter_rename_view_stmt.bnf", "//docs/generated/sql/bnf:alter_role_stmt.bnf", "//docs/generated/sql/bnf:alter_scatter_index_stmt.bnf", @@ -71,7 +68,6 @@ DOCS_SRCS = [ "//docs/generated/sql/bnf:alter_table_reset_storage_param.bnf", "//docs/generated/sql/bnf:alter_table_set_schema_stmt.bnf", "//docs/generated/sql/bnf:alter_table_set_storage_param.bnf", - "//docs/generated/sql/bnf:alter_table_stmt.bnf", "//docs/generated/sql/bnf:alter_type.bnf", "//docs/generated/sql/bnf:alter_view.bnf", "//docs/generated/sql/bnf:alter_view_owner_stmt.bnf", From ff87db04dd4e167b2a8a618dca8a16e5eb173c8f Mon Sep 17 00:00:00 2001 From: Mark Sirek Date: Thu, 26 Jan 2023 19:08:50 -0800 Subject: [PATCH 03/11] optbuilder: fix internal error in LATERAL queries with redundant function calls Fixes #95615 Function `optbuilder.Builder.buildZip` may try to build 2 identical function calls into 2 zip expressions, each one adding one or more columns to the output. However, if buildZip's called to `buildScalar` recognizes a scalar expression has already been built, it skips adding a new output columns and returns the previously-built output column. This mismatch in the number of output columns actually built, and the number of columns communicated to the parent expression via `outScope` confuses the execution engine, and in this case we error out because the expected data type of one column doesn't match the actual data type. The fix is to skip building zip expressions with redundant expressions which already exist in `outScope`. Release note (bug fix): This patch fixes rare internal errors in LATERAL queries with redundant function calls. --- pkg/sql/logictest/testdata/logic_test/srfs | 31 ++++++++++++++ pkg/sql/opt/optbuilder/srfs.go | 29 ++++++++++--- pkg/sql/opt/optbuilder/testdata/srfs | 48 ++++++++++++++++++++++ 3 files changed, 103 insertions(+), 5 deletions(-) diff --git a/pkg/sql/logictest/testdata/logic_test/srfs b/pkg/sql/logictest/testdata/logic_test/srfs index fedcd30fcbeb..487d2439ba33 100644 --- a/pkg/sql/logictest/testdata/logic_test/srfs +++ b/pkg/sql/logictest/testdata/logic_test/srfs @@ -1270,3 +1270,34 @@ SELECT (x).* FROM (SELECT unnest(ARRAY[t58438.*]) FROM t58438) v(x); 1 2 3 4 5 6 + +subtest redundant_zip_expression + +statement ok +SET testing_optimizer_disable_rule_probability = 1.0; + +statement ok +CREATE TABLE t95615 (c1 INET PRIMARY KEY) + +statement ok +INSERT INTO t95615 VALUES ('192.168.0.1'::INET) + +# Test that a redundant function call in a ProjectSet doesn't crash due to +# being built into more than one zip expression. +query TTTI +SELECT * FROM t95615, LATERAL ROWS FROM (hostmask(c1), hostmask(c1)) WITH ORDINALITY +---- +192.168.0.1 0.0.0.0 0.0.0.0 1 + +statement ok +CREATE FUNCTION f95615() RETURNS FLOAT IMMUTABLE LEAKPROOF LANGUAGE SQL AS 'SELECT 1.1' + +# A redundant UDF call should be added only once in a zip expression and not +# error out. +query TFFI +SELECT * FROM t95615, LATERAL ROWS FROM (f95615(), f95615()) WITH ORDINALITY +---- +192.168.0.1 1.1 1.1 1 + +statement ok +RESET testing_optimizer_disable_rule_probability diff --git a/pkg/sql/opt/optbuilder/srfs.go b/pkg/sql/opt/optbuilder/srfs.go index 86b301c0e027..a93cdc802304 100644 --- a/pkg/sql/opt/optbuilder/srfs.go +++ b/pkg/sql/opt/optbuilder/srfs.go @@ -85,8 +85,9 @@ func (b *Builder) buildZip(exprs tree.Exprs, inScope *scope) (outScope *scope) { inScope.context = exprKindFrom // Build each of the provided expressions. - zip := make(memo.ZipExpr, len(exprs)) - for i, expr := range exprs { + zip := make(memo.ZipExpr, 0, len(exprs)) + var outCols opt.ColSet + for _, expr := range exprs { // Output column names should exactly match the original expression, so we // have to determine the output column name before we perform type // checking. However, the alias may be overridden later below if the expression @@ -121,11 +122,29 @@ func (b *Builder) buildZip(exprs tree.Exprs, inScope *scope) (outScope *scope) { } scalar := b.buildScalar(texpr, inScope, outScope, outCol, nil) - cols := make(opt.ColList, len(outScope.cols)-startCols) + numExpectedOutputCols := len(outScope.cols) - startCols + cols := make(opt.ColList, 0, numExpectedOutputCols) for j := startCols; j < len(outScope.cols); j++ { - cols[j-startCols] = outScope.cols[j].id + // If a newly-added outScope column has already been added in a zip + // expression, don't add it again. + if outCols.Contains(outScope.cols[j].id) { + continue + } + cols = append(cols, outScope.cols[j].id) + + // Record that this output column has been added. + outCols.Add(outScope.cols[j].id) + } + // Only add the zip expression if it has output columns which haven't + // already been built. + if len(cols) != 0 { + if len(cols) != numExpectedOutputCols { + // Building more columns than were just added to outScope could cause + // problems for the execution engine. Catch this case. + panic(errors.AssertionFailedf("zip expression builds more columns than added to outScope")) + } + zip = append(zip, b.factory.ConstructZipItem(scalar, cols)) } - zip[i] = b.factory.ConstructZipItem(scalar, cols) } // Construct the zip as a ProjectSet with empty input. diff --git a/pkg/sql/opt/optbuilder/testdata/srfs b/pkg/sql/opt/optbuilder/testdata/srfs index f1c96dfa37f8..d82c7520fc88 100644 --- a/pkg/sql/opt/optbuilder/testdata/srfs +++ b/pkg/sql/opt/optbuilder/testdata/srfs @@ -1089,3 +1089,51 @@ project │ └── filters (true) └── projections └── a:6 || 'foo' [as="?column?":8] + +exec-ddl +CREATE TABLE t95615 (c1 INET PRIMARY KEY); +---- + +# Only a single hostmask function should be added as a zip expression. +build +SELECT * FROM t95615, LATERAL ROWS FROM (hostmask(c1), hostmask(c1)) WITH ORDINALITY; +---- +project + ├── columns: c1:1!null hostmask:4 hostmask:4 ordinality:5!null + └── inner-join-apply + ├── columns: c1:1!null crdb_internal_mvcc_timestamp:2 tableoid:3 hostmask:4 ordinality:5!null + ├── scan t95615 + │ └── columns: c1:1!null crdb_internal_mvcc_timestamp:2 tableoid:3 + ├── ordinality + │ ├── columns: hostmask:4 ordinality:5!null + │ └── project-set + │ ├── columns: hostmask:4 + │ ├── values + │ │ └── () + │ └── zip + │ └── hostmask(c1:1) + └── filters (true) + +exec-ddl +CREATE FUNCTION f95615() RETURNS FLOAT IMMUTABLE LEAKPROOF LANGUAGE SQL AS 'SELECT 1.1' +---- + +# A redundant UDF call should not be added as a zip expression. +build +SELECT * FROM t95615, LATERAL ROWS FROM (f95615(), f95615()) WITH ORDINALITY +---- +project + ├── columns: c1:1!null f95615:6 f95615:6 ordinality:9!null + └── inner-join-apply + ├── columns: c1:1!null crdb_internal_mvcc_timestamp:2 tableoid:3 f95615:6 ordinality:9!null + ├── scan t95615 + │ └── columns: c1:1!null crdb_internal_mvcc_timestamp:2 tableoid:3 + ├── ordinality + │ ├── columns: f95615:6 ordinality:9!null + │ └── project-set + │ ├── columns: f95615:6 + │ ├── values + │ │ └── () + │ └── zip + │ └── f95615() + └── filters (true) From c335a4e0bd9a71e707f4ea39a1986727d8a1a07d Mon Sep 17 00:00:00 2001 From: Mark Sirek Date: Thu, 26 Jan 2023 07:33:21 -0800 Subject: [PATCH 04/11] eval: fix internal error casting bytes to bit Fixes #95543 Release note (bug fix): This patch fixes an internal error which may occur in the SHOW RANGE FROM TABLE statement when the FOR ROW clause specifies a BYTE literal and the corresponding column data type is BIT. --- pkg/sql/logictest/testdata/logic_test/show_ranges | 8 ++++++++ pkg/sql/sem/eval/cast.go | 8 +++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/pkg/sql/logictest/testdata/logic_test/show_ranges b/pkg/sql/logictest/testdata/logic_test/show_ranges index fdd6668f0353..7bf700c3cccf 100644 --- a/pkg/sql/logictest/testdata/logic_test/show_ranges +++ b/pkg/sql/logictest/testdata/logic_test/show_ranges @@ -446,3 +446,11 @@ start_key end_key range_id lease_holder split …/20 59 1 2262-04-11 23:47:16.854776 +0000 +0000 …/20 …/30 60 1 2262-04-11 23:47:16.854776 +0000 +0000 …/30 61 1 2262-04-11 23:47:16.854776 +0000 +0000 + +subtest cast_error + +statement ok +CREATE TABLE v0 (c1 BIT PRIMARY KEY ); + +statement error pgcode 42846 pq: crdb_internal.encode_key\(\): invalid cast: bytes -> bit +SHOW RANGE FROM TABLE v0 FOR ROW ( b'\x68') diff --git a/pkg/sql/sem/eval/cast.go b/pkg/sql/sem/eval/cast.go index aa8c870492d3..e959eb666fd6 100644 --- a/pkg/sql/sem/eval/cast.go +++ b/pkg/sql/sem/eval/cast.go @@ -149,10 +149,12 @@ func performCastWithoutPrecisionTruncation( } ba = &tree.DBitArray{BitArray: res} } - if truncateWidth { - ba = tree.FormatBitArrayToType(ba, t) + if ba != nil { + if truncateWidth { + ba = tree.FormatBitArrayToType(ba, t) + } + return ba, nil } - return ba, nil case types.BoolFamily: switch v := d.(type) { From ecd00d1d78b6da4128753d77585b33a72e2bb9c0 Mon Sep 17 00:00:00 2001 From: Pavel Kalinnikov Date: Fri, 20 Jan 2023 10:23:26 +0000 Subject: [PATCH 05/11] tenantrate,aggmetric: properly handle releasing gauges Unfortunately we have to accept that child metrics continue to see usage (at least for a short period of time) despite already having been "Destroyed". The code assumed that a call to `Destroy()` would prevent all future use of the metric, but this is not the case. To paper over this, `Destroy` (for a gauge) was resetting to zero, but this would mean that some callers who were careful to always decrement the gauge themselves (for example, the quota pool's "pending waiters" metric) would actually end up with a corrupted value of the gauge. In a follow up commit we will stop pretending that `Destroy` destroys the metric. Instead, we will rename to "Unlink" to make clear(er) that this really only unlinks the child from the parent. It would be nice to also make clear in the name that the other direction doesn't hold (updates to child continue to update the parent) but I can't find a better way to do this than to spell it out explicitly in the comment. The semantics here aren't that fortunate, but it's a bigger re-work to make this work better. Epic: none Release note: None --- pkg/kv/kvserver/metrics.go | 43 +++++++----- pkg/kv/kvserver/replica.go | 9 +++ pkg/kv/kvserver/tenantrate/BUILD.bazel | 2 + pkg/kv/kvserver/tenantrate/factory.go | 1 + pkg/kv/kvserver/tenantrate/limiter_test.go | 65 +++++++++++++++++++ pkg/util/metric/aggmetric/gauge.go | 18 +++-- .../aggmetric/testdata/add_after_destroy.txt | 4 +- .../metric/aggmetric/testdata/destroy.txt | 4 +- 8 files changed, 118 insertions(+), 28 deletions(-) diff --git a/pkg/kv/kvserver/metrics.go b/pkg/kv/kvserver/metrics.go index 22070dedafb5..ecb7088b5170 100644 --- a/pkg/kv/kvserver/metrics.go +++ b/pkg/kv/kvserver/metrics.go @@ -2152,24 +2152,31 @@ func (sm *TenantsStorageMetrics) releaseTenant(ctx context.Context, ref *tenantM // The refCount is zero, delete this instance after destroying its metrics. // Note that concurrent attempts to create an instance will detect the zero // refCount value and construct a new instance. - m.LiveBytes.Destroy() - m.KeyBytes.Destroy() - m.ValBytes.Destroy() - m.RangeKeyBytes.Destroy() - m.RangeValBytes.Destroy() - m.TotalBytes.Destroy() - m.IntentBytes.Destroy() - m.LiveCount.Destroy() - m.KeyCount.Destroy() - m.ValCount.Destroy() - m.RangeKeyCount.Destroy() - m.RangeValCount.Destroy() - m.IntentCount.Destroy() - m.IntentAge.Destroy() - m.GcBytesAge.Destroy() - m.SysBytes.Destroy() - m.SysCount.Destroy() - m.AbortSpanBytes.Destroy() + for _, gptr := range []**aggmetric.Gauge{ + &m.LiveBytes, + &m.KeyBytes, + &m.ValBytes, + &m.RangeKeyBytes, + &m.RangeValBytes, + &m.TotalBytes, + &m.IntentBytes, + &m.LiveCount, + &m.KeyCount, + &m.ValCount, + &m.RangeKeyCount, + &m.RangeValCount, + &m.IntentCount, + &m.IntentAge, + &m.GcBytesAge, + &m.SysBytes, + &m.SysCount, + &m.AbortSpanBytes, + } { + // Reset before unlinking, see Destroy. + (*gptr).Update(0) + (*gptr).Destroy() + *gptr = nil + } sm.tenants.Delete(int64(ref._tenantID.ToUint64())) } diff --git a/pkg/kv/kvserver/replica.go b/pkg/kv/kvserver/replica.go index 24c11e1c27f5..93edb180af70 100644 --- a/pkg/kv/kvserver/replica.go +++ b/pkg/kv/kvserver/replica.go @@ -312,6 +312,15 @@ type Replica struct { // tenantLimiter rate limits requests on a per-tenant basis and accumulates // metrics about it. This is determined by the start key of the Replica, // once initialized. + // + // The lifecycle of this is tricky. Because we can't reliably bar requests + // from accessing this even when the replica is destroyed[^1], this will + // stick around on a destroyed replica and can be accessed. The quota pool + // will be closed, however, so it will not accept any writes. + // + // See tenantrate.TestUseAfterRelease. + // + // [^1]: TODO(pavelkalinnikov): we can but it'd be a larger refactor. tenantLimiter tenantrate.Limiter // tenantMetricsRef is a metrics reference indicating the tenant under diff --git a/pkg/kv/kvserver/tenantrate/BUILD.bazel b/pkg/kv/kvserver/tenantrate/BUILD.bazel index 98d5d8c306e7..1a9c6d6d1789 100644 --- a/pkg/kv/kvserver/tenantrate/BUILD.bazel +++ b/pkg/kv/kvserver/tenantrate/BUILD.bazel @@ -44,10 +44,12 @@ go_test( "//pkg/testutils/metrictestutils", "//pkg/util/leaktest", "//pkg/util/metric", + "//pkg/util/stop", "//pkg/util/timeutil", "@com_github_cockroachdb_datadriven//:datadriven", "@com_github_cockroachdb_errors//:errors", "@com_github_dustin_go_humanize//:go-humanize", + "@com_github_stretchr_testify//assert", "@com_github_stretchr_testify//require", "@in_gopkg_yaml_v2//:yaml_v2", ], diff --git a/pkg/kv/kvserver/tenantrate/factory.go b/pkg/kv/kvserver/tenantrate/factory.go index 1ea80098de6b..250c9b6415ca 100644 --- a/pkg/kv/kvserver/tenantrate/factory.go +++ b/pkg/kv/kvserver/tenantrate/factory.go @@ -121,6 +121,7 @@ func (rl *LimiterFactory) Release(lim Limiter) { } if rcLim.refCount--; rcLim.refCount == 0 { l.metrics.destroy() + l.qp.Close("released") delete(rl.mu.tenants, l.tenantID) } } diff --git a/pkg/kv/kvserver/tenantrate/limiter_test.go b/pkg/kv/kvserver/tenantrate/limiter_test.go index 78d12400e714..16107a91f507 100644 --- a/pkg/kv/kvserver/tenantrate/limiter_test.go +++ b/pkg/kv/kvserver/tenantrate/limiter_test.go @@ -13,6 +13,7 @@ package tenantrate_test import ( "context" "fmt" + "math" "regexp" "runtime" "sort" @@ -29,10 +30,12 @@ import ( "github.com/cockroachdb/cockroach/pkg/testutils/metrictestutils" "github.com/cockroachdb/cockroach/pkg/util/leaktest" "github.com/cockroachdb/cockroach/pkg/util/metric" + "github.com/cockroachdb/cockroach/pkg/util/stop" "github.com/cockroachdb/cockroach/pkg/util/timeutil" "github.com/cockroachdb/datadriven" "github.com/cockroachdb/errors" "github.com/dustin/go-humanize" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gopkg.in/yaml.v2" ) @@ -63,6 +66,68 @@ func TestCloser(t *testing.T) { require.Regexp(t, "closer", <-errCh) } +func TestUseAfterRelease(t *testing.T) { + defer leaktest.AfterTest(t)() + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + cs := cluster.MakeTestingClusterSettings() + + factory := tenantrate.NewLimiterFactory(&cs.SV, nil) + s := stop.NewStopper() + defer s.Stop(ctx) + ctx, cancel2 := s.WithCancelOnQuiesce(ctx) + defer cancel2() + + lim := factory.GetTenant(ctx, roachpb.MinTenantID, s.ShouldQuiesce()) + + // Pick a large acquisition size which will cause the quota acquisition to + // block ~forever. We scale it a bit to stay away from overflow. + const n = math.MaxInt64 / 50 + + rq := tenantcostmodel.TestingRequestInfo( + 2 /* writeReplicas */, n /* writeCount */, n /* writeBytes */) + rs := tenantcostmodel.TestingResponseInfo( + true /* isRead */, n /* readCount */, n /* readBytes */) + + // Acquire once to exhaust the burst. The bucket is now deeply in the red. + require.NoError(t, lim.Wait(ctx, rq)) + + waitErr := make(chan error, 1) + _ = s.RunAsyncTask(ctx, "wait", func(ctx context.Context) { + waitErr <- lim.Wait(ctx, rq) + }) + + _ = s.RunAsyncTask(ctx, "release", func(ctx context.Context) { + require.Eventually(t, func() bool { + return factory.Metrics().CurrentBlocked.Value() == 1 + }, 10*time.Second, time.Nanosecond) + factory.Release(lim) + }) + + select { + case <-time.After(10 * time.Second): + t.Errorf("releasing limiter did not unblock acquisition") + case err := <-waitErr: + t.Logf("waiting returned: %s", err) + assert.Error(t, err) + } + + lim.RecordRead(ctx, rs) + + // The read bytes are still recorded to the parent, even though the limiter + // was already released at that point. This isn't required behavior, what's + // more important is that we don't crash. + require.Equal(t, rs.ReadBytes(), factory.Metrics().ReadBytesAdmitted.Count()) + // Write bytes got admitted only once because second attempt got aborted + // during Wait(). + require.Equal(t, rq.WriteBytes(), factory.Metrics().WriteBytesAdmitted.Count()) + // This is a Gauge and we want to make sure that we don't leak an increment to + // it, i.e. the Wait call both added and removed despite interleaving with the + // gauge being unlinked from the aggregating parent. + require.Zero(t, factory.Metrics().CurrentBlocked.Value()) + require.NoError(t, ctx.Err()) // didn't time out +} + func TestDataDriven(t *testing.T) { defer leaktest.AfterTest(t)() datadriven.Walk(t, datapathutils.TestDataPath(t), func(t *testing.T, path string) { diff --git a/pkg/util/metric/aggmetric/gauge.go b/pkg/util/metric/aggmetric/gauge.go index 57572ab00f79..edcb1ed4b6d6 100644 --- a/pkg/util/metric/aggmetric/gauge.go +++ b/pkg/util/metric/aggmetric/gauge.go @@ -107,10 +107,13 @@ func (g *Gauge) ToPrometheusMetric() *io_prometheus_client.Metric { } } -// Destroy is used to destroy the gauge associated with this child. The value of -// the Gauge will be decremented from its parent. +// Destroy is used to destroy the gauge associated with this child. It unlinks the +// Gauge from the parent, meaning it will no longer be reported as contributing to +// it. However, this Gauge will continue to be functional and it will continue to +// update the parent. +// +// See tenantrate.TestUseAfterRelease. func (g *Gauge) Destroy() { - g.Update(0) g.parent.remove(g) } @@ -224,10 +227,13 @@ func (g *GaugeFloat64) ToPrometheusMetric() *io_prometheus_client.Metric { } } -// Destroy is used to destroy the gauge associated with this child. The value of -// the GaugeFloat64 will be decremented from its parent. +// Destroy is used to destroy the gauge associated with this child. It unlinks the +// GaugeFloat64 from the parent, meaning it will no longer be reported as contributing to +// it. However, this Gauge will continue to be functional and it will continue to +// update the parent. +// +// See tenantrate.TestUseAfterRelease. func (g *GaugeFloat64) Destroy() { - g.Update(0) g.parent.remove(g) } diff --git a/pkg/util/metric/aggmetric/testdata/add_after_destroy.txt b/pkg/util/metric/aggmetric/testdata/add_after_destroy.txt index bd8f19486ab3..45b67cb0a546 100644 --- a/pkg/util/metric/aggmetric/testdata/add_after_destroy.txt +++ b/pkg/util/metric/aggmetric/testdata/add_after_destroy.txt @@ -1,9 +1,9 @@ echo ---- -bar_gauge 2 +bar_gauge 4 bar_gauge{tenant_id="2"} 2 bar_gauge{tenant_id="3"} 0 -baz_gauge 1.5 +baz_gauge 4 baz_gauge{tenant_id="2"} 1.5 baz_gauge{tenant_id="3"} 0 foo_counter 6 diff --git a/pkg/util/metric/aggmetric/testdata/destroy.txt b/pkg/util/metric/aggmetric/testdata/destroy.txt index 5096f911e6a5..c7ad402b85a4 100644 --- a/pkg/util/metric/aggmetric/testdata/destroy.txt +++ b/pkg/util/metric/aggmetric/testdata/destroy.txt @@ -1,8 +1,8 @@ echo ---- -bar_gauge 2 +bar_gauge 4 bar_gauge{tenant_id="2"} 2 -baz_gauge 1.5 +baz_gauge 4 baz_gauge{tenant_id="2"} 1.5 foo_counter 6 foo_counter{tenant_id="3"} 4 From 9eee6acb04d61ab48b04fb860988d1d02f631a28 Mon Sep 17 00:00:00 2001 From: Chengxiong Ruan Date: Wed, 11 Jan 2023 11:35:50 -0500 Subject: [PATCH 06/11] sql/schemachanger: decomp function elements This commit adds function element definitions and decomposition logics. Release note: None --- pkg/sql/catalog/catpb/function.proto | 22 ++ pkg/sql/schemachanger/scdecomp/decomp.go | 124 +++++++++- .../schemachanger/scdecomp/testdata/function | 143 ++++++++++++ pkg/sql/schemachanger/scpb/elements.proto | 78 ++++++- .../schemachanger/scpb/elements_generated.go | 217 ++++++++++++++++++ pkg/sql/schemachanger/scpb/uml/table.puml | 50 ++++ 6 files changed, 622 insertions(+), 12 deletions(-) create mode 100644 pkg/sql/schemachanger/scdecomp/testdata/function diff --git a/pkg/sql/catalog/catpb/function.proto b/pkg/sql/catalog/catpb/function.proto index 5155f0247ccc..6518d1bdfc05 100644 --- a/pkg/sql/catalog/catpb/function.proto +++ b/pkg/sql/catalog/catpb/function.proto @@ -46,3 +46,25 @@ message Function { } } } + +// These wrappers are for the convenience of referencing the enum types from a +// proto3 definition. +message FunctionVolatility { + option (gogoproto.equal) = true; + optional Function.Volatility volatility = 1 [(gogoproto.nullable) = false]; +} + +message FunctionNullInputBehavior { + option (gogoproto.equal) = true; + optional Function.NullInputBehavior nullInputBehavior = 1 [(gogoproto.nullable) = false]; +} + +message FunctionLanguage { + option (gogoproto.equal) = true; + optional Function.Language lang = 1 [(gogoproto.nullable) = false]; +} + +message FunctionParamClass { + option (gogoproto.equal) = true; + optional Function.Param.Class class = 1 [(gogoproto.nullable) = false]; +} diff --git a/pkg/sql/schemachanger/scdecomp/decomp.go b/pkg/sql/schemachanger/scdecomp/decomp.go index 1e5d7e0eb1d6..c19c68b92a58 100644 --- a/pkg/sql/schemachanger/scdecomp/decomp.go +++ b/pkg/sql/schemachanger/scdecomp/decomp.go @@ -18,7 +18,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/catalog/catenumpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" - "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scerrors" "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb" "github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants" "github.com/cockroachdb/cockroach/pkg/sql/sem/catid" @@ -81,12 +80,14 @@ func WalkDescriptor( func (w *walkCtx) walkRoot() { // Common elements. - w.ev(scpb.Status_PUBLIC, &scpb.Namespace{ - DatabaseID: w.desc.GetParentID(), - SchemaID: w.desc.GetParentSchemaID(), - DescriptorID: w.desc.GetID(), - Name: w.desc.GetName(), - }) + if !w.desc.SkipNamespace() { + w.ev(scpb.Status_PUBLIC, &scpb.Namespace{ + DatabaseID: w.desc.GetParentID(), + SchemaID: w.desc.GetParentSchemaID(), + DescriptorID: w.desc.GetID(), + Name: w.desc.GetName(), + }) + } privileges := w.desc.GetPrivileges() w.ev(scpb.Status_PUBLIC, &scpb.Owner{ DescriptorID: w.desc.GetID(), @@ -110,10 +111,7 @@ func (w *walkCtx) walkRoot() { case catalog.TableDescriptor: w.walkRelation(d) case catalog.FunctionDescriptor: - // TODO (Chengxiong) #83235 implement DROP FUNCTION. - // Fall back to legacy schema changer if there is any function descriptor in - // the drop cascade dependency graph. - panic(scerrors.NotImplementedErrorf(nil, "function descriptor not supported in declarative schema changer")) + w.walkFunction(d) default: panic(errors.AssertionFailedf("unexpected descriptor type %T: %+v", w.desc, w.desc)) @@ -667,3 +665,107 @@ func (w *walkCtx) walkForeignKeyConstraint( }) } } + +func (w *walkCtx) walkFunction(fnDesc catalog.FunctionDescriptor) { + typeT := newTypeT(fnDesc.GetReturnType().Type) + fn := &scpb.Function{ + FunctionID: fnDesc.GetID(), + ReturnSet: fnDesc.GetReturnType().ReturnSet, + ReturnType: *typeT, + Params: make([]scpb.Function_Parameter, len(fnDesc.GetParams())), + } + for i, param := range fnDesc.GetParams() { + typeT := newTypeT(param.Type) + fn.Params[i] = scpb.Function_Parameter{ + Name: param.Name, + Class: catpb.FunctionParamClass{Class: param.Class}, + Type: *typeT, + } + if param.DefaultExpr != nil { + expr, err := w.newExpression(*param.DefaultExpr) + if err != nil { + panic(err) + } + w.ev(scpb.Status_PUBLIC, &scpb.FunctionParamDefaultExpression{ + FunctionID: fnDesc.GetID(), + Ordinal: uint32(i), + Expression: *expr, + }) + } + } + + w.ev(descriptorStatus(fnDesc), fn) + w.ev(scpb.Status_PUBLIC, &scpb.ObjectParent{ + ObjectID: fnDesc.GetID(), + ParentSchemaID: fnDesc.GetParentSchemaID(), + }) + w.ev(scpb.Status_PUBLIC, &scpb.FunctionName{ + FunctionID: fnDesc.GetID(), + Name: fnDesc.GetName(), + }) + w.ev(scpb.Status_PUBLIC, &scpb.FunctionVolatility{ + FunctionID: fnDesc.GetID(), + Volatility: catpb.FunctionVolatility{Volatility: fnDesc.GetVolatility()}, + }) + w.ev(scpb.Status_PUBLIC, &scpb.FunctionLeakProof{ + FunctionID: fnDesc.GetID(), + LeakProof: fnDesc.GetLeakProof(), + }) + w.ev(scpb.Status_PUBLIC, &scpb.FunctionNullInputBehavior{ + FunctionID: fnDesc.GetID(), + NullInputBehavior: catpb.FunctionNullInputBehavior{NullInputBehavior: fnDesc.GetNullInputBehavior()}, + }) + + fnBody := &scpb.FunctionBody{ + FunctionID: fnDesc.GetID(), + Body: fnDesc.GetFunctionBody(), + Lang: catpb.FunctionLanguage{Lang: fnDesc.GetLanguage()}, + UsesTypeIDs: fnDesc.GetDependsOnTypes(), + // TODO(chengxiong): add UsesFunctionIDs + } + dedupeColIDs := func(colIDs []catid.ColumnID) []catid.ColumnID { + ret := catalog.MakeTableColSet() + for _, id := range colIDs { + ret.Add(id) + } + return ret.Ordered() + } + for _, toID := range fnDesc.GetDependsOn() { + to := w.lookupFn(toID) + toDesc, err := catalog.AsTableDescriptor(to) + if err != nil { + panic(err) + } + if toDesc.IsSequence() { + fnBody.UsesSequenceIDs = append(fnBody.UsesSequenceIDs, toDesc.GetID()) + } else if toDesc.IsView() { + if err := toDesc.ForeachDependedOnBy(func(dep *descpb.TableDescriptor_Reference) error { + if dep.ID != fnDesc.GetID() { + return nil + } + fnBody.UsesViews = append(fnBody.UsesViews, scpb.FunctionBody_ViewReference{ + ViewID: toDesc.GetID(), + ColumnIDs: dedupeColIDs(dep.ColumnIDs), + }) + return nil + }); err != nil { + panic(err) + } + } else { + if err := toDesc.ForeachDependedOnBy(func(dep *descpb.TableDescriptor_Reference) error { + if dep.ID != fnDesc.GetID() { + return nil + } + fnBody.UsesTables = append(fnBody.UsesTables, scpb.FunctionBody_TableReference{ + TableID: toDesc.GetID(), + IndexID: dep.IndexID, + ColumnIDs: dedupeColIDs(dep.ColumnIDs), + }) + return nil + }); err != nil { + panic(err) + } + } + } + w.ev(scpb.Status_PUBLIC, fnBody) +} diff --git a/pkg/sql/schemachanger/scdecomp/testdata/function b/pkg/sql/schemachanger/scdecomp/testdata/function new file mode 100644 index 000000000000..b83bb4d9d958 --- /dev/null +++ b/pkg/sql/schemachanger/scdecomp/testdata/function @@ -0,0 +1,143 @@ +setup +CREATE TABLE t( + a INT PRIMARY KEY, + b INT, + C INT, + INDEX t_idx_b(b), + INDEX t_idx_c(c) +); +CREATE SEQUENCE sq1; +CREATE TABLE t2(a INT PRIMARY KEY); +CREATE VIEW v AS SELECT a FROM t2; +CREATE TYPE notmyworkday AS ENUM ('Monday', 'Tuesday'); +CREATE FUNCTION f(a notmyworkday) RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ + SELECT a FROM t; + SELECT b FROM t@t_idx_b; + SELECT c FROM t@t_idx_c; + SELECT a FROM v; + SELECT nextval('sq1'); +$$; +---- + +decompose +f +---- +BackReferencedIDs: +ElementState: +- Function: + functionId: 110 + params: + - class: + class: IN + name: a + type: + closedTypeIds: + - 108 + - 109 + type: + arrayContents: null + arrayDimensions: [] + arrayElemType: null + family: EnumFamily + geoMetadata: null + intervalDurationField: null + locale: null + oid: 100108 + precision: 0 + timePrecisionIsSet: false + tupleContents: [] + tupleLabels: [] + udtMetadata: + arrayTypeOid: 100109 + visibleType: 0 + width: 0 + returnSet: false + returnType: + closedTypeIds: [] + type: + arrayContents: null + arrayDimensions: [] + arrayElemType: null + family: IntFamily + geoMetadata: null + intervalDurationField: null + locale: null + oid: 20 + precision: 0 + timePrecisionIsSet: false + tupleContents: [] + tupleLabels: [] + udtMetadata: null + visibleType: 0 + width: 64 + Status: PUBLIC +- Owner: + descriptorId: 110 + owner: root + Status: PUBLIC +- UserPrivileges: + descriptorId: 110 + privileges: 2 + userName: admin + Status: PUBLIC +- UserPrivileges: + descriptorId: 110 + privileges: 2 + userName: root + Status: PUBLIC +- ObjectParent: + objectId: 110 + parentSchemaId: 101 + Status: PUBLIC +- FunctionName: + functionId: 110 + name: f + Status: PUBLIC +- FunctionVolatility: + functionId: 110 + volatility: + volatility: IMMUTABLE + Status: PUBLIC +- FunctionLeakProof: + functionId: 110 + leakProof: false + Status: PUBLIC +- FunctionNullInputBehavior: + functionId: 110 + nullInputBehavior: + nullInputBehavior: CALLED_ON_NULL_INPUT + Status: PUBLIC +- FunctionBody: + body: |- + SELECT a FROM defaultdb.public.t; + SELECT b FROM defaultdb.public.t@t_idx_b; + SELECT c FROM defaultdb.public.t@t_idx_c; + SELECT a FROM defaultdb.public.v; + SELECT nextval(105:::REGCLASS); + functionId: 110 + lang: + lang: SQL + usesFunctionIds: [] + usesSequenceIds: + - 105 + usesTables: + - columnIds: + - 1 + indexId: 0 + tableId: 104 + - columnIds: + - 2 + indexId: 2 + tableId: 104 + - columnIds: + - 3 + indexId: 3 + tableId: 104 + usesTypeIds: + - 108 + - 109 + usesViews: + - columnIds: + - 1 + viewId: 107 + Status: PUBLIC diff --git a/pkg/sql/schemachanger/scpb/elements.proto b/pkg/sql/schemachanger/scpb/elements.proto index 9c01b205aac5..122cfcce5711 100644 --- a/pkg/sql/schemachanger/scpb/elements.proto +++ b/pkg/sql/schemachanger/scpb/elements.proto @@ -15,6 +15,7 @@ option go_package = "scpb"; import "sql/catalog/catenumpb/index.proto"; import "sql/catalog/catpb/catalog.proto"; import "sql/sem/semenumpb/constraint.proto"; +import "sql/catalog/catpb/function.proto"; import "sql/types/types.proto"; import "gogoproto/gogo.proto"; import "geo/geoindex/config.proto"; @@ -61,6 +62,7 @@ message ElementProto { EnumType enum_type = 6; AliasType alias_type = 7; CompositeType composite_type = 8; + Function function = 9; // Relation elements. ColumnFamily column_family = 20 [(gogoproto.moretags) = "parent:\"Table\""]; @@ -128,7 +130,15 @@ message ElementProto { CompositeTypeAttrType composite_type_attr_type = 140 [(gogoproto.moretags) = "parent:\"CompositeType\""]; CompositeTypeAttrName composite_type_attr_name = 141 [(gogoproto.moretags) = "parent:\"CompositeType\""]; - // Next element group start id: 160 + // Function elements. + FunctionName function_name = 160 [(gogoproto.moretags) = "parent:\"Function\""]; + FunctionVolatility function_volatility = 161 [(gogoproto.moretags) = "parent:\"Function\""]; + FunctionLeakProof function_leak_proof = 162 [(gogoproto.moretags) = "parent:\"Function\""]; + FunctionNullInputBehavior function_null_input_behavior = 163 [(gogoproto.moretags) = "parent:\"Function\""]; + FunctionBody function_body = 164 [(gogoproto.moretags) = "parent:\"Function\""]; + FunctionParamDefaultExpression function_param_default = 165 [(gogoproto.moretags) = "parent:\"Function\""]; + + // Next element group start id: 170 } // TypeT is a wrapper for a types.T which contains its user-defined type ID @@ -567,3 +577,69 @@ message IndexData { message TablePartitioning { uint32 table_id = 1 [(gogoproto.customname) = "TableID", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.DescID"]; } + +message Function { + message Parameter { + string name = 1; + cockroach.sql.catalog.catpb.FunctionParamClass class = 2 [(gogoproto.nullable) = false]; + TypeT type = 3 [(gogoproto.nullable) = false]; + } + + uint32 function_id = 1 [(gogoproto.customname) = "FunctionID", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.DescID"]; + repeated Parameter params = 2 [(gogoproto.nullable) = false]; + + bool return_set = 3; + TypeT return_type = 4 [(gogoproto.nullable) = false]; +} + +message FunctionName { + uint32 function_id = 1 [(gogoproto.customname) = "FunctionID", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.DescID"]; + string name = 2; +} + +message FunctionVolatility { + uint32 function_id = 1 [(gogoproto.customname) = "FunctionID", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.DescID"]; + cockroach.sql.catalog.catpb.FunctionVolatility volatility = 2 [(gogoproto.nullable) = false]; +} + +message FunctionLeakProof { + uint32 function_id = 1 [(gogoproto.customname) = "FunctionID", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.DescID"]; + bool leak_proof = 2; +} + +message FunctionNullInputBehavior { + uint32 function_id = 1 [(gogoproto.customname) = "FunctionID", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.DescID"]; + cockroach.sql.catalog.catpb.FunctionNullInputBehavior null_input_behavior = 11 [(gogoproto.nullable) = false]; +} + +message FunctionBody { + message TableReference { + uint32 table_id = 1 [(gogoproto.customname) = "TableID", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.DescID"]; + repeated uint32 column_ids = 2 [(gogoproto.customname) = "ColumnIDs", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.ColumnID"]; + uint32 index_id = 3 [(gogoproto.customname) = "IndexID", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.IndexID"]; + } + + message ViewReference { + uint32 view_id = 1 [(gogoproto.customname) = "ViewID", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.DescID"]; + repeated uint32 column_ids = 2 [(gogoproto.customname) = "ColumnIDs", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.ColumnID"]; + } + + uint32 function_id = 1 [(gogoproto.customname) = "FunctionID", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.DescID"]; + string body = 2; + cockroach.sql.catalog.catpb.FunctionLanguage lang = 3 [(gogoproto.nullable) = false]; + + repeated TableReference uses_tables = 4 [(gogoproto.nullable) = false]; + repeated ViewReference uses_views = 5 [(gogoproto.nullable) = false]; + repeated uint32 uses_sequence_ids = 6 [(gogoproto.customname) = "UsesSequenceIDs", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.DescID"]; + repeated uint32 uses_type_ids = 7 [(gogoproto.customname) = "UsesTypeIDs", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.DescID"]; + repeated uint32 uses_function_ids = 8 [(gogoproto.customname) = "UsesFunctionIDs", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.DescID"]; +} + +message FunctionParamDefaultExpression { + uint32 function_id = 1 [(gogoproto.customname) = "FunctionID", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.DescID"]; + // This is the 0-indexed ordinal of the function parameter in function + // signatures. With this we'd be able to tell which parameter this expression + // belongs to. + uint32 ordinal = 2; + Expression embedded_expr = 3 [(gogoproto.nullable) = false, (gogoproto.embed) = true]; +} diff --git a/pkg/sql/schemachanger/scpb/elements_generated.go b/pkg/sql/schemachanger/scpb/elements_generated.go index 37f5f51ba000..d2690e82d477 100644 --- a/pkg/sql/schemachanger/scpb/elements_generated.go +++ b/pkg/sql/schemachanger/scpb/elements_generated.go @@ -699,6 +699,223 @@ func FindForeignKeyConstraint(b ElementStatusIterator) (current Status, target T return current, target, element } +func (e Function) element() {} + +// ForEachFunction iterates over elements of type Function. +func ForEachFunction( + b ElementStatusIterator, fn func(current Status, target TargetStatus, e *Function), +) { + if b == nil { + return + } + b.ForEachElementStatus(func(current Status, target TargetStatus, e Element) { + if elt, ok := e.(*Function); ok { + fn(current, target, elt) + } + }) +} + +// FindFunction finds the first element of type Function. +func FindFunction(b ElementStatusIterator) (current Status, target TargetStatus, element *Function) { + if b == nil { + return current, target, element + } + b.ForEachElementStatus(func(c Status, t TargetStatus, e Element) { + if elt, ok := e.(*Function); ok { + element = elt + current = c + target = t + } + }) + return current, target, element +} + +func (e FunctionBody) element() {} + +// ForEachFunctionBody iterates over elements of type FunctionBody. +func ForEachFunctionBody( + b ElementStatusIterator, fn func(current Status, target TargetStatus, e *FunctionBody), +) { + if b == nil { + return + } + b.ForEachElementStatus(func(current Status, target TargetStatus, e Element) { + if elt, ok := e.(*FunctionBody); ok { + fn(current, target, elt) + } + }) +} + +// FindFunctionBody finds the first element of type FunctionBody. +func FindFunctionBody(b ElementStatusIterator) (current Status, target TargetStatus, element *FunctionBody) { + if b == nil { + return current, target, element + } + b.ForEachElementStatus(func(c Status, t TargetStatus, e Element) { + if elt, ok := e.(*FunctionBody); ok { + element = elt + current = c + target = t + } + }) + return current, target, element +} + +func (e FunctionLeakProof) element() {} + +// ForEachFunctionLeakProof iterates over elements of type FunctionLeakProof. +func ForEachFunctionLeakProof( + b ElementStatusIterator, fn func(current Status, target TargetStatus, e *FunctionLeakProof), +) { + if b == nil { + return + } + b.ForEachElementStatus(func(current Status, target TargetStatus, e Element) { + if elt, ok := e.(*FunctionLeakProof); ok { + fn(current, target, elt) + } + }) +} + +// FindFunctionLeakProof finds the first element of type FunctionLeakProof. +func FindFunctionLeakProof(b ElementStatusIterator) (current Status, target TargetStatus, element *FunctionLeakProof) { + if b == nil { + return current, target, element + } + b.ForEachElementStatus(func(c Status, t TargetStatus, e Element) { + if elt, ok := e.(*FunctionLeakProof); ok { + element = elt + current = c + target = t + } + }) + return current, target, element +} + +func (e FunctionName) element() {} + +// ForEachFunctionName iterates over elements of type FunctionName. +func ForEachFunctionName( + b ElementStatusIterator, fn func(current Status, target TargetStatus, e *FunctionName), +) { + if b == nil { + return + } + b.ForEachElementStatus(func(current Status, target TargetStatus, e Element) { + if elt, ok := e.(*FunctionName); ok { + fn(current, target, elt) + } + }) +} + +// FindFunctionName finds the first element of type FunctionName. +func FindFunctionName(b ElementStatusIterator) (current Status, target TargetStatus, element *FunctionName) { + if b == nil { + return current, target, element + } + b.ForEachElementStatus(func(c Status, t TargetStatus, e Element) { + if elt, ok := e.(*FunctionName); ok { + element = elt + current = c + target = t + } + }) + return current, target, element +} + +func (e FunctionNullInputBehavior) element() {} + +// ForEachFunctionNullInputBehavior iterates over elements of type FunctionNullInputBehavior. +func ForEachFunctionNullInputBehavior( + b ElementStatusIterator, fn func(current Status, target TargetStatus, e *FunctionNullInputBehavior), +) { + if b == nil { + return + } + b.ForEachElementStatus(func(current Status, target TargetStatus, e Element) { + if elt, ok := e.(*FunctionNullInputBehavior); ok { + fn(current, target, elt) + } + }) +} + +// FindFunctionNullInputBehavior finds the first element of type FunctionNullInputBehavior. +func FindFunctionNullInputBehavior(b ElementStatusIterator) (current Status, target TargetStatus, element *FunctionNullInputBehavior) { + if b == nil { + return current, target, element + } + b.ForEachElementStatus(func(c Status, t TargetStatus, e Element) { + if elt, ok := e.(*FunctionNullInputBehavior); ok { + element = elt + current = c + target = t + } + }) + return current, target, element +} + +func (e FunctionParamDefaultExpression) element() {} + +// ForEachFunctionParamDefaultExpression iterates over elements of type FunctionParamDefaultExpression. +func ForEachFunctionParamDefaultExpression( + b ElementStatusIterator, fn func(current Status, target TargetStatus, e *FunctionParamDefaultExpression), +) { + if b == nil { + return + } + b.ForEachElementStatus(func(current Status, target TargetStatus, e Element) { + if elt, ok := e.(*FunctionParamDefaultExpression); ok { + fn(current, target, elt) + } + }) +} + +// FindFunctionParamDefaultExpression finds the first element of type FunctionParamDefaultExpression. +func FindFunctionParamDefaultExpression(b ElementStatusIterator) (current Status, target TargetStatus, element *FunctionParamDefaultExpression) { + if b == nil { + return current, target, element + } + b.ForEachElementStatus(func(c Status, t TargetStatus, e Element) { + if elt, ok := e.(*FunctionParamDefaultExpression); ok { + element = elt + current = c + target = t + } + }) + return current, target, element +} + +func (e FunctionVolatility) element() {} + +// ForEachFunctionVolatility iterates over elements of type FunctionVolatility. +func ForEachFunctionVolatility( + b ElementStatusIterator, fn func(current Status, target TargetStatus, e *FunctionVolatility), +) { + if b == nil { + return + } + b.ForEachElementStatus(func(current Status, target TargetStatus, e Element) { + if elt, ok := e.(*FunctionVolatility); ok { + fn(current, target, elt) + } + }) +} + +// FindFunctionVolatility finds the first element of type FunctionVolatility. +func FindFunctionVolatility(b ElementStatusIterator) (current Status, target TargetStatus, element *FunctionVolatility) { + if b == nil { + return current, target, element + } + b.ForEachElementStatus(func(c Status, t TargetStatus, e Element) { + if elt, ok := e.(*FunctionVolatility); ok { + element = elt + current = c + target = t + } + }) + return current, target, element +} + func (e IndexColumn) element() {} // ForEachIndexColumn iterates over elements of type IndexColumn. diff --git a/pkg/sql/schemachanger/scpb/uml/table.puml b/pkg/sql/schemachanger/scpb/uml/table.puml index 1963a5aee765..9c41e5ca8cdf 100644 --- a/pkg/sql/schemachanger/scpb/uml/table.puml +++ b/pkg/sql/schemachanger/scpb/uml/table.puml @@ -45,6 +45,13 @@ object CompositeType CompositeType : TypeID CompositeType : ArrayTypeID +object Function + +Function : FunctionID +Function : []Params +Function : ReturnSet +Function : ReturnType + object ColumnFamily ColumnFamily : TableID @@ -307,6 +314,43 @@ object CompositeTypeAttrName CompositeTypeAttrName : CompositeTypeID CompositeTypeAttrName : Name +object FunctionName + +FunctionName : FunctionID +FunctionName : Name + +object FunctionVolatility + +FunctionVolatility : FunctionID +FunctionVolatility : Volatility + +object FunctionLeakProof + +FunctionLeakProof : FunctionID +FunctionLeakProof : LeakProof + +object FunctionNullInputBehavior + +FunctionNullInputBehavior : FunctionID +FunctionNullInputBehavior : NullInputBehavior + +object FunctionBody + +FunctionBody : FunctionID +FunctionBody : Body +FunctionBody : Lang +FunctionBody : []UsesTables +FunctionBody : []UsesViews +FunctionBody : []UsesSequenceIDs +FunctionBody : []UsesTypeIDs +FunctionBody : []UsesFunctionIDs + +object FunctionParamDefaultExpression + +FunctionParamDefaultExpression : FunctionID +FunctionParamDefaultExpression : Ordinal +FunctionParamDefaultExpression : Expression + Table <|-- ColumnFamily Table <|-- Column View <|-- Column @@ -396,4 +440,10 @@ Sequence <|-- ObjectParent EnumType <|-- EnumTypeValue CompositeType <|-- CompositeTypeAttrType CompositeType <|-- CompositeTypeAttrName +Function <|-- FunctionName +Function <|-- FunctionVolatility +Function <|-- FunctionLeakProof +Function <|-- FunctionNullInputBehavior +Function <|-- FunctionBody +Function <|-- FunctionParamDefaultExpression @enduml From 468b9073b4783c40cdeaea53f981841e85c305db Mon Sep 17 00:00:00 2001 From: Chengxiong Ruan Date: Wed, 11 Jan 2023 20:47:07 -0500 Subject: [PATCH 07/11] sql/schemachanger: rename RemoveViewBackReferencesInRelations This should work for both view and function, so renaming it to RemoveBackReferencesInRelations. Release note: None --- .../scexec/scmutationexec/references.go | 10 ++++---- .../schemachanger/scop/immediate_mutation.go | 8 +++---- .../immediate_mutation_visitor_generated.go | 6 ++--- .../scplan/internal/opgen/opgen_view.go | 8 +++---- .../scplan/testdata/alter_table_drop_column | 8 +++---- .../scplan/testdata/drop_database | 20 ++++++++-------- .../schemachanger/scplan/testdata/drop_index | 8 +++---- .../scplan/testdata/drop_owned_by | 8 +++---- .../schemachanger/scplan/testdata/drop_schema | 20 ++++++++-------- .../schemachanger/scplan/testdata/drop_table | 4 ++-- .../schemachanger/scplan/testdata/drop_view | 24 +++++++++---------- .../drop_index_with_materialized_view_dep | 2 +- .../drop_index_with_materialized_view_dep | 4 ++-- 13 files changed, 65 insertions(+), 65 deletions(-) diff --git a/pkg/sql/schemachanger/scexec/scmutationexec/references.go b/pkg/sql/schemachanger/scexec/scmutationexec/references.go index 9aececa7ffb1..20ef3048a585 100644 --- a/pkg/sql/schemachanger/scexec/scmutationexec/references.go +++ b/pkg/sql/schemachanger/scexec/scmutationexec/references.go @@ -294,11 +294,11 @@ func updateBackReferencesInSequences( return nil } -func (i *immediateVisitor) RemoveViewBackReferencesInRelations( - ctx context.Context, op scop.RemoveViewBackReferencesInRelations, +func (m *immediateVisitor) RemoveBackReferencesInRelations( + ctx context.Context, op scop.RemoveBackReferencesInRelations, ) error { for _, relationID := range op.RelationIDs { - if err := removeViewBackReferencesInRelation(ctx, i, relationID, op.BackReferencedViewID); err != nil { + if err := removeViewBackReferencesInRelation(ctx, m, relationID, op.BackReferencedID); err != nil { return err } } @@ -306,7 +306,7 @@ func (i *immediateVisitor) RemoveViewBackReferencesInRelations( } func removeViewBackReferencesInRelation( - ctx context.Context, m *immediateVisitor, relationID, viewID descpb.ID, + ctx context.Context, m *immediateVisitor, relationID, backReferencedID descpb.ID, ) error { tbl, err := m.checkOutTable(ctx, relationID) if err != nil || tbl.Dropped() { @@ -315,7 +315,7 @@ func removeViewBackReferencesInRelation( } var newBackRefs []descpb.TableDescriptor_Reference for _, by := range tbl.DependedOnBy { - if by.ID != viewID { + if by.ID != backReferencedID { newBackRefs = append(newBackRefs, by) } } diff --git a/pkg/sql/schemachanger/scop/immediate_mutation.go b/pkg/sql/schemachanger/scop/immediate_mutation.go index a37a7542b776..5106e9d01156 100644 --- a/pkg/sql/schemachanger/scop/immediate_mutation.go +++ b/pkg/sql/schemachanger/scop/immediate_mutation.go @@ -447,12 +447,12 @@ type UpdateBackReferencesInSequences struct { SequenceIDs []descpb.ID } -// RemoveViewBackReferencesInRelations removes back references to a view in +// RemoveBackReferencesInRelations removes back references to a view in // the specified tables, views or sequences. -type RemoveViewBackReferencesInRelations struct { +type RemoveBackReferencesInRelations struct { immediateMutationOp - BackReferencedViewID descpb.ID - RelationIDs []descpb.ID + BackReferencedID descpb.ID + RelationIDs []descpb.ID } // SetColumnName renames a column. diff --git a/pkg/sql/schemachanger/scop/immediate_mutation_visitor_generated.go b/pkg/sql/schemachanger/scop/immediate_mutation_visitor_generated.go index 675c2e017475..9a373efd7d63 100644 --- a/pkg/sql/schemachanger/scop/immediate_mutation_visitor_generated.go +++ b/pkg/sql/schemachanger/scop/immediate_mutation_visitor_generated.go @@ -75,7 +75,7 @@ type ImmediateMutationVisitor interface { UpdateTypeBackReferencesInTypes(context.Context, UpdateTypeBackReferencesInTypes) error RemoveBackReferenceInTypes(context.Context, RemoveBackReferenceInTypes) error UpdateBackReferencesInSequences(context.Context, UpdateBackReferencesInSequences) error - RemoveViewBackReferencesInRelations(context.Context, RemoveViewBackReferencesInRelations) error + RemoveBackReferencesInRelations(context.Context, RemoveBackReferencesInRelations) error SetColumnName(context.Context, SetColumnName) error SetIndexName(context.Context, SetIndexName) error SetConstraintName(context.Context, SetConstraintName) error @@ -365,8 +365,8 @@ func (op UpdateBackReferencesInSequences) Visit(ctx context.Context, v Immediate } // Visit is part of the ImmediateMutationOp interface. -func (op RemoveViewBackReferencesInRelations) Visit(ctx context.Context, v ImmediateMutationVisitor) error { - return v.RemoveViewBackReferencesInRelations(ctx, op) +func (op RemoveBackReferencesInRelations) Visit(ctx context.Context, v ImmediateMutationVisitor) error { + return v.RemoveBackReferencesInRelations(ctx, op) } // Visit is part of the ImmediateMutationOp interface. diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_view.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_view.go index 61014a4d21bc..e69b52ba2a33 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_view.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_view.go @@ -51,13 +51,13 @@ func init() { TypeIDs: this.UsesTypeIDs, } }), - emit(func(this *scpb.View) *scop.RemoveViewBackReferencesInRelations { + emit(func(this *scpb.View) *scop.RemoveBackReferencesInRelations { if len(this.UsesRelationIDs) == 0 { return nil } - return &scop.RemoveViewBackReferencesInRelations{ - BackReferencedViewID: this.ViewID, - RelationIDs: this.UsesRelationIDs, + return &scop.RemoveBackReferencesInRelations{ + BackReferencedID: this.ViewID, + RelationIDs: this.UsesRelationIDs, } }), ), diff --git a/pkg/sql/schemachanger/scplan/testdata/alter_table_drop_column b/pkg/sql/schemachanger/scplan/testdata/alter_table_drop_column index bcca17b701f2..3f5b181cb46d 100644 --- a/pkg/sql/schemachanger/scplan/testdata/alter_table_drop_column +++ b/pkg/sql/schemachanger/scplan/testdata/alter_table_drop_column @@ -387,8 +387,8 @@ PostCommitNonRevertiblePhase stage 1 of 3 with 24 MutationType ops BackReferencedDescriptorID: 108 TypeIDs: - 104 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 108 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 108 RelationIDs: - 107 *scop.RemoveDroppedColumnType @@ -1376,8 +1376,8 @@ PostCommitNonRevertiblePhase stage 1 of 3 with 25 MutationType ops BackReferencedDescriptorID: 108 TypeIDs: - 104 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 108 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 108 RelationIDs: - 107 *scop.RemoveDroppedColumnType diff --git a/pkg/sql/schemachanger/scplan/testdata/drop_database b/pkg/sql/schemachanger/scplan/testdata/drop_database index ec3c6e866bec..09076481b486 100644 --- a/pkg/sql/schemachanger/scplan/testdata/drop_database +++ b/pkg/sql/schemachanger/scplan/testdata/drop_database @@ -830,27 +830,27 @@ PreCommitPhase stage 2 of 2 with 89 MutationType ops - 108 *scop.MarkDescriptorAsDropped DescriptorID: 111 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 111 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 111 RelationIDs: - 109 *scop.MarkDescriptorAsDropped DescriptorID: 112 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 112 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 112 RelationIDs: - 111 *scop.MarkDescriptorAsDropped DescriptorID: 113 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 113 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 113 RelationIDs: - 111 - 112 *scop.MarkDescriptorAsDropped DescriptorID: 114 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 114 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 114 RelationIDs: - 112 *scop.MarkDescriptorAsDropped @@ -864,8 +864,8 @@ PreCommitPhase stage 2 of 2 with 89 MutationType ops TypeIDs: - 115 - 116 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 117 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 117 RelationIDs: - 114 *scop.DrainDescriptorName diff --git a/pkg/sql/schemachanger/scplan/testdata/drop_index b/pkg/sql/schemachanger/scplan/testdata/drop_index index f0b2c38757de..64a240b31d13 100644 --- a/pkg/sql/schemachanger/scplan/testdata/drop_index +++ b/pkg/sql/schemachanger/scplan/testdata/drop_index @@ -702,8 +702,8 @@ PreCommitPhase stage 2 of 2 with 10 MutationType ops ops: *scop.MarkDescriptorAsDropped DescriptorID: 105 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 105 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 105 RelationIDs: - 104 *scop.MakePublicSecondaryIndexWriteOnly @@ -1070,8 +1070,8 @@ PreCommitPhase stage 2 of 2 with 12 MutationType ops ops: *scop.MarkDescriptorAsDropped DescriptorID: 107 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 107 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 107 RelationIDs: - 106 *scop.RemoveColumnDefaultExpression diff --git a/pkg/sql/schemachanger/scplan/testdata/drop_owned_by b/pkg/sql/schemachanger/scplan/testdata/drop_owned_by index 92d62fa6801c..2ba0eb382b13 100644 --- a/pkg/sql/schemachanger/scplan/testdata/drop_owned_by +++ b/pkg/sql/schemachanger/scplan/testdata/drop_owned_by @@ -548,8 +548,8 @@ PreCommitPhase stage 2 of 2 with 57 MutationType ops - 107 *scop.MarkDescriptorAsDropped DescriptorID: 110 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 110 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 110 RelationIDs: - 108 *scop.MarkDescriptorAsDropped @@ -563,8 +563,8 @@ PreCommitPhase stage 2 of 2 with 57 MutationType ops TypeIDs: - 111 - 112 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 113 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 113 RelationIDs: - 110 *scop.DrainDescriptorName diff --git a/pkg/sql/schemachanger/scplan/testdata/drop_schema b/pkg/sql/schemachanger/scplan/testdata/drop_schema index 4211191459a9..77bb08969ee6 100644 --- a/pkg/sql/schemachanger/scplan/testdata/drop_schema +++ b/pkg/sql/schemachanger/scplan/testdata/drop_schema @@ -1728,27 +1728,27 @@ PreCommitPhase stage 2 of 2 with 68 MutationType ops - 105 *scop.MarkDescriptorAsDropped DescriptorID: 107 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 107 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 107 RelationIDs: - 106 *scop.MarkDescriptorAsDropped DescriptorID: 108 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 108 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 108 RelationIDs: - 107 *scop.MarkDescriptorAsDropped DescriptorID: 109 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 109 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 109 RelationIDs: - 107 - 108 *scop.MarkDescriptorAsDropped DescriptorID: 110 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 110 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 110 RelationIDs: - 108 *scop.MarkDescriptorAsDropped @@ -1762,8 +1762,8 @@ PreCommitPhase stage 2 of 2 with 68 MutationType ops TypeIDs: - 111 - 112 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 113 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 113 RelationIDs: - 110 *scop.DrainDescriptorName diff --git a/pkg/sql/schemachanger/scplan/testdata/drop_table b/pkg/sql/schemachanger/scplan/testdata/drop_table index 44f2fc5ba190..842ea3011950 100644 --- a/pkg/sql/schemachanger/scplan/testdata/drop_table +++ b/pkg/sql/schemachanger/scplan/testdata/drop_table @@ -371,8 +371,8 @@ PreCommitPhase stage 2 of 2 with 43 MutationType ops DescriptorID: 110 *scop.MarkDescriptorAsDropped DescriptorID: 111 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 111 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 111 RelationIDs: - 109 *scop.DrainDescriptorName diff --git a/pkg/sql/schemachanger/scplan/testdata/drop_view b/pkg/sql/schemachanger/scplan/testdata/drop_view index b6209925e2f3..333e35ad5f00 100644 --- a/pkg/sql/schemachanger/scplan/testdata/drop_view +++ b/pkg/sql/schemachanger/scplan/testdata/drop_view @@ -76,8 +76,8 @@ PreCommitPhase stage 2 of 2 with 9 MutationType ops ops: *scop.MarkDescriptorAsDropped DescriptorID: 105 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 105 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 105 RelationIDs: - 104 *scop.DrainDescriptorName @@ -605,27 +605,27 @@ PreCommitPhase stage 2 of 2 with 45 MutationType ops ops: *scop.MarkDescriptorAsDropped DescriptorID: 105 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 105 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 105 RelationIDs: - 104 *scop.MarkDescriptorAsDropped DescriptorID: 106 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 106 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 106 RelationIDs: - 105 *scop.MarkDescriptorAsDropped DescriptorID: 107 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 107 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 107 RelationIDs: - 105 - 106 *scop.MarkDescriptorAsDropped DescriptorID: 108 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 108 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 108 RelationIDs: - 106 *scop.MarkDescriptorAsDropped @@ -635,8 +635,8 @@ PreCommitPhase stage 2 of 2 with 45 MutationType ops TypeIDs: - 109 - 110 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 111 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 111 RelationIDs: - 108 *scop.DrainDescriptorName diff --git a/pkg/sql/schemachanger/testdata/explain/drop_index_with_materialized_view_dep b/pkg/sql/schemachanger/testdata/explain/drop_index_with_materialized_view_dep index 5121ea642c4a..8545dacb89a4 100644 --- a/pkg/sql/schemachanger/testdata/explain/drop_index_with_materialized_view_dep +++ b/pkg/sql/schemachanger/testdata/explain/drop_index_with_materialized_view_dep @@ -107,7 +107,7 @@ Schema change plan for DROP INDEX ‹defaultdb›.‹public›.‹v2›@‹idx │ │ └── PUBLIC → ABSENT IndexName:{DescID: 106, Name: v3_pkey, IndexID: 1} │ └── 12 Mutation operations │ ├── MarkDescriptorAsDropped {"DescriptorID":106} - │ ├── RemoveViewBackReferencesInRelations {"BackReferencedViewID":106} + │ ├── RemoveBackReferencesInRelations {"BackReferencedID":106} │ ├── RemoveColumnDefaultExpression {"ColumnID":2,"TableID":106} │ ├── MakePublicSecondaryIndexWriteOnly {"IndexID":2,"TableID":105} │ ├── DrainDescriptorName {"Namespace":{"DatabaseID":100,"DescriptorID":106,"Name":"v3","SchemaID":101}} diff --git a/pkg/sql/schemachanger/testdata/explain_verbose/drop_index_with_materialized_view_dep b/pkg/sql/schemachanger/testdata/explain_verbose/drop_index_with_materialized_view_dep index a3c44d917ff7..366b5a323e56 100644 --- a/pkg/sql/schemachanger/testdata/explain_verbose/drop_index_with_materialized_view_dep +++ b/pkg/sql/schemachanger/testdata/explain_verbose/drop_index_with_materialized_view_dep @@ -730,8 +730,8 @@ EXPLAIN (ddl, verbose) DROP INDEX idx CASCADE; │ ├── • MarkDescriptorAsDropped │ │ DescriptorID: 106 │ │ -│ ├── • RemoveViewBackReferencesInRelations -│ │ BackReferencedViewID: 106 +│ ├── • RemoveBackReferencesInRelations +│ │ BackReferencedID: 106 │ │ RelationIDs: │ │ - 105 │ │ From 07ba6fbe265f512cce2714bf81263e613414fa10 Mon Sep 17 00:00:00 2001 From: Chengxiong Ruan Date: Wed, 11 Jan 2023 21:47:04 -0500 Subject: [PATCH 08/11] sql/schemachanger: declarative drop function This commit implements DROP FUNCTION given existing declarative schema changer infra (rules and ops). `RemoveObjectParent` is added to handle the removal of function signature from schema. Release note: None --- .../backup-restore/user-defined-functions | 6 +- .../backup_base_generated_test.go | 5 + .../drop_database_multiregion_primary_region | 4 +- .../end_to_end/drop_table_multiregion | 4 +- .../drop_table_multiregion_primary_region | 4 +- .../drop_database_multiregion_primary_region | 10 +- .../testdata/explain/drop_table_multiregion | 6 +- .../drop_table_multiregion_primary_region | 6 +- .../drop_database_multiregion_primary_region | 28 +- .../explain_verbose/drop_table_multiregion | 12 +- .../drop_table_multiregion_primary_region | 12 +- pkg/sql/catalog/schemadesc/schema_desc.go | 2 +- pkg/sql/drop_function_test.go | 35 +- pkg/sql/schemachanger/scbuild/BUILD.bazel | 1 + .../schemachanger/scbuild/builder_state.go | 39 ++ pkg/sql/schemachanger/scbuild/event_log.go | 4 + .../scbuild/internal/scbuildstmt/BUILD.bazel | 1 + .../scbuildstmt/alter_table_drop_column.go | 15 + .../internal/scbuildstmt/dependencies.go | 3 + .../internal/scbuildstmt/drop_function.go | 38 ++ .../internal/scbuildstmt/drop_index.go | 30 ++ .../scbuild/internal/scbuildstmt/helpers.go | 17 + .../scbuild/internal/scbuildstmt/process.go | 1 + .../scbuild/testdata/drop_function | 43 ++ .../schemachanger/scdecomp/testdata/function | 4 +- .../scdeps/sctestdeps/BUILD.bazel | 1 + .../scdeps/sctestdeps/test_deps.go | 53 ++- .../scexec/scmutationexec/references.go | 22 +- .../schemachanger/scop/immediate_mutation.go | 6 + .../immediate_mutation_visitor_generated.go | 6 + .../scplan/internal/opgen/BUILD.bazel | 7 + .../scplan/internal/opgen/opgen_function.go | 48 +++ .../internal/opgen/opgen_function_body.go | 60 +++ .../opgen/opgen_function_leakproof.go | 37 ++ .../internal/opgen/opgen_function_name.go | 37 ++ .../opgen/opgen_function_null_input.go | 37 ++ .../opgen/opgen_function_param_default.go | 39 ++ .../opgen/opgen_function_volatility.go | 37 ++ .../internal/opgen/opgen_object_parent.go | 6 + .../scplan/internal/rules/current/helpers.go | 7 +- .../scplan/internal/rules/current/op_drop.go | 4 + .../internal/rules/current/testdata/deprules | 20 +- .../internal/rules/current/testdata/oprules | 6 +- .../scplan/testdata/alter_table_drop_column | 28 +- .../scplan/testdata/drop_database | 90 ++++- .../scplan/testdata/drop_function | 169 ++++++++ .../schemachanger/scplan/testdata/drop_index | 28 +- .../scplan/testdata/drop_owned_by | 60 ++- .../schemachanger/scplan/testdata/drop_schema | 78 +++- .../scplan/testdata/drop_sequence | 20 +- .../schemachanger/scplan/testdata/drop_table | 36 +- .../schemachanger/scplan/testdata/drop_type | 32 +- .../schemachanger/scplan/testdata/drop_view | 68 +++- pkg/sql/schemachanger/screl/attr.go | 21 + pkg/sql/schemachanger/screl/scalars.go | 4 +- pkg/sql/schemachanger/sctest/cumulative.go | 1 + .../schemachanger/sctest_generated_test.go | 25 ++ .../testdata/end_to_end/drop_function | 382 ++++++++++++++++++ .../drop_index_with_materialized_view_dep | 4 +- .../testdata/end_to_end/drop_table | 4 +- .../testdata/explain/drop_function | 94 +++++ .../drop_index_with_materialized_view_dep | 8 +- .../schemachanger/testdata/explain/drop_table | 6 +- .../testdata/explain_verbose/drop_function | 374 +++++++++++++++++ .../drop_index_with_materialized_view_dep | 16 +- .../testdata/explain_verbose/drop_table | 12 +- 66 files changed, 2166 insertions(+), 157 deletions(-) create mode 100644 pkg/sql/schemachanger/scbuild/internal/scbuildstmt/drop_function.go create mode 100644 pkg/sql/schemachanger/scbuild/testdata/drop_function create mode 100644 pkg/sql/schemachanger/scplan/internal/opgen/opgen_function.go create mode 100644 pkg/sql/schemachanger/scplan/internal/opgen/opgen_function_body.go create mode 100644 pkg/sql/schemachanger/scplan/internal/opgen/opgen_function_leakproof.go create mode 100644 pkg/sql/schemachanger/scplan/internal/opgen/opgen_function_name.go create mode 100644 pkg/sql/schemachanger/scplan/internal/opgen/opgen_function_null_input.go create mode 100644 pkg/sql/schemachanger/scplan/internal/opgen/opgen_function_param_default.go create mode 100644 pkg/sql/schemachanger/scplan/internal/opgen/opgen_function_volatility.go create mode 100644 pkg/sql/schemachanger/scplan/testdata/drop_function create mode 100644 pkg/sql/schemachanger/testdata/end_to_end/drop_function create mode 100644 pkg/sql/schemachanger/testdata/explain/drop_function create mode 100644 pkg/sql/schemachanger/testdata/explain_verbose/drop_function diff --git a/pkg/ccl/backupccl/testdata/backup-restore/user-defined-functions b/pkg/ccl/backupccl/testdata/backup-restore/user-defined-functions index 8c5131fd28a8..f71274a81483 100644 --- a/pkg/ccl/backupccl/testdata/backup-restore/user-defined-functions +++ b/pkg/ccl/backupccl/testdata/backup-restore/user-defined-functions @@ -123,8 +123,7 @@ pq: cannot drop sequence sq1 because other objects depend on it exec-sql DROP TABLE sc1.tbl1 ---- -pq: cannot drop relation "tbl1" because function "f1" depends on it -HINT: you can drop f1 instead. +pq: cannot drop table tbl1 because other objects depend on it exec-sql ALTER TABLE sc1.tbl1 RENAME TO tbl1_new @@ -274,8 +273,7 @@ pq: cannot drop sequence sq1 because other objects depend on it exec-sql DROP TABLE sc1.tbl1 ---- -pq: cannot drop relation "tbl1" because function "f1" depends on it -HINT: you can drop f1 instead. +pq: cannot drop table tbl1 because other objects depend on it exec-sql ALTER TABLE sc1.tbl1 RENAME TO tbl1_new diff --git a/pkg/ccl/schemachangerccl/backup_base_generated_test.go b/pkg/ccl/schemachangerccl/backup_base_generated_test.go index 9410194ff93e..f3df746c6acd 100644 --- a/pkg/ccl/schemachangerccl/backup_base_generated_test.go +++ b/pkg/ccl/schemachangerccl/backup_base_generated_test.go @@ -103,6 +103,11 @@ func TestBackup_base_drop_column_with_index(t *testing.T) { defer log.Scope(t).Close(t) sctest.Backup(t, "pkg/sql/schemachanger/testdata/end_to_end/drop_column_with_index", newCluster) } +func TestBackup_base_drop_function(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + sctest.Backup(t, "pkg/sql/schemachanger/testdata/end_to_end/drop_function", newCluster) +} func TestBackup_base_drop_index_hash_sharded_index(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) diff --git a/pkg/ccl/schemachangerccl/testdata/end_to_end/drop_database_multiregion_primary_region b/pkg/ccl/schemachangerccl/testdata/end_to_end/drop_database_multiregion_primary_region index 69056ea0e6ed..7df3e7d67496 100644 --- a/pkg/ccl/schemachangerccl/testdata/end_to_end/drop_database_multiregion_primary_region +++ b/pkg/ccl/schemachangerccl/testdata/end_to_end/drop_database_multiregion_primary_region @@ -29,7 +29,7 @@ write *eventpb.DropDatabase to event log: statement: DROP DATABASE ‹multi_region_test_db› CASCADE tag: DROP DATABASE user: root -## StatementPhase stage 1 of 1 with 12 MutationType ops +## StatementPhase stage 1 of 1 with 15 MutationType ops delete database namespace entry {0 0 multi_region_test_db} -> 104 delete schema namespace entry {104 0 public} -> 105 delete object namespace entry {104 105 crdb_internal_region} -> 106 @@ -85,7 +85,7 @@ upsert descriptor #108 ## PreCommitPhase stage 1 of 2 with 1 MutationType op undo all catalog changes within txn #1 persist all catalog changes to storage -## PreCommitPhase stage 2 of 2 with 22 MutationType ops +## PreCommitPhase stage 2 of 2 with 25 MutationType ops delete database namespace entry {0 0 multi_region_test_db} -> 104 delete schema namespace entry {104 0 public} -> 105 delete object namespace entry {104 105 crdb_internal_region} -> 106 diff --git a/pkg/ccl/schemachangerccl/testdata/end_to_end/drop_table_multiregion b/pkg/ccl/schemachangerccl/testdata/end_to_end/drop_table_multiregion index 9adfa7d904e4..5d4e179c8bbc 100644 --- a/pkg/ccl/schemachangerccl/testdata/end_to_end/drop_table_multiregion +++ b/pkg/ccl/schemachangerccl/testdata/end_to_end/drop_table_multiregion @@ -25,7 +25,7 @@ write *eventpb.DropTable to event log: tag: DROP TABLE user: root tableName: multi_region_test_db.public.table_regional_by_row -## StatementPhase stage 1 of 1 with 6 MutationType ops +## StatementPhase stage 1 of 1 with 7 MutationType ops delete object namespace entry {104 105 table_regional_by_row} -> 108 upsert descriptor #106 ... @@ -65,7 +65,7 @@ upsert descriptor #108 ## PreCommitPhase stage 1 of 2 with 1 MutationType op undo all catalog changes within txn #1 persist all catalog changes to storage -## PreCommitPhase stage 2 of 2 with 14 MutationType ops +## PreCommitPhase stage 2 of 2 with 15 MutationType ops delete object namespace entry {104 105 table_regional_by_row} -> 108 upsert descriptor #106 type: diff --git a/pkg/ccl/schemachangerccl/testdata/end_to_end/drop_table_multiregion_primary_region b/pkg/ccl/schemachangerccl/testdata/end_to_end/drop_table_multiregion_primary_region index c040da4c5e1d..85ed0c1eb7d8 100644 --- a/pkg/ccl/schemachangerccl/testdata/end_to_end/drop_table_multiregion_primary_region +++ b/pkg/ccl/schemachangerccl/testdata/end_to_end/drop_table_multiregion_primary_region @@ -26,7 +26,7 @@ write *eventpb.DropTable to event log: tag: DROP TABLE user: root tableName: multi_region_test_db.public.table_regional_by_table -## StatementPhase stage 1 of 1 with 3 MutationType ops +## StatementPhase stage 1 of 1 with 4 MutationType ops delete object namespace entry {104 105 table_regional_by_table} -> 108 upsert descriptor #106 ... @@ -58,7 +58,7 @@ upsert descriptor #108 ## PreCommitPhase stage 1 of 2 with 1 MutationType op undo all catalog changes within txn #1 persist all catalog changes to storage -## PreCommitPhase stage 2 of 2 with 9 MutationType ops +## PreCommitPhase stage 2 of 2 with 10 MutationType ops delete object namespace entry {104 105 table_regional_by_table} -> 108 upsert descriptor #106 type: diff --git a/pkg/ccl/schemachangerccl/testdata/explain/drop_database_multiregion_primary_region b/pkg/ccl/schemachangerccl/testdata/explain/drop_database_multiregion_primary_region index d73e4e4abc8e..d6d7b53b709a 100644 --- a/pkg/ccl/schemachangerccl/testdata/explain/drop_database_multiregion_primary_region +++ b/pkg/ccl/schemachangerccl/testdata/explain/drop_database_multiregion_primary_region @@ -63,13 +63,16 @@ Schema change plan for DROP DATABASE ‹multi_region_test_db› CASCADE; │ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 108, ColumnID: 1, IndexID: 1} │ │ ├── PUBLIC → VALIDATED PrimaryIndex:{DescID: 108, IndexID: 1, ConstraintID: 1} │ │ └── PUBLIC → ABSENT IndexName:{DescID: 108, Name: table_regional_by_table_pkey, IndexID: 1} - │ └── 12 Mutation operations + │ └── 15 Mutation operations │ ├── MarkDescriptorAsDropped {"DescriptorID":104} │ ├── MarkDescriptorAsDropped {"DescriptorID":105} │ ├── RemoveSchemaParent {"Parent":{"ParentDatabaseID":104,"SchemaID":105}} │ ├── MarkDescriptorAsDropped {"DescriptorID":106} + │ ├── RemoveObjectParent {"ObjectID":106,"ParentSchemaID":105} │ ├── MarkDescriptorAsDropped {"DescriptorID":107} + │ ├── RemoveObjectParent {"ObjectID":107,"ParentSchemaID":105} │ ├── MarkDescriptorAsDropped {"DescriptorID":108} + │ ├── RemoveObjectParent {"ObjectID":108,"ParentSchemaID":105} │ ├── RemoveBackReferenceInTypes {"BackReferencedDescriptorID":108} │ ├── DrainDescriptorName {"Namespace":{"DescriptorID":104,"Name":"multi_region_tes..."}} │ ├── DrainDescriptorName {"Namespace":{"DatabaseID":104,"DescriptorID":105,"Name":"public"}} @@ -187,14 +190,17 @@ Schema change plan for DROP DATABASE ‹multi_region_test_db› CASCADE; │ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 108, ColumnID: 1, IndexID: 1} │ │ ├── PUBLIC → ABSENT PrimaryIndex:{DescID: 108, IndexID: 1, ConstraintID: 1} │ │ └── PUBLIC → ABSENT IndexName:{DescID: 108, Name: table_regional_by_table_pkey, IndexID: 1} - │ └── 22 Mutation operations + │ └── 25 Mutation operations │ ├── MarkDescriptorAsDropped {"DescriptorID":104} │ ├── RemoveDatabaseRoleSettings {"DatabaseID":104} │ ├── MarkDescriptorAsDropped {"DescriptorID":105} │ ├── RemoveSchemaParent {"Parent":{"ParentDatabaseID":104,"SchemaID":105}} │ ├── MarkDescriptorAsDropped {"DescriptorID":106} + │ ├── RemoveObjectParent {"ObjectID":106,"ParentSchemaID":105} │ ├── MarkDescriptorAsDropped {"DescriptorID":107} + │ ├── RemoveObjectParent {"ObjectID":107,"ParentSchemaID":105} │ ├── MarkDescriptorAsDropped {"DescriptorID":108} + │ ├── RemoveObjectParent {"ObjectID":108,"ParentSchemaID":105} │ ├── RemoveBackReferenceInTypes {"BackReferencedDescriptorID":108} │ ├── DrainDescriptorName {"Namespace":{"DescriptorID":104,"Name":"multi_region_tes..."}} │ ├── DrainDescriptorName {"Namespace":{"DatabaseID":104,"DescriptorID":105,"Name":"public"}} diff --git a/pkg/ccl/schemachangerccl/testdata/explain/drop_table_multiregion b/pkg/ccl/schemachangerccl/testdata/explain/drop_table_multiregion index e81881b49b92..3d8c527bc787 100644 --- a/pkg/ccl/schemachangerccl/testdata/explain/drop_table_multiregion +++ b/pkg/ccl/schemachangerccl/testdata/explain/drop_table_multiregion @@ -38,8 +38,9 @@ Schema change plan for DROP TABLE ‹multi_region_test_db›.‹public›.‹tab │ │ ├── PUBLIC → VALIDATED PrimaryIndex:{DescID: 108, IndexID: 1, ConstraintID: 1} │ │ ├── PUBLIC → ABSENT IndexPartitioning:{DescID: 108, IndexID: 1} │ │ └── PUBLIC → ABSENT IndexName:{DescID: 108, Name: table_regional_by_row_pkey, IndexID: 1} - │ └── 6 Mutation operations + │ └── 7 Mutation operations │ ├── MarkDescriptorAsDropped {"DescriptorID":108} + │ ├── RemoveObjectParent {"ObjectID":108,"ParentSchemaID":105} │ ├── RemoveColumnDefaultExpression {"ColumnID":2,"TableID":108} │ ├── UpdateTableBackReferencesInTypes {"BackReferencedTableID":108} │ ├── DrainDescriptorName {"Namespace":{"DatabaseID":104,"DescriptorID":108,"Name":"table_regional_b...","SchemaID":105}} @@ -106,8 +107,9 @@ Schema change plan for DROP TABLE ‹multi_region_test_db›.‹public›.‹tab │ │ ├── PUBLIC → ABSENT PrimaryIndex:{DescID: 108, IndexID: 1, ConstraintID: 1} │ │ ├── PUBLIC → ABSENT IndexPartitioning:{DescID: 108, IndexID: 1} │ │ └── PUBLIC → ABSENT IndexName:{DescID: 108, Name: table_regional_by_row_pkey, IndexID: 1} - │ └── 14 Mutation operations + │ └── 15 Mutation operations │ ├── MarkDescriptorAsDropped {"DescriptorID":108} + │ ├── RemoveObjectParent {"ObjectID":108,"ParentSchemaID":105} │ ├── RemoveColumnDefaultExpression {"ColumnID":2,"TableID":108} │ ├── UpdateTableBackReferencesInTypes {"BackReferencedTableID":108} │ ├── DrainDescriptorName {"Namespace":{"DatabaseID":104,"DescriptorID":108,"Name":"table_regional_b...","SchemaID":105}} diff --git a/pkg/ccl/schemachangerccl/testdata/explain/drop_table_multiregion_primary_region b/pkg/ccl/schemachangerccl/testdata/explain/drop_table_multiregion_primary_region index bf8a56d52f8f..9512d93b0ecd 100644 --- a/pkg/ccl/schemachangerccl/testdata/explain/drop_table_multiregion_primary_region +++ b/pkg/ccl/schemachangerccl/testdata/explain/drop_table_multiregion_primary_region @@ -31,8 +31,9 @@ Schema change plan for DROP TABLE ‹multi_region_test_db›.‹public›.‹tab │ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 108, ColumnID: 1, IndexID: 1} │ │ ├── PUBLIC → VALIDATED PrimaryIndex:{DescID: 108, IndexID: 1, ConstraintID: 1} │ │ └── PUBLIC → ABSENT IndexName:{DescID: 108, Name: table_regional_by_table_pkey, IndexID: 1} - │ └── 3 Mutation operations + │ └── 4 Mutation operations │ ├── MarkDescriptorAsDropped {"DescriptorID":108} + │ ├── RemoveObjectParent {"ObjectID":108,"ParentSchemaID":105} │ ├── RemoveBackReferenceInTypes {"BackReferencedDescriptorID":108} │ └── DrainDescriptorName {"Namespace":{"DatabaseID":104,"DescriptorID":108,"Name":"table_regional_b...","SchemaID":105}} ├── PreCommitPhase @@ -82,8 +83,9 @@ Schema change plan for DROP TABLE ‹multi_region_test_db›.‹public›.‹tab │ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 108, ColumnID: 1, IndexID: 1} │ │ ├── PUBLIC → ABSENT PrimaryIndex:{DescID: 108, IndexID: 1, ConstraintID: 1} │ │ └── PUBLIC → ABSENT IndexName:{DescID: 108, Name: table_regional_by_table_pkey, IndexID: 1} - │ └── 9 Mutation operations + │ └── 10 Mutation operations │ ├── MarkDescriptorAsDropped {"DescriptorID":108} + │ ├── RemoveObjectParent {"ObjectID":108,"ParentSchemaID":105} │ ├── RemoveBackReferenceInTypes {"BackReferencedDescriptorID":108} │ ├── DrainDescriptorName {"Namespace":{"DatabaseID":104,"DescriptorID":108,"Name":"table_regional_b...","SchemaID":105}} │ ├── MakeDeleteOnlyColumnAbsent {"ColumnID":1,"TableID":108} diff --git a/pkg/ccl/schemachangerccl/testdata/explain_verbose/drop_database_multiregion_primary_region b/pkg/ccl/schemachangerccl/testdata/explain_verbose/drop_database_multiregion_primary_region index c718614a57ea..2c9075cb2d70 100644 --- a/pkg/ccl/schemachangerccl/testdata/explain_verbose/drop_database_multiregion_primary_region +++ b/pkg/ccl/schemachangerccl/testdata/explain_verbose/drop_database_multiregion_primary_region @@ -466,7 +466,7 @@ EXPLAIN (ddl, verbose) DROP DATABASE multi_region_test_db CASCADE; │ │ └── • skip PUBLIC → ABSENT operations │ │ rule: "skip index dependents removal ops on relation drop" │ │ -│ └── • 12 Mutation operations +│ └── • 15 Mutation operations │ │ │ ├── • MarkDescriptorAsDropped │ │ DescriptorID: 104 @@ -482,12 +482,24 @@ EXPLAIN (ddl, verbose) DROP DATABASE multi_region_test_db CASCADE; │ ├── • MarkDescriptorAsDropped │ │ DescriptorID: 106 │ │ +│ ├── • RemoveObjectParent +│ │ ObjectID: 106 +│ │ ParentSchemaID: 105 +│ │ │ ├── • MarkDescriptorAsDropped │ │ DescriptorID: 107 │ │ +│ ├── • RemoveObjectParent +│ │ ObjectID: 107 +│ │ ParentSchemaID: 105 +│ │ │ ├── • MarkDescriptorAsDropped │ │ DescriptorID: 108 │ │ +│ ├── • RemoveObjectParent +│ │ ObjectID: 108 +│ │ ParentSchemaID: 105 +│ │ │ ├── • RemoveBackReferenceInTypes │ │ BackReferencedDescriptorID: 108 │ │ TypeIDs: @@ -1192,7 +1204,7 @@ EXPLAIN (ddl, verbose) DROP DATABASE multi_region_test_db CASCADE; │ │ └── • skip PUBLIC → ABSENT operations │ │ rule: "skip index dependents removal ops on relation drop" │ │ -│ └── • 22 Mutation operations +│ └── • 25 Mutation operations │ │ │ ├── • MarkDescriptorAsDropped │ │ DescriptorID: 104 @@ -1211,12 +1223,24 @@ EXPLAIN (ddl, verbose) DROP DATABASE multi_region_test_db CASCADE; │ ├── • MarkDescriptorAsDropped │ │ DescriptorID: 106 │ │ +│ ├── • RemoveObjectParent +│ │ ObjectID: 106 +│ │ ParentSchemaID: 105 +│ │ │ ├── • MarkDescriptorAsDropped │ │ DescriptorID: 107 │ │ +│ ├── • RemoveObjectParent +│ │ ObjectID: 107 +│ │ ParentSchemaID: 105 +│ │ │ ├── • MarkDescriptorAsDropped │ │ DescriptorID: 108 │ │ +│ ├── • RemoveObjectParent +│ │ ObjectID: 108 +│ │ ParentSchemaID: 105 +│ │ │ ├── • RemoveBackReferenceInTypes │ │ BackReferencedDescriptorID: 108 │ │ TypeIDs: diff --git a/pkg/ccl/schemachangerccl/testdata/explain_verbose/drop_table_multiregion b/pkg/ccl/schemachangerccl/testdata/explain_verbose/drop_table_multiregion index 4039d1003bd2..757919587dc4 100644 --- a/pkg/ccl/schemachangerccl/testdata/explain_verbose/drop_table_multiregion +++ b/pkg/ccl/schemachangerccl/testdata/explain_verbose/drop_table_multiregion @@ -278,11 +278,15 @@ EXPLAIN (ddl, verbose) DROP TABLE multi_region_test_db.public.table_regional_by_ │ │ └── • skip PUBLIC → ABSENT operations │ │ rule: "skip index dependents removal ops on relation drop" │ │ -│ └── • 6 Mutation operations +│ └── • 7 Mutation operations │ │ │ ├── • MarkDescriptorAsDropped │ │ DescriptorID: 108 │ │ +│ ├── • RemoveObjectParent +│ │ ObjectID: 108 +│ │ ParentSchemaID: 105 +│ │ │ ├── • RemoveColumnDefaultExpression │ │ ColumnID: 2 │ │ TableID: 108 @@ -735,11 +739,15 @@ EXPLAIN (ddl, verbose) DROP TABLE multi_region_test_db.public.table_regional_by_ │ │ └── • skip PUBLIC → ABSENT operations │ │ rule: "skip index dependents removal ops on relation drop" │ │ -│ └── • 14 Mutation operations +│ └── • 15 Mutation operations │ │ │ ├── • MarkDescriptorAsDropped │ │ DescriptorID: 108 │ │ +│ ├── • RemoveObjectParent +│ │ ObjectID: 108 +│ │ ParentSchemaID: 105 +│ │ │ ├── • RemoveColumnDefaultExpression │ │ ColumnID: 2 │ │ TableID: 108 diff --git a/pkg/ccl/schemachangerccl/testdata/explain_verbose/drop_table_multiregion_primary_region b/pkg/ccl/schemachangerccl/testdata/explain_verbose/drop_table_multiregion_primary_region index 3d017a93a10c..64d664f5ba0a 100644 --- a/pkg/ccl/schemachangerccl/testdata/explain_verbose/drop_table_multiregion_primary_region +++ b/pkg/ccl/schemachangerccl/testdata/explain_verbose/drop_table_multiregion_primary_region @@ -199,11 +199,15 @@ EXPLAIN (ddl, verbose) DROP TABLE multi_region_test_db.public.table_regional_by_ │ │ └── • skip PUBLIC → ABSENT operations │ │ rule: "skip index dependents removal ops on relation drop" │ │ -│ └── • 3 Mutation operations +│ └── • 4 Mutation operations │ │ │ ├── • MarkDescriptorAsDropped │ │ DescriptorID: 108 │ │ +│ ├── • RemoveObjectParent +│ │ ObjectID: 108 +│ │ ParentSchemaID: 105 +│ │ │ ├── • RemoveBackReferenceInTypes │ │ BackReferencedDescriptorID: 108 │ │ TypeIDs: @@ -520,11 +524,15 @@ EXPLAIN (ddl, verbose) DROP TABLE multi_region_test_db.public.table_regional_by_ │ │ └── • skip PUBLIC → ABSENT operations │ │ rule: "skip index dependents removal ops on relation drop" │ │ -│ └── • 9 Mutation operations +│ └── • 10 Mutation operations │ │ │ ├── • MarkDescriptorAsDropped │ │ DescriptorID: 108 │ │ +│ ├── • RemoveObjectParent +│ │ ObjectID: 108 +│ │ ParentSchemaID: 105 +│ │ │ ├── • RemoveBackReferenceInTypes │ │ BackReferencedDescriptorID: 108 │ │ TypeIDs: diff --git a/pkg/sql/catalog/schemadesc/schema_desc.go b/pkg/sql/catalog/schemadesc/schema_desc.go index 05f6cacb0246..11fd1c6d0693 100644 --- a/pkg/sql/catalog/schemadesc/schema_desc.go +++ b/pkg/sql/catalog/schemadesc/schema_desc.go @@ -458,7 +458,7 @@ func (desc *Mutable) AddFunction(name string, f descpb.SchemaDescriptor_Function // RemoveFunction removes a UDF overload signature from the schema descriptor. func (desc *Mutable) RemoveFunction(name string, id descpb.ID) { if fn, ok := desc.Functions[name]; ok { - updated := fn.Overloads[:0] + var updated []descpb.SchemaDescriptor_FunctionOverload for _, ol := range fn.Overloads { if ol.ID != id { updated = append(updated, ol) diff --git a/pkg/sql/drop_function_test.go b/pkg/sql/drop_function_test.go index dfd740348b99..ee3032daa94b 100644 --- a/pkg/sql/drop_function_test.go +++ b/pkg/sql/drop_function_test.go @@ -147,7 +147,7 @@ SELECT nextval(105:::REGCLASS);`, err = sql.TestingDescsTxn(ctx, s, func(ctx context.Context, txn isql.Txn, col *descs.Collection) error { _, err := col.ByIDWithLeased(txn.KV()).WithoutNonPublic().Get().Function(ctx, 109) require.Error(t, err) - require.Regexp(t, "descriptor is being dropped", err.Error()) + require.Regexp(t, "function undefined", err.Error()) // Make sure columns and indexes has correct back references. tn := tree.MakeTableNameWithSchema("defaultdb", "public", "t") @@ -197,6 +197,7 @@ CREATE TABLE t( a INT PRIMARY KEY, b INT, C INT, + d INT, INDEX t_idx_b(b), INDEX t_idx_c(c) ); @@ -209,6 +210,7 @@ CREATE FUNCTION test_sc.f(a notmyworkday) RETURNS INT IMMUTABLE LANGUAGE SQL AS SELECT a FROM t; SELECT b FROM t@t_idx_b; SELECT c FROM t@t_idx_c; + SELECT d FROM t; SELECT a FROM v; SELECT nextval('sq1'); $$; @@ -225,20 +227,23 @@ USE defaultdb; tDB.Exec(t, "SET use_declarative_schema_changer = off;") testCases := []struct { - stmt string - expectedErr string + stmt string + expectedErr string + dscExpectedErr string }{ { stmt: "DROP SEQUENCE sq1", expectedErr: "pq: cannot drop sequence sq1 because other objects depend on it", }, { - stmt: "DROP TABLE t", - expectedErr: `pq: cannot drop relation "t" because function "f" depends on it`, + stmt: "DROP TABLE t", + expectedErr: `pq: cannot drop relation "t" because function "f" depends on it`, + dscExpectedErr: `pq: cannot drop table t because other objects depend on it`, }, { - stmt: "DROP VIEW v", - expectedErr: `pq: cannot drop relation "v" because function "f" depends on it`, + stmt: "DROP VIEW v", + expectedErr: `pq: cannot drop relation "v" because function "f" depends on it`, + dscExpectedErr: `pq: cannot drop view v because other objects depend on it`, }, { stmt: "ALTER TABLE t RENAME TO t_new", @@ -249,8 +254,8 @@ USE defaultdb; expectedErr: `pq: cannot set schema on relation "t" because function "f" depends on it`, }, { - stmt: "ALTER TABLE t DROP COLUMN b", - expectedErr: `pq: cannot drop column "b" because function "f" depends on it`, + stmt: "ALTER TABLE t DROP COLUMN d", + expectedErr: `pq: cannot drop column "d" because function "f" depends on it`, }, { stmt: "ALTER TABLE t RENAME COLUMN b TO bb", @@ -296,7 +301,11 @@ USE defaultdb; for i, tc := range testCases { t.Run(strconv.Itoa(i), func(t *testing.T) { _, err := sqlDB.Exec(tc.stmt) - require.Equal(t, tc.expectedErr, err.Error()) + if tc.dscExpectedErr != "" { + require.Equal(t, tc.dscExpectedErr, err.Error()) + } else { + require.Equal(t, tc.expectedErr, err.Error()) + } }) } } @@ -312,6 +321,7 @@ CREATE TABLE t( a INT PRIMARY KEY, b INT, C INT, + d INT, INDEX t_idx_b(b), INDEX t_idx_c(c) ); @@ -324,6 +334,7 @@ CREATE FUNCTION test_sc.f(a notmyworkday) RETURNS INT IMMUTABLE LANGUAGE SQL AS SELECT a FROM t; SELECT b FROM t@t_idx_b; SELECT c FROM t@t_idx_c; + SELECT d FROM t; SELECT a FROM v; SELECT nextval('sq1'); $$; @@ -347,7 +358,7 @@ $$; }, { testName: "drop column", - stmt: "ALTER TABLE t DROP COLUMN b CASCADE", + stmt: "ALTER TABLE t DROP COLUMN d CASCADE", }, { testName: "drop index", @@ -420,7 +431,7 @@ $$; err = sql.TestingDescsTxn(ctx, s, func(ctx context.Context, txn isql.Txn, col *descs.Collection) error { _, err := col.ByIDWithLeased(txn.KV()).WithoutNonPublic().Get().Function(ctx, 113) require.Error(t, err) - require.Regexp(t, "descriptor is being dropped", err.Error()) + require.Regexp(t, "function undefined", err.Error()) return nil }) require.NoError(t, err) diff --git a/pkg/sql/schemachanger/scbuild/BUILD.bazel b/pkg/sql/schemachanger/scbuild/BUILD.bazel index eb3cc0075797..6258aae35fbc 100644 --- a/pkg/sql/schemachanger/scbuild/BUILD.bazel +++ b/pkg/sql/schemachanger/scbuild/BUILD.bazel @@ -23,6 +23,7 @@ go_library( "//pkg/sql/catalog/colinfo", "//pkg/sql/catalog/descpb", "//pkg/sql/catalog/descs", + "//pkg/sql/catalog/funcdesc", "//pkg/sql/catalog/nstree", "//pkg/sql/catalog/resolver", "//pkg/sql/catalog/schemaexpr", diff --git a/pkg/sql/schemachanger/scbuild/builder_state.go b/pkg/sql/schemachanger/scbuild/builder_state.go index 28f642f0ed85..22fc055530ec 100644 --- a/pkg/sql/schemachanger/scbuild/builder_state.go +++ b/pkg/sql/schemachanger/scbuild/builder_state.go @@ -19,6 +19,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/funcdesc" "github.com/cockroachdb/cockroach/pkg/sql/catalog/schemaexpr" "github.com/cockroachdb/cockroach/pkg/sql/catalog/seqexpr" "github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc" @@ -959,6 +960,44 @@ func (b *builderState) ResolveConstraint( }) } +func (b *builderState) ResolveUDF( + fnObj *tree.FuncObj, p scbuildstmt.ResolveParams, +) scbuildstmt.ElementResultSet { + fd, err := b.cr.ResolveFunction(b.ctx, fnObj.FuncName.ToUnresolvedObjectName().ToUnresolvedName(), b.semaCtx.SearchPath) + if err != nil { + if p.IsExistenceOptional && errors.Is(err, tree.ErrFunctionUndefined) { + return nil + } + panic(err) + } + + paramTypes, err := fnObj.ParamTypes(b.ctx, b.cr) + if err != nil { + return nil + } + ol, err := fd.MatchOverload(paramTypes, fnObj.FuncName.Schema(), b.semaCtx.SearchPath) + if err != nil { + if p.IsExistenceOptional && errors.Is(err, tree.ErrFunctionUndefined) { + return nil + } + panic(err) + } + + if !ol.IsUDF { + panic( + errors.Errorf( + "cannot perform schema change on function %s%s because it is required by the database system", + fnObj.FuncName.Object(), ol.Signature(true), + ), + ) + } + + fnID := funcdesc.UserDefinedFunctionOIDToID(ol.Oid) + b.mustOwn(fnID) + b.ensureDescriptor(fnID) + return b.descCache[fnID].ers +} + func (b *builderState) ensureDescriptor(id catid.DescID) { if _, found := b.descCache[id]; found { return diff --git a/pkg/sql/schemachanger/scbuild/event_log.go b/pkg/sql/schemachanger/scbuild/event_log.go index 6ed6934e95c9..116d3797a8b9 100644 --- a/pkg/sql/schemachanger/scbuild/event_log.go +++ b/pkg/sql/schemachanger/scbuild/event_log.go @@ -343,6 +343,10 @@ func (pb payloadBuilder) build(b buildCtx) logpb.EventPayload { Comment: e.Comment, NullComment: pb.TargetStatus != scpb.Status_PUBLIC, } + case *scpb.Function: + return &eventpb.DropFunction{ + FunctionName: fullyQualifiedName(b, e), + } } if _, _, tbl := scpb.FindTable(b.QueryByID(screl.GetDescID(pb.Element()))); tbl != nil { return &eventpb.AlterTable{ diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/BUILD.bazel b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/BUILD.bazel index 0021dc016c6e..ade7f1c7fdbd 100644 --- a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/BUILD.bazel +++ b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/BUILD.bazel @@ -13,6 +13,7 @@ go_library( "create_index.go", "dependencies.go", "drop_database.go", + "drop_function.go", "drop_index.go", "drop_owned_by.go", "drop_schema.go", diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_drop_column.go b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_drop_column.go index 8fdebc503322..a2e73aa1faba 100644 --- a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_drop_column.go +++ b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_drop_column.go @@ -261,6 +261,15 @@ func dropColumn( dropRestrictDescriptor(b, e.SequenceID) undroppedSeqBackrefsToCheck.Add(e.SequenceID) } + case *scpb.FunctionBody: + if behavior != tree.DropCascade { + _, _, fnName := scpb.FindFunctionName(b.QueryByID(e.FunctionID)) + panic(sqlerrors.NewDependentObjectErrorf( + "cannot drop column %q because function %q depends on it", + cn.Name, fnName.Name), + ) + } + dropCascadeDescriptor(b, e.FunctionID) default: b.Drop(e) } @@ -362,6 +371,12 @@ func walkDropColumnDependencies(b BuildCtx, col *scpb.Column, fn func(e scpb.Ele catalog.MakeTableColSet(elt.ReferencedColumnIDs...).Contains(col.ColumnID) { fn(e) } + case *scpb.FunctionBody: + for _, ref := range elt.UsesTables { + if ref.TableID == col.TableID && catalog.MakeTableColSet(ref.ColumnIDs...).Contains(col.ColumnID) { + fn(e) + } + } } }) } diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/dependencies.go b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/dependencies.go index aee5948e03c8..8dc30f7837e9 100644 --- a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/dependencies.go +++ b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/dependencies.go @@ -310,6 +310,9 @@ type NameResolver interface { // ResolveIndex retrieves an index by name and returns its elements. ResolveIndex(relationID catid.DescID, indexName tree.Name, p ResolveParams) ElementResultSet + // ResolveUDF retrieves a user defined function and returns its elements. + ResolveUDF(fnObj *tree.FuncObj, p ResolveParams) ElementResultSet + // ResolveIndexByName retrieves a table which contains the target // index and returns its elements. Name of database, schema or table may be // missing. diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/drop_function.go b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/drop_function.go new file mode 100644 index 000000000000..b790109d4e2f --- /dev/null +++ b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/drop_function.go @@ -0,0 +1,38 @@ +// Copyright 2023 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 scbuildstmt + +import ( + "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scerrors" + "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb" + "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" +) + +func DropFunction(b BuildCtx, n *tree.DropFunction) { + if n.DropBehavior == tree.DropCascade { + // TODO(chengxiong): remove this when we allow UDF usage. + panic(scerrors.NotImplementedErrorf(n, "cascade dropping functions")) + } + for _, f := range n.Functions { + elts := b.ResolveUDF(&f, ResolveParams{ + IsExistenceOptional: n.IfExists, + }) + _, _, fn := scpb.FindFunction(elts) + if fn == nil { + continue + } + f.FuncName.ObjectNamePrefix = b.NamePrefix(fn) + dropRestrictDescriptor(b, fn.FunctionID) + b.LogEventForExistingTarget(fn) + b.IncrementSubWorkID() + b.IncrementSchemaChangeDropCounter("function") + } +} diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/drop_index.go b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/drop_index.go index b3b2915d28c9..95aeb29531e1 100644 --- a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/drop_index.go +++ b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/drop_index.go @@ -129,6 +129,9 @@ func dropSecondaryIndex( // dependents. maybeDropDependentViews(next, sie, indexName.Index.String(), dropBehavior) + // Maybe drop dependent functions. + maybeDropDependentFunctions(next, sie, indexName.Index.String(), dropBehavior) + // Maybe drop dependent FK constraints. // A PK or unique constraint is required to serve an inbound FK constraint. // It is possible that there is an inbound FK constraint 'fk' and it's @@ -186,6 +189,33 @@ func maybeDropDependentViews( }) } +func maybeDropDependentFunctions( + b BuildCtx, + toBeDroppedIndex *scpb.SecondaryIndex, + toBeDroppedIndexName string, + dropBehavior tree.DropBehavior, +) { + scpb.ForEachFunctionBody(b.BackReferences(toBeDroppedIndex.TableID), func( + current scpb.Status, target scpb.TargetStatus, e *scpb.FunctionBody, + ) { + for _, forwardRef := range e.UsesTables { + if forwardRef.IndexID != toBeDroppedIndex.IndexID { + continue + } + // This view depends on the to-be-dropped index; + if dropBehavior != tree.DropCascade { + // Get view name for the error message + _, _, fnName := scpb.FindFunctionName(b.QueryByID(e.FunctionID)) + panic(errors.WithHintf( + sqlerrors.NewDependentObjectErrorf("cannot drop index %q because function %q depends on it", + toBeDroppedIndexName, fnName.Name), "you can drop %q instead.", fnName.Name)) + } else { + dropCascadeDescriptor(b, e.FunctionID) + } + } + }) +} + // maybeDropDependentFKConstraints attempts to drop all FK constraints // that depend on the to be dropped index if CASCADE. // A FK constraint can only exist if there is `PRIMARY KEY` or `UNIQUE` diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/helpers.go b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/helpers.go index a6761ebbee24..2990b8acc6c0 100644 --- a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/helpers.go +++ b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/helpers.go @@ -30,6 +30,11 @@ import ( func qualifiedName(b BuildCtx, id catid.DescID) string { _, _, ns := scpb.FindNamespace(b.QueryByID(id)) + if ns == nil { + // Function descriptors don't have namespace. So we need to handle this + // special case here. + return qualifiedFunctionName(b, id) + } _, _, sc := scpb.FindNamespace(b.QueryByID(ns.SchemaID)) _, _, db := scpb.FindNamespace(b.QueryByID(ns.DatabaseID)) if db == nil { @@ -41,6 +46,16 @@ func qualifiedName(b BuildCtx, id catid.DescID) string { return db.Name + "." + sc.Name + "." + ns.Name } +func qualifiedFunctionName(b BuildCtx, id catid.DescID) string { + elts := b.QueryByID(id) + _, _, fnName := scpb.FindFunctionName(elts) + _, _, objParent := scpb.FindObjectParent(elts) + _, _, scName := scpb.FindNamespace(b.QueryByID(objParent.ParentSchemaID)) + _, _, scParent := scpb.FindSchemaParent(b.QueryByID(objParent.ParentSchemaID)) + _, _, dbName := scpb.FindNamespace(b.QueryByID(scParent.ParentDatabaseID)) + return dbName.Name + "." + scName.Name + "." + fnName.Name +} + func simpleName(b BuildCtx, id catid.DescID) string { _, _, ns := scpb.FindNamespace(b.QueryByID(id)) return ns.Name @@ -204,6 +219,8 @@ func dropCascadeDescriptor(b BuildCtx, id catid.DescID) { dropCascadeDescriptor(next, t.TypeID) case *scpb.CompositeType: dropCascadeDescriptor(next, t.TypeID) + case *scpb.FunctionBody: + dropCascadeDescriptor(next, t.FunctionID) case *scpb.Column, *scpb.ColumnType, *scpb.SecondaryIndexPartial: // These only have type references. break diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/process.go b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/process.go index a99164a142a2..1747fd0a7f8d 100644 --- a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/process.go +++ b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/process.go @@ -103,6 +103,7 @@ var supportedStatements = map[reflect.Type]supportedStatement{ reflect.TypeOf((*tree.CommentOnIndex)(nil)): {fn: CommentOnIndex, on: true, minSupportedClusterVersion: clusterversion.V22_2Start}, reflect.TypeOf((*tree.CommentOnConstraint)(nil)): {fn: CommentOnConstraint, on: true, minSupportedClusterVersion: clusterversion.V22_2Start}, reflect.TypeOf((*tree.DropIndex)(nil)): {fn: DropIndex, on: true, minSupportedClusterVersion: clusterversion.V23_1Start}, + reflect.TypeOf((*tree.DropFunction)(nil)): {fn: DropFunction, on: true, minSupportedClusterVersion: clusterversion.V23_1Start}, } func init() { diff --git a/pkg/sql/schemachanger/scbuild/testdata/drop_function b/pkg/sql/schemachanger/scbuild/testdata/drop_function new file mode 100644 index 000000000000..ed694de7c4a3 --- /dev/null +++ b/pkg/sql/schemachanger/scbuild/testdata/drop_function @@ -0,0 +1,43 @@ +setup +CREATE TABLE t( + a INT PRIMARY KEY, + b INT, + C INT, + INDEX t_idx_b(b), + INDEX t_idx_c(c) +); +CREATE SEQUENCE sq1; +CREATE VIEW v AS SELECT a FROM t; +CREATE TYPE notmyworkday AS ENUM ('Monday', 'Tuesday'); +CREATE FUNCTION f(a notmyworkday) RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ + SELECT a FROM t; + SELECT b FROM t@t_idx_b; + SELECT c FROM t@t_idx_c; + SELECT a FROM v; + SELECT nextval('sq1'); +$$; +---- + +build +DROP FUNCTION f; +---- +- [[Owner:{DescID: 109}, ABSENT], PUBLIC] + {descriptorId: 109, owner: root} +- [[UserPrivileges:{DescID: 109, Name: admin}, ABSENT], PUBLIC] + {descriptorId: 109, privileges: "2", userName: admin} +- [[UserPrivileges:{DescID: 109, Name: root}, ABSENT], PUBLIC] + {descriptorId: 109, privileges: "2", userName: root} +- [[Function:{DescID: 109}, ABSENT], PUBLIC] + {functionId: 109, params: [{class: {class: IN}, name: a, type: {closedTypeIds: [107, 108], type: {family: EnumFamily, oid: 100107, udtMetadata: {arrayTypeOid: 100108}}}}], returnType: {type: {family: IntFamily, oid: 20, width: 64}}} +- [[ObjectParent:{DescID: 109, ReferencedDescID: 101}, ABSENT], PUBLIC] + {objectId: 109, parentSchemaId: 101} +- [[FunctionName:{DescID: 109}, ABSENT], PUBLIC] + {functionId: 109, name: f} +- [[FunctionVolatility:{DescID: 109}, ABSENT], PUBLIC] + {functionId: 109, volatility: {volatility: IMMUTABLE}} +- [[FunctionLeakProof:{DescID: 109}, ABSENT], PUBLIC] + {functionId: 109} +- [[FunctionNullInputBehavior:{DescID: 109}, ABSENT], PUBLIC] + {functionId: 109, nullInputBehavior: {nullInputBehavior: CALLED_ON_NULL_INPUT}} +- [[FunctionBody:{DescID: 109}, ABSENT], PUBLIC] + {body: "SELECT a FROM defaultdb.public.t;\nSELECT b FROM defaultdb.public.t@t_idx_b;\nSELECT c FROM defaultdb.public.t@t_idx_c;\nSELECT a FROM defaultdb.public.v;\nSELECT nextval(105:::REGCLASS);", functionId: 109, lang: {lang: SQL}, usesSequenceIds: [105], usesTables: [{columnIds: [1], tableId: 104}, {columnIds: [2], indexId: 2, tableId: 104}, {columnIds: [3], indexId: 3, tableId: 104}], usesTypeIds: [107, 108], usesViews: [{columnIds: [1], viewId: 106}]} diff --git a/pkg/sql/schemachanger/scdecomp/testdata/function b/pkg/sql/schemachanger/scdecomp/testdata/function index b83bb4d9d958..30581174ff11 100644 --- a/pkg/sql/schemachanger/scdecomp/testdata/function +++ b/pkg/sql/schemachanger/scdecomp/testdata/function @@ -77,12 +77,12 @@ ElementState: Status: PUBLIC - UserPrivileges: descriptorId: 110 - privileges: 2 + privileges: "2" userName: admin Status: PUBLIC - UserPrivileges: descriptorId: 110 - privileges: 2 + privileges: "2" userName: root Status: PUBLIC - ObjectParent: diff --git a/pkg/sql/schemachanger/scdeps/sctestdeps/BUILD.bazel b/pkg/sql/schemachanger/scdeps/sctestdeps/BUILD.bazel index deb9e209c24c..e21ab526efed 100644 --- a/pkg/sql/schemachanger/scdeps/sctestdeps/BUILD.bazel +++ b/pkg/sql/schemachanger/scdeps/sctestdeps/BUILD.bazel @@ -26,6 +26,7 @@ go_library( "//pkg/sql/catalog/dbdesc", "//pkg/sql/catalog/descbuilder", "//pkg/sql/catalog/descpb", + "//pkg/sql/catalog/funcdesc", "//pkg/sql/catalog/nstree", "//pkg/sql/catalog/schemadesc", "//pkg/sql/catalog/tabledesc", diff --git a/pkg/sql/schemachanger/scdeps/sctestdeps/test_deps.go b/pkg/sql/schemachanger/scdeps/sctestdeps/test_deps.go index 6360fb119556..b6b2053c6b3d 100644 --- a/pkg/sql/schemachanger/scdeps/sctestdeps/test_deps.go +++ b/pkg/sql/schemachanger/scdeps/sctestdeps/test_deps.go @@ -28,6 +28,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descbuilder" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/funcdesc" "github.com/cockroachdb/cockroach/pkg/sql/catalog/nstree" "github.com/cockroachdb/cockroach/pkg/sql/catalog/schemadesc" "github.com/cockroachdb/cockroach/pkg/sql/catalog/typedesc" @@ -1180,15 +1181,27 @@ func (s *TestState) AddTableForStatsRefresh(id descpb.ID) { func (s *TestState) ResolveFunction( ctx context.Context, name *tree.UnresolvedName, path tree.SearchPath, ) (*tree.ResolvedFunctionDefinition, error) { - // TODO(chengxiong): add UDF support for test. - fn, err := name.ToFunctionName() + fnName, err := name.ToFunctionName() if err != nil { return nil, err } - fd, err := tree.GetBuiltinFuncDefinitionOrFail(fn, path) + fd, err := tree.GetBuiltinFuncDefinition(fnName, path) if err != nil { return nil, err } + if fd != nil { + return fd, nil + } + + _, sc := s.mayResolvePrefix(fnName.ObjectNamePrefix) + scDesc, err := catalog.AsSchemaDescriptor(sc) + if err != nil { + return nil, err + } + fd, found := scDesc.GetResolvedFuncDefinition(fnName.Object()) + if !found { + return nil, errors.Newf("function %s not found", fnName.String()) + } return fd, nil } @@ -1196,18 +1209,34 @@ func (s *TestState) ResolveFunction( func (s *TestState) ResolveFunctionByOID( ctx context.Context, oid oid.Oid, ) (string, *tree.Overload, error) { - // TODO(chengxiong): add UDF support for test. - name, ok := tree.OidToBuiltinName[oid] - if !ok { + if !funcdesc.IsOIDUserDefinedFunc(oid) { + name, ok := tree.OidToBuiltinName[oid] + if !ok { + return "", nil, errors.Newf("function %d not found", oid) + } + funcDef := tree.FunDefs[name] + for _, o := range funcDef.Definition { + if o.Oid == oid { + return funcDef.Name, o, nil + } + } return "", nil, errors.Newf("function %d not found", oid) } - funcDef := tree.FunDefs[name] - for _, o := range funcDef.Definition { - if o.Oid == oid { - return funcDef.Name, o, nil - } + + fnID := funcdesc.UserDefinedFunctionOIDToID(oid) + desc, err := s.mustReadImmutableDescriptor(fnID) + if err != nil { + return "", nil, err + } + fnDesc, err := catalog.AsFunctionDescriptor(desc) + if err != nil { + return "", nil, err + } + ol, err := fnDesc.ToOverload() + if err != nil { + return "", nil, err } - return "", nil, errors.Newf("function %d not found", oid) + return fnDesc.GetName(), ol, nil } // ZoneConfigGetter implements scexec.Dependencies. diff --git a/pkg/sql/schemachanger/scexec/scmutationexec/references.go b/pkg/sql/schemachanger/scexec/scmutationexec/references.go index 20ef3048a585..d2a73949134d 100644 --- a/pkg/sql/schemachanger/scexec/scmutationexec/references.go +++ b/pkg/sql/schemachanger/scexec/scmutationexec/references.go @@ -294,11 +294,11 @@ func updateBackReferencesInSequences( return nil } -func (m *immediateVisitor) RemoveBackReferencesInRelations( +func (i *immediateVisitor) RemoveBackReferencesInRelations( ctx context.Context, op scop.RemoveBackReferencesInRelations, ) error { for _, relationID := range op.RelationIDs { - if err := removeViewBackReferencesInRelation(ctx, m, relationID, op.BackReferencedID); err != nil { + if err := removeViewBackReferencesInRelation(ctx, i, relationID, op.BackReferencedID); err != nil { return err } } @@ -322,3 +322,21 @@ func removeViewBackReferencesInRelation( tbl.DependedOnBy = newBackRefs return nil } + +func (i *immediateVisitor) RemoveObjectParent( + ctx context.Context, op scop.RemoveObjectParent, +) error { + obj, err := i.checkOutDescriptor(ctx, op.ObjectID) + if err != nil { + return err + } + switch obj.DescriptorType() { + case catalog.Function: + sc, err := i.checkOutSchema(ctx, op.ParentSchemaID) + if err != nil { + return err + } + sc.RemoveFunction(obj.GetName(), obj.GetID()) + } + return nil +} diff --git a/pkg/sql/schemachanger/scop/immediate_mutation.go b/pkg/sql/schemachanger/scop/immediate_mutation.go index 5106e9d01156..e8d546cad3a8 100644 --- a/pkg/sql/schemachanger/scop/immediate_mutation.go +++ b/pkg/sql/schemachanger/scop/immediate_mutation.go @@ -625,3 +625,9 @@ type RemoveColumnFromIndex struct { Ordinal uint32 InvertedKind catpb.InvertedIndexColumnKind } + +type RemoveObjectParent struct { + immediateMutationOp + ObjectID descpb.ID + ParentSchemaID descpb.ID +} diff --git a/pkg/sql/schemachanger/scop/immediate_mutation_visitor_generated.go b/pkg/sql/schemachanger/scop/immediate_mutation_visitor_generated.go index 9a373efd7d63..4b0cd4af7748 100644 --- a/pkg/sql/schemachanger/scop/immediate_mutation_visitor_generated.go +++ b/pkg/sql/schemachanger/scop/immediate_mutation_visitor_generated.go @@ -97,6 +97,7 @@ type ImmediateMutationVisitor interface { RemoveConstraintComment(context.Context, RemoveConstraintComment) error AddColumnToIndex(context.Context, AddColumnToIndex) error RemoveColumnFromIndex(context.Context, RemoveColumnFromIndex) error + RemoveObjectParent(context.Context, RemoveObjectParent) error } // Visit is part of the ImmediateMutationOp interface. @@ -473,3 +474,8 @@ func (op AddColumnToIndex) Visit(ctx context.Context, v ImmediateMutationVisitor func (op RemoveColumnFromIndex) Visit(ctx context.Context, v ImmediateMutationVisitor) error { return v.RemoveColumnFromIndex(ctx, op) } + +// Visit is part of the ImmediateMutationOp interface. +func (op RemoveObjectParent) Visit(ctx context.Context, v ImmediateMutationVisitor) error { + return v.RemoveObjectParent(ctx, op) +} diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/BUILD.bazel b/pkg/sql/schemachanger/scplan/internal/opgen/BUILD.bazel index ea41b8443b8a..f6cc7f362a8a 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/BUILD.bazel +++ b/pkg/sql/schemachanger/scplan/internal/opgen/BUILD.bazel @@ -28,6 +28,13 @@ go_library( "opgen_enum_type.go", "opgen_enum_type_value.go", "opgen_foreign_key_constraint.go", + "opgen_function.go", + "opgen_function_body.go", + "opgen_function_leakproof.go", + "opgen_function_name.go", + "opgen_function_null_input.go", + "opgen_function_param_default.go", + "opgen_function_volatility.go", "opgen_index_column.go", "opgen_index_comment.go", "opgen_index_data.go", diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_function.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_function.go new file mode 100644 index 000000000000..19e012198d46 --- /dev/null +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_function.go @@ -0,0 +1,48 @@ +// Copyright 2023 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 opgen + +import ( + "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scop" + "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb" +) + +func init() { + opRegistry.register((*scpb.Function)(nil), + toPublic( + scpb.Status_ABSENT, + equiv(scpb.Status_DROPPED), + to(scpb.Status_PUBLIC, + emit(func(this *scpb.Function) *scop.NotImplemented { + return notImplemented(this) + }), + ), + ), + toAbsent( + scpb.Status_PUBLIC, + to(scpb.Status_DROPPED, + revertible(false), + emit(func(this *scpb.Function) *scop.MarkDescriptorAsDropped { + return &scop.MarkDescriptorAsDropped{ + DescriptorID: this.FunctionID, + } + }), + ), + to(scpb.Status_ABSENT, + emit(func(this *scpb.Function) *scop.DeleteDescriptor { + return &scop.DeleteDescriptor{ + DescriptorID: this.FunctionID, + } + }), + ), + ), + ) +} diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_function_body.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_function_body.go new file mode 100644 index 000000000000..52a726782773 --- /dev/null +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_function_body.go @@ -0,0 +1,60 @@ +// Copyright 2023 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 opgen + +import ( + "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" + "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scop" + "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb" +) + +func init() { + opRegistry.register((*scpb.FunctionBody)(nil), + toPublic( + scpb.Status_ABSENT, + to(scpb.Status_PUBLIC, + emit(func(this *scpb.FunctionBody) *scop.NotImplemented { + return notImplemented(this) + }), + ), + ), + toAbsent( + scpb.Status_PUBLIC, + to(scpb.Status_ABSENT, + emit(func(this *scpb.FunctionBody) *scop.RemoveBackReferenceInTypes { + if len(this.UsesTypeIDs) == 0 { + return nil + } + return &scop.RemoveBackReferenceInTypes{ + BackReferencedDescriptorID: this.FunctionID, + TypeIDs: this.UsesTypeIDs, + } + }), + emit(func(this *scpb.FunctionBody) *scop.RemoveBackReferencesInRelations { + var relationIDs []descpb.ID + for _, ref := range this.UsesTables { + relationIDs = append(relationIDs, ref.TableID) + } + for _, ref := range this.UsesViews { + relationIDs = append(relationIDs, ref.ViewID) + } + relationIDs = append(relationIDs, this.UsesSequenceIDs...) + if len(relationIDs) == 0 { + return nil + } + return &scop.RemoveBackReferencesInRelations{ + BackReferencedID: this.FunctionID, + RelationIDs: relationIDs, + } + })), + ), + ) +} diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_function_leakproof.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_function_leakproof.go new file mode 100644 index 000000000000..5a7e4711c39c --- /dev/null +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_function_leakproof.go @@ -0,0 +1,37 @@ +// Copyright 2023 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 opgen + +import ( + "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scop" + "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb" +) + +func init() { + opRegistry.register((*scpb.FunctionLeakProof)(nil), + toPublic( + scpb.Status_ABSENT, + to(scpb.Status_PUBLIC, + emit(func(this *scpb.FunctionLeakProof) *scop.NotImplemented { + return notImplemented(this) + }), + ), + ), + toAbsent( + scpb.Status_PUBLIC, + to(scpb.Status_ABSENT, + emit(func(this *scpb.FunctionLeakProof) *scop.NotImplemented { + return notImplemented(this) + }), + ), + ), + ) +} diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_function_name.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_function_name.go new file mode 100644 index 000000000000..d392cd954886 --- /dev/null +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_function_name.go @@ -0,0 +1,37 @@ +// Copyright 2023 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 opgen + +import ( + "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scop" + "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb" +) + +func init() { + opRegistry.register((*scpb.FunctionName)(nil), + toPublic( + scpb.Status_ABSENT, + to(scpb.Status_PUBLIC, + emit(func(this *scpb.FunctionName) *scop.NotImplemented { + return notImplemented(this) + }), + ), + ), + toAbsent( + scpb.Status_PUBLIC, + to(scpb.Status_ABSENT, + emit(func(this *scpb.FunctionName) *scop.NotImplemented { + return notImplemented(this) + }), + ), + ), + ) +} diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_function_null_input.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_function_null_input.go new file mode 100644 index 000000000000..a5787b684c62 --- /dev/null +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_function_null_input.go @@ -0,0 +1,37 @@ +// Copyright 2023 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 opgen + +import ( + "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scop" + "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb" +) + +func init() { + opRegistry.register((*scpb.FunctionNullInputBehavior)(nil), + toPublic( + scpb.Status_ABSENT, + to(scpb.Status_PUBLIC, + emit(func(this *scpb.FunctionNullInputBehavior) *scop.NotImplemented { + return notImplemented(this) + }), + ), + ), + toAbsent( + scpb.Status_PUBLIC, + to(scpb.Status_ABSENT, + emit(func(this *scpb.FunctionNullInputBehavior) *scop.NotImplemented { + return notImplemented(this) + }), + ), + ), + ) +} diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_function_param_default.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_function_param_default.go new file mode 100644 index 000000000000..bb467a5e33d7 --- /dev/null +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_function_param_default.go @@ -0,0 +1,39 @@ +// Copyright 2023 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 opgen + +import ( + "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scop" + "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb" +) + +func init() { + opRegistry.register((*scpb.FunctionParamDefaultExpression)(nil), + toPublic( + scpb.Status_ABSENT, + to(scpb.Status_PUBLIC, + // TODO(chengxiong): add operations when default value is supported. + emit(func(this *scpb.FunctionParamDefaultExpression) *scop.NotImplemented { + return notImplemented(this) + }), + ), + ), + toAbsent( + scpb.Status_PUBLIC, + to(scpb.Status_ABSENT, + // TODO(chengxiong): add operations when default value is supported. + emit(func(this *scpb.FunctionParamDefaultExpression) *scop.NotImplemented { + return notImplemented(this) + }), + ), + ), + ) +} diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_function_volatility.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_function_volatility.go new file mode 100644 index 000000000000..f6ed74e89d08 --- /dev/null +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_function_volatility.go @@ -0,0 +1,37 @@ +// Copyright 2023 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 opgen + +import ( + "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scop" + "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb" +) + +func init() { + opRegistry.register((*scpb.FunctionVolatility)(nil), + toPublic( + scpb.Status_ABSENT, + to(scpb.Status_PUBLIC, + emit(func(this *scpb.FunctionVolatility) *scop.NotImplemented { + return notImplemented(this) + }), + ), + ), + toAbsent( + scpb.Status_PUBLIC, + to(scpb.Status_ABSENT, + emit(func(this *scpb.FunctionVolatility) *scop.NotImplemented { + return notImplemented(this) + }), + ), + ), + ) +} diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_object_parent.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_object_parent.go index 781b54d6fc7c..755135e44bd5 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_object_parent.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_object_parent.go @@ -30,6 +30,12 @@ func init() { to(scpb.Status_ABSENT, // TODO(postamar): remove revertibility constraint when possible revertible(false), + emit(func(this *scpb.ObjectParent) *scop.RemoveObjectParent { + return &scop.RemoveObjectParent{ + ObjectID: this.ObjectID, + ParentSchemaID: this.ParentSchemaID, + } + }), ), ), ) diff --git a/pkg/sql/schemachanger/scplan/internal/rules/current/helpers.go b/pkg/sql/schemachanger/scplan/internal/rules/current/helpers.go index 6700a6987fb8..ee22ece9a7f8 100644 --- a/pkg/sql/schemachanger/scplan/internal/rules/current/helpers.go +++ b/pkg/sql/schemachanger/scplan/internal/rules/current/helpers.go @@ -49,7 +49,7 @@ var descriptorIsNotBeingDropped = screl.Schema.DefNotJoin1( func isDescriptor(e scpb.Element) bool { switch e.(type) { case *scpb.Database, *scpb.Schema, *scpb.Table, *scpb.View, *scpb.Sequence, - *scpb.AliasType, *scpb.EnumType, *scpb.CompositeType: + *scpb.AliasType, *scpb.EnumType, *scpb.CompositeType, *scpb.Function: return true } return false @@ -133,6 +133,11 @@ func getExpression(element scpb.Element) (*scpb.Expression, error) { return nil, nil } return &e.Expression, nil + case *scpb.FunctionParamDefaultExpression: + if e == nil { + return nil, nil + } + return &e.Expression, nil } return nil, errors.AssertionFailedf("element %T does not have an embedded scpb.Expression", element) } diff --git a/pkg/sql/schemachanger/scplan/internal/rules/current/op_drop.go b/pkg/sql/schemachanger/scplan/internal/rules/current/op_drop.go index b8a801347cd4..18a1fdb40031 100644 --- a/pkg/sql/schemachanger/scplan/internal/rules/current/op_drop.go +++ b/pkg/sql/schemachanger/scplan/internal/rules/current/op_drop.go @@ -241,6 +241,10 @@ func init() { (*scpb.UserPrivileges)(nil), (*scpb.EnumTypeValue)(nil), (*scpb.TablePartitioning)(nil), + (*scpb.FunctionName)(nil), + (*scpb.FunctionVolatility)(nil), + (*scpb.FunctionLeakProof)(nil), + (*scpb.FunctionNullInputBehavior)(nil), ), JoinOnDescID(desc, dep, descID), diff --git a/pkg/sql/schemachanger/scplan/internal/rules/current/testdata/deprules b/pkg/sql/schemachanger/scplan/internal/rules/current/testdata/deprules index b442398cb6cb..e3dd64a83d15 100644 --- a/pkg/sql/schemachanger/scplan/internal/rules/current/testdata/deprules +++ b/pkg/sql/schemachanger/scplan/internal/rules/current/testdata/deprules @@ -14,7 +14,7 @@ ToPublicOrTransient($target1, $target2): - $target2[TargetStatus] IN [PUBLIC, TRANSIENT_ABSENT] descriptorIsNotBeingDropped-23.1($element): not-join: - - $descriptor[Type] IN ['*scpb.Database', '*scpb.Schema', '*scpb.View', '*scpb.Sequence', '*scpb.Table', '*scpb.EnumType', '*scpb.AliasType', '*scpb.CompositeType'] + - $descriptor[Type] IN ['*scpb.Database', '*scpb.Schema', '*scpb.View', '*scpb.Sequence', '*scpb.Table', '*scpb.EnumType', '*scpb.AliasType', '*scpb.CompositeType', '*scpb.Function'] - joinTarget($descriptor, $descriptor-Target) - joinOnDescID($descriptor, $element, $id) - $descriptor-Target[TargetStatus] = ABSENT @@ -2036,8 +2036,8 @@ deprules kind: SameStagePrecedence to: referencing-via-attr-Node query: - - $referenced-descriptor[Type] IN ['*scpb.Database', '*scpb.Schema', '*scpb.View', '*scpb.Sequence', '*scpb.Table', '*scpb.EnumType', '*scpb.AliasType', '*scpb.CompositeType'] - - $referencing-via-attr[Type] IN ['*scpb.ColumnFamily', '*scpb.TableComment', '*scpb.RowLevelTTL', '*scpb.TableZoneConfig', '*scpb.TablePartitioning', '*scpb.TableLocalityGlobal', '*scpb.TableLocalityPrimaryRegion', '*scpb.TableLocalitySecondaryRegion', '*scpb.TableLocalityRegionalByRow', '*scpb.ColumnName', '*scpb.ColumnType', '*scpb.ColumnDefaultExpression', '*scpb.ColumnOnUpdateExpression', '*scpb.SequenceOwner', '*scpb.ColumnComment', '*scpb.IndexName', '*scpb.IndexPartitioning', '*scpb.SecondaryIndexPartial', '*scpb.IndexComment', '*scpb.IndexColumn', '*scpb.ConstraintWithoutIndexName', '*scpb.ConstraintComment', '*scpb.Namespace', '*scpb.Owner', '*scpb.UserPrivileges', '*scpb.DatabaseRegionConfig', '*scpb.DatabaseRoleSetting', '*scpb.DatabaseComment', '*scpb.SchemaParent', '*scpb.SchemaComment', '*scpb.ObjectParent', '*scpb.EnumTypeValue', '*scpb.CompositeTypeAttrType', '*scpb.CompositeTypeAttrName'] + - $referenced-descriptor[Type] IN ['*scpb.Database', '*scpb.Schema', '*scpb.View', '*scpb.Sequence', '*scpb.Table', '*scpb.EnumType', '*scpb.AliasType', '*scpb.CompositeType', '*scpb.Function'] + - $referencing-via-attr[Type] IN ['*scpb.ColumnFamily', '*scpb.TableComment', '*scpb.RowLevelTTL', '*scpb.TableZoneConfig', '*scpb.TablePartitioning', '*scpb.TableLocalityGlobal', '*scpb.TableLocalityPrimaryRegion', '*scpb.TableLocalitySecondaryRegion', '*scpb.TableLocalityRegionalByRow', '*scpb.ColumnName', '*scpb.ColumnType', '*scpb.ColumnDefaultExpression', '*scpb.ColumnOnUpdateExpression', '*scpb.SequenceOwner', '*scpb.ColumnComment', '*scpb.IndexName', '*scpb.IndexPartitioning', '*scpb.SecondaryIndexPartial', '*scpb.IndexComment', '*scpb.IndexColumn', '*scpb.ConstraintWithoutIndexName', '*scpb.ConstraintComment', '*scpb.Namespace', '*scpb.Owner', '*scpb.UserPrivileges', '*scpb.DatabaseRegionConfig', '*scpb.DatabaseRoleSetting', '*scpb.DatabaseComment', '*scpb.SchemaParent', '*scpb.SchemaComment', '*scpb.ObjectParent', '*scpb.EnumTypeValue', '*scpb.CompositeTypeAttrType', '*scpb.CompositeTypeAttrName', '*scpb.FunctionName', '*scpb.FunctionVolatility', '*scpb.FunctionLeakProof', '*scpb.FunctionNullInputBehavior', '*scpb.FunctionBody', '*scpb.FunctionParamDefaultExpression'] - joinReferencedDescID($referencing-via-attr, $referenced-descriptor, $desc-id) - toAbsent($referenced-descriptor-Target, $referencing-via-attr-Target) - $referenced-descriptor-Node[CurrentStatus] = DROPPED @@ -2052,7 +2052,7 @@ deprules - $referenced-descriptor[Type] = '*scpb.Sequence' - $referenced-descriptor[DescID] = $seqID - $referencing-via-expr[ReferencedSequenceIDs] CONTAINS $seqID - - $referencing-via-expr[Type] IN ['*scpb.ColumnType', '*scpb.ColumnDefaultExpression', '*scpb.ColumnOnUpdateExpression', '*scpb.SecondaryIndexPartial'] + - $referencing-via-expr[Type] IN ['*scpb.ColumnType', '*scpb.ColumnDefaultExpression', '*scpb.ColumnOnUpdateExpression', '*scpb.SecondaryIndexPartial', '*scpb.FunctionParamDefaultExpression'] - toAbsent($referenced-descriptor-Target, $referencing-via-expr-Target) - $referenced-descriptor-Node[CurrentStatus] = DROPPED - $referencing-via-expr-Node[CurrentStatus] = ABSENT @@ -2066,7 +2066,7 @@ deprules - $referenced-descriptor[Type] IN ['*scpb.EnumType', '*scpb.AliasType', '*scpb.CompositeType'] - $referenced-descriptor[DescID] = $fromDescID - $referencing-via-type[ReferencedTypeIDs] CONTAINS $fromDescID - - $referencing-via-type[Type] IN ['*scpb.ColumnType', '*scpb.ColumnDefaultExpression', '*scpb.ColumnOnUpdateExpression', '*scpb.SecondaryIndexPartial'] + - $referencing-via-type[Type] IN ['*scpb.ColumnType', '*scpb.ColumnDefaultExpression', '*scpb.ColumnOnUpdateExpression', '*scpb.SecondaryIndexPartial', '*scpb.FunctionParamDefaultExpression'] - toAbsent($referenced-descriptor-Target, $referencing-via-type-Target) - $referenced-descriptor-Node[CurrentStatus] = DROPPED - $referencing-via-type-Node[CurrentStatus] = ABSENT @@ -2077,8 +2077,8 @@ deprules kind: Precedence to: dependent-Node query: - - $descriptor[Type] IN ['*scpb.Database', '*scpb.Schema', '*scpb.View', '*scpb.Sequence', '*scpb.Table', '*scpb.EnumType', '*scpb.AliasType', '*scpb.CompositeType'] - - $dependent[Type] IN ['*scpb.ColumnFamily', '*scpb.TableComment', '*scpb.RowLevelTTL', '*scpb.TableZoneConfig', '*scpb.TablePartitioning', '*scpb.TableLocalityGlobal', '*scpb.TableLocalityPrimaryRegion', '*scpb.TableLocalitySecondaryRegion', '*scpb.TableLocalityRegionalByRow', '*scpb.ColumnName', '*scpb.ColumnType', '*scpb.ColumnDefaultExpression', '*scpb.ColumnOnUpdateExpression', '*scpb.SequenceOwner', '*scpb.ColumnComment', '*scpb.IndexName', '*scpb.IndexPartitioning', '*scpb.SecondaryIndexPartial', '*scpb.IndexComment', '*scpb.IndexColumn', '*scpb.ConstraintWithoutIndexName', '*scpb.ConstraintComment', '*scpb.Namespace', '*scpb.Owner', '*scpb.UserPrivileges', '*scpb.DatabaseRegionConfig', '*scpb.DatabaseRoleSetting', '*scpb.DatabaseComment', '*scpb.SchemaParent', '*scpb.SchemaComment', '*scpb.ObjectParent', '*scpb.EnumTypeValue', '*scpb.CompositeTypeAttrType', '*scpb.CompositeTypeAttrName'] + - $descriptor[Type] IN ['*scpb.Database', '*scpb.Schema', '*scpb.View', '*scpb.Sequence', '*scpb.Table', '*scpb.EnumType', '*scpb.AliasType', '*scpb.CompositeType', '*scpb.Function'] + - $dependent[Type] IN ['*scpb.ColumnFamily', '*scpb.TableComment', '*scpb.RowLevelTTL', '*scpb.TableZoneConfig', '*scpb.TablePartitioning', '*scpb.TableLocalityGlobal', '*scpb.TableLocalityPrimaryRegion', '*scpb.TableLocalitySecondaryRegion', '*scpb.TableLocalityRegionalByRow', '*scpb.ColumnName', '*scpb.ColumnType', '*scpb.ColumnDefaultExpression', '*scpb.ColumnOnUpdateExpression', '*scpb.SequenceOwner', '*scpb.ColumnComment', '*scpb.IndexName', '*scpb.IndexPartitioning', '*scpb.SecondaryIndexPartial', '*scpb.IndexComment', '*scpb.IndexColumn', '*scpb.ConstraintWithoutIndexName', '*scpb.ConstraintComment', '*scpb.Namespace', '*scpb.Owner', '*scpb.UserPrivileges', '*scpb.DatabaseRegionConfig', '*scpb.DatabaseRoleSetting', '*scpb.DatabaseComment', '*scpb.SchemaParent', '*scpb.SchemaComment', '*scpb.ObjectParent', '*scpb.EnumTypeValue', '*scpb.CompositeTypeAttrType', '*scpb.CompositeTypeAttrName', '*scpb.FunctionName', '*scpb.FunctionVolatility', '*scpb.FunctionLeakProof', '*scpb.FunctionNullInputBehavior', '*scpb.FunctionBody', '*scpb.FunctionParamDefaultExpression'] - joinOnDescID($descriptor, $dependent, $desc-id) - toAbsent($descriptor-Target, $dependent-Target) - $descriptor-Node[CurrentStatus] = DROPPED @@ -2090,7 +2090,7 @@ deprules kind: PreviousStagePrecedence to: absent-Node query: - - $dropped[Type] IN ['*scpb.Database', '*scpb.Schema', '*scpb.View', '*scpb.Sequence', '*scpb.Table', '*scpb.EnumType', '*scpb.AliasType', '*scpb.CompositeType'] + - $dropped[Type] IN ['*scpb.Database', '*scpb.Schema', '*scpb.View', '*scpb.Sequence', '*scpb.Table', '*scpb.EnumType', '*scpb.AliasType', '*scpb.CompositeType', '*scpb.Function'] - $dropped[DescID] = $_ - $dropped[Self] = $absent - toAbsent($dropped-Target, $absent-Target) @@ -2103,7 +2103,7 @@ deprules kind: SameStagePrecedence to: data-Node query: - - $database[Type] IN ['*scpb.Database', '*scpb.Schema', '*scpb.View', '*scpb.Sequence', '*scpb.Table', '*scpb.EnumType', '*scpb.AliasType', '*scpb.CompositeType'] + - $database[Type] IN ['*scpb.Database', '*scpb.Schema', '*scpb.View', '*scpb.Sequence', '*scpb.Table', '*scpb.EnumType', '*scpb.AliasType', '*scpb.CompositeType', '*scpb.Function'] - $data[Type] = '*scpb.DatabaseData' - joinOnDescID($database, $data, $db-id) - toAbsent($database-Target, $data-Target) @@ -2853,7 +2853,7 @@ deprules kind: SameStagePrecedence to: data-Node query: - - $table[Type] IN ['*scpb.Database', '*scpb.Schema', '*scpb.View', '*scpb.Sequence', '*scpb.Table', '*scpb.EnumType', '*scpb.AliasType', '*scpb.CompositeType'] + - $table[Type] IN ['*scpb.Database', '*scpb.Schema', '*scpb.View', '*scpb.Sequence', '*scpb.Table', '*scpb.EnumType', '*scpb.AliasType', '*scpb.CompositeType', '*scpb.Function'] - $data[Type] = '*scpb.TableData' - joinOnDescID($table, $data, $table-id) - toAbsent($table-Target, $data-Target) diff --git a/pkg/sql/schemachanger/scplan/internal/rules/current/testdata/oprules b/pkg/sql/schemachanger/scplan/internal/rules/current/testdata/oprules index 622bd5bcec4e..f56b1fd58e02 100644 --- a/pkg/sql/schemachanger/scplan/internal/rules/current/testdata/oprules +++ b/pkg/sql/schemachanger/scplan/internal/rules/current/testdata/oprules @@ -14,7 +14,7 @@ ToPublicOrTransient($target1, $target2): - $target2[TargetStatus] IN [PUBLIC, TRANSIENT_ABSENT] descriptorIsNotBeingDropped-23.1($element): not-join: - - $descriptor[Type] IN ['*scpb.Database', '*scpb.Schema', '*scpb.View', '*scpb.Sequence', '*scpb.Table', '*scpb.EnumType', '*scpb.AliasType', '*scpb.CompositeType'] + - $descriptor[Type] IN ['*scpb.Database', '*scpb.Schema', '*scpb.View', '*scpb.Sequence', '*scpb.Table', '*scpb.EnumType', '*scpb.AliasType', '*scpb.CompositeType', '*scpb.Function'] - joinTarget($descriptor, $descriptor-Target) - joinOnDescID($descriptor, $element, $id) - $descriptor-Target[TargetStatus] = ABSENT @@ -153,8 +153,8 @@ oprules - name: skip element removal ops on descriptor drop from: dep-Node query: - - $desc[Type] IN ['*scpb.Database', '*scpb.Schema', '*scpb.View', '*scpb.Sequence', '*scpb.Table', '*scpb.EnumType', '*scpb.AliasType', '*scpb.CompositeType'] - - $dep[Type] IN ['*scpb.ColumnFamily', '*scpb.Owner', '*scpb.UserPrivileges', '*scpb.EnumTypeValue', '*scpb.TablePartitioning'] + - $desc[Type] IN ['*scpb.Database', '*scpb.Schema', '*scpb.View', '*scpb.Sequence', '*scpb.Table', '*scpb.EnumType', '*scpb.AliasType', '*scpb.CompositeType', '*scpb.Function'] + - $dep[Type] IN ['*scpb.ColumnFamily', '*scpb.Owner', '*scpb.UserPrivileges', '*scpb.EnumTypeValue', '*scpb.TablePartitioning', '*scpb.FunctionName', '*scpb.FunctionVolatility', '*scpb.FunctionLeakProof', '*scpb.FunctionNullInputBehavior'] - joinOnDescID($desc, $dep, $desc-id) - joinTarget($desc, $desc-Target) - $desc-Target[TargetStatus] = ABSENT diff --git a/pkg/sql/schemachanger/scplan/testdata/alter_table_drop_column b/pkg/sql/schemachanger/scplan/testdata/alter_table_drop_column index 3f5b181cb46d..c17f0688a043 100644 --- a/pkg/sql/schemachanger/scplan/testdata/alter_table_drop_column +++ b/pkg/sql/schemachanger/scplan/testdata/alter_table_drop_column @@ -14,7 +14,7 @@ SET sql_safe_updates = false; ops ALTER TABLE defaultdb.foo DROP COLUMN v1 CASCADE; ---- -StatementPhase stage 1 of 1 with 15 MutationType ops +StatementPhase stage 1 of 1 with 16 MutationType ops transitions: [[Column:{DescID: 107, ColumnID: 2}, ABSENT], PUBLIC] -> WRITE_ONLY [[ColumnName:{DescID: 107, Name: v1, ColumnID: 2}, ABSENT], PUBLIC] -> ABSENT @@ -67,10 +67,13 @@ StatementPhase stage 1 of 1 with 15 MutationType ops BackReferencedDescriptorID: 108 TypeIDs: - 104 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 108 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 108 RelationIDs: - 107 + *scop.RemoveObjectParent + ObjectID: 108 + ParentSchemaID: 101 *scop.RemoveDroppedColumnType ColumnID: 2 TableID: 108 @@ -331,7 +334,7 @@ PostCommitPhase stage 7 of 7 with 1 ValidationType op *scop.ValidateIndex IndexID: 3 TableID: 107 -PostCommitNonRevertiblePhase stage 1 of 3 with 24 MutationType ops +PostCommitNonRevertiblePhase stage 1 of 3 with 25 MutationType ops transitions: [[Column:{DescID: 107, ColumnID: 2}, ABSENT], WRITE_ONLY] -> DELETE_ONLY [[IndexColumn:{DescID: 107, ColumnID: 1, IndexID: 1}, ABSENT], PUBLIC] -> ABSENT @@ -391,6 +394,9 @@ PostCommitNonRevertiblePhase stage 1 of 3 with 24 MutationType ops BackReferencedID: 108 RelationIDs: - 107 + *scop.RemoveObjectParent + ObjectID: 108 + ParentSchemaID: 101 *scop.RemoveDroppedColumnType ColumnID: 2 TableID: 108 @@ -991,7 +997,7 @@ ALTER TABLE defaultdb.foo DROP COLUMN v1 CASCADE; ops ALTER TABLE defaultdb.foo DROP COLUMN v2 CASCADE; ---- -StatementPhase stage 1 of 1 with 15 MutationType ops +StatementPhase stage 1 of 1 with 16 MutationType ops transitions: [[Column:{DescID: 107, ColumnID: 3}, ABSENT], PUBLIC] -> WRITE_ONLY [[ColumnName:{DescID: 107, Name: v2, ColumnID: 3}, ABSENT], PUBLIC] -> ABSENT @@ -1037,10 +1043,13 @@ StatementPhase stage 1 of 1 with 15 MutationType ops BackReferencedDescriptorID: 108 TypeIDs: - 104 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 108 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 108 RelationIDs: - 107 + *scop.RemoveObjectParent + ObjectID: 108 + ParentSchemaID: 101 *scop.RemoveDroppedColumnType ColumnID: 2 TableID: 108 @@ -1320,7 +1329,7 @@ PostCommitPhase stage 7 of 7 with 1 ValidationType op *scop.ValidateIndex IndexID: 3 TableID: 107 -PostCommitNonRevertiblePhase stage 1 of 3 with 25 MutationType ops +PostCommitNonRevertiblePhase stage 1 of 3 with 26 MutationType ops transitions: [[Column:{DescID: 107, ColumnID: 3}, ABSENT], WRITE_ONLY] -> DELETE_ONLY [[IndexColumn:{DescID: 107, ColumnID: 1, IndexID: 1}, ABSENT], PUBLIC] -> ABSENT @@ -1380,6 +1389,9 @@ PostCommitNonRevertiblePhase stage 1 of 3 with 25 MutationType ops BackReferencedID: 108 RelationIDs: - 107 + *scop.RemoveObjectParent + ObjectID: 108 + ParentSchemaID: 101 *scop.RemoveDroppedColumnType ColumnID: 2 TableID: 108 diff --git a/pkg/sql/schemachanger/scplan/testdata/drop_database b/pkg/sql/schemachanger/scplan/testdata/drop_database index 09076481b486..99615d0b3bee 100644 --- a/pkg/sql/schemachanger/scplan/testdata/drop_database +++ b/pkg/sql/schemachanger/scplan/testdata/drop_database @@ -19,7 +19,7 @@ COMMENT ON TABLE db1.sc1.t1 IS 't1 is good'; ops DROP DATABASE db1 CASCADE ---- -StatementPhase stage 1 of 1 with 43 MutationType ops +StatementPhase stage 1 of 1 with 54 MutationType ops transitions: [[Namespace:{DescID: 104, Name: db1, ReferencedDescID: 0}, ABSENT], PUBLIC] -> ABSENT [[Owner:{DescID: 104}, ABSENT], PUBLIC] -> ABSENT @@ -238,8 +238,14 @@ StatementPhase stage 1 of 1 with 43 MutationType ops SchemaID: 106 *scop.MarkDescriptorAsDropped DescriptorID: 107 + *scop.RemoveObjectParent + ObjectID: 107 + ParentSchemaID: 105 *scop.MarkDescriptorAsDropped DescriptorID: 110 + *scop.RemoveObjectParent + ObjectID: 110 + ParentSchemaID: 105 *scop.RemoveColumnDefaultExpression ColumnID: 3 TableID: 110 @@ -250,8 +256,14 @@ StatementPhase stage 1 of 1 with 43 MutationType ops - 107 *scop.MarkDescriptorAsDropped DescriptorID: 108 + *scop.RemoveObjectParent + ObjectID: 108 + ParentSchemaID: 106 *scop.MarkDescriptorAsDropped DescriptorID: 109 + *scop.RemoveObjectParent + ObjectID: 109 + ParentSchemaID: 106 *scop.RemoveTableComment TableID: 109 *scop.RemoveColumnDefaultExpression @@ -264,33 +276,51 @@ StatementPhase stage 1 of 1 with 43 MutationType ops - 108 *scop.MarkDescriptorAsDropped DescriptorID: 111 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 111 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 111 RelationIDs: - 109 + *scop.RemoveObjectParent + ObjectID: 111 + ParentSchemaID: 106 *scop.MarkDescriptorAsDropped DescriptorID: 112 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 112 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 112 RelationIDs: - 111 + *scop.RemoveObjectParent + ObjectID: 112 + ParentSchemaID: 106 *scop.MarkDescriptorAsDropped DescriptorID: 113 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 113 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 113 RelationIDs: - 111 - 112 + *scop.RemoveObjectParent + ObjectID: 113 + ParentSchemaID: 106 *scop.MarkDescriptorAsDropped DescriptorID: 114 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 114 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 114 RelationIDs: - 112 + *scop.RemoveObjectParent + ObjectID: 114 + ParentSchemaID: 106 *scop.MarkDescriptorAsDropped DescriptorID: 115 + *scop.RemoveObjectParent + ObjectID: 115 + ParentSchemaID: 106 *scop.MarkDescriptorAsDropped DescriptorID: 116 + *scop.RemoveObjectParent + ObjectID: 116 + ParentSchemaID: 106 *scop.MarkDescriptorAsDropped DescriptorID: 117 *scop.RemoveBackReferenceInTypes @@ -298,10 +328,13 @@ StatementPhase stage 1 of 1 with 43 MutationType ops TypeIDs: - 115 - 116 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 117 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 117 RelationIDs: - 114 + *scop.RemoveObjectParent + ObjectID: 117 + ParentSchemaID: 106 *scop.DrainDescriptorName Namespace: DescriptorID: 104 @@ -583,7 +616,7 @@ PreCommitPhase stage 1 of 2 with 1 MutationType op ops: *scop.UndoAllInTxnImmediateMutationOpSideEffects {} -PreCommitPhase stage 2 of 2 with 89 MutationType ops +PreCommitPhase stage 2 of 2 with 100 MutationType ops transitions: [[Namespace:{DescID: 104, Name: db1, ReferencedDescID: 0}, ABSENT], PUBLIC] -> ABSENT [[Owner:{DescID: 104}, ABSENT], PUBLIC] -> ABSENT @@ -804,8 +837,14 @@ PreCommitPhase stage 2 of 2 with 89 MutationType ops SchemaID: 106 *scop.MarkDescriptorAsDropped DescriptorID: 107 + *scop.RemoveObjectParent + ObjectID: 107 + ParentSchemaID: 105 *scop.MarkDescriptorAsDropped DescriptorID: 110 + *scop.RemoveObjectParent + ObjectID: 110 + ParentSchemaID: 105 *scop.RemoveColumnDefaultExpression ColumnID: 3 TableID: 110 @@ -816,8 +855,14 @@ PreCommitPhase stage 2 of 2 with 89 MutationType ops - 107 *scop.MarkDescriptorAsDropped DescriptorID: 108 + *scop.RemoveObjectParent + ObjectID: 108 + ParentSchemaID: 106 *scop.MarkDescriptorAsDropped DescriptorID: 109 + *scop.RemoveObjectParent + ObjectID: 109 + ParentSchemaID: 106 *scop.RemoveTableComment TableID: 109 *scop.RemoveColumnDefaultExpression @@ -834,12 +879,18 @@ PreCommitPhase stage 2 of 2 with 89 MutationType ops BackReferencedID: 111 RelationIDs: - 109 + *scop.RemoveObjectParent + ObjectID: 111 + ParentSchemaID: 106 *scop.MarkDescriptorAsDropped DescriptorID: 112 *scop.RemoveBackReferencesInRelations BackReferencedID: 112 RelationIDs: - 111 + *scop.RemoveObjectParent + ObjectID: 112 + ParentSchemaID: 106 *scop.MarkDescriptorAsDropped DescriptorID: 113 *scop.RemoveBackReferencesInRelations @@ -847,16 +898,28 @@ PreCommitPhase stage 2 of 2 with 89 MutationType ops RelationIDs: - 111 - 112 + *scop.RemoveObjectParent + ObjectID: 113 + ParentSchemaID: 106 *scop.MarkDescriptorAsDropped DescriptorID: 114 *scop.RemoveBackReferencesInRelations BackReferencedID: 114 RelationIDs: - 112 + *scop.RemoveObjectParent + ObjectID: 114 + ParentSchemaID: 106 *scop.MarkDescriptorAsDropped DescriptorID: 115 + *scop.RemoveObjectParent + ObjectID: 115 + ParentSchemaID: 106 *scop.MarkDescriptorAsDropped DescriptorID: 116 + *scop.RemoveObjectParent + ObjectID: 116 + ParentSchemaID: 106 *scop.MarkDescriptorAsDropped DescriptorID: 117 *scop.RemoveBackReferenceInTypes @@ -868,6 +931,9 @@ PreCommitPhase stage 2 of 2 with 89 MutationType ops BackReferencedID: 117 RelationIDs: - 114 + *scop.RemoveObjectParent + ObjectID: 117 + ParentSchemaID: 106 *scop.DrainDescriptorName Namespace: DescriptorID: 104 diff --git a/pkg/sql/schemachanger/scplan/testdata/drop_function b/pkg/sql/schemachanger/scplan/testdata/drop_function new file mode 100644 index 000000000000..7855e05fba45 --- /dev/null +++ b/pkg/sql/schemachanger/scplan/testdata/drop_function @@ -0,0 +1,169 @@ +setup +CREATE TABLE t( + a INT PRIMARY KEY, + b INT, + C INT, + INDEX t_idx_b(b), + INDEX t_idx_c(c) +); +CREATE SEQUENCE sq1; +CREATE VIEW v AS SELECT a FROM t; +CREATE TYPE notmyworkday AS ENUM ('Monday', 'Tuesday'); +CREATE FUNCTION f(a notmyworkday) RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ + SELECT a FROM t; + SELECT b FROM t@t_idx_b; + SELECT c FROM t@t_idx_c; + SELECT a FROM v; + SELECT nextval('sq1'); +$$; +---- + +ops +DROP FUNCTION f; +---- +StatementPhase stage 1 of 1 with 4 MutationType ops + transitions: + [[Owner:{DescID: 109}, ABSENT], PUBLIC] -> ABSENT + [[UserPrivileges:{DescID: 109, Name: admin}, ABSENT], PUBLIC] -> ABSENT + [[UserPrivileges:{DescID: 109, Name: root}, ABSENT], PUBLIC] -> ABSENT + [[Function:{DescID: 109}, ABSENT], PUBLIC] -> DROPPED + [[ObjectParent:{DescID: 109, ReferencedDescID: 101}, ABSENT], PUBLIC] -> ABSENT + [[FunctionName:{DescID: 109}, ABSENT], PUBLIC] -> ABSENT + [[FunctionVolatility:{DescID: 109}, ABSENT], PUBLIC] -> ABSENT + [[FunctionLeakProof:{DescID: 109}, ABSENT], PUBLIC] -> ABSENT + [[FunctionNullInputBehavior:{DescID: 109}, ABSENT], PUBLIC] -> ABSENT + [[FunctionBody:{DescID: 109}, ABSENT], PUBLIC] -> ABSENT + ops: + *scop.MarkDescriptorAsDropped + DescriptorID: 109 + *scop.RemoveObjectParent + ObjectID: 109 + ParentSchemaID: 101 + *scop.RemoveBackReferenceInTypes + BackReferencedDescriptorID: 109 + TypeIDs: + - 107 + - 108 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 109 + RelationIDs: + - 104 + - 104 + - 104 + - 106 + - 105 +PreCommitPhase stage 1 of 2 with 1 MutationType op + transitions: + [[Owner:{DescID: 109}, ABSENT], ABSENT] -> PUBLIC + [[UserPrivileges:{DescID: 109, Name: admin}, ABSENT], ABSENT] -> PUBLIC + [[UserPrivileges:{DescID: 109, Name: root}, ABSENT], ABSENT] -> PUBLIC + [[Function:{DescID: 109}, ABSENT], DROPPED] -> PUBLIC + [[ObjectParent:{DescID: 109, ReferencedDescID: 101}, ABSENT], ABSENT] -> PUBLIC + [[FunctionName:{DescID: 109}, ABSENT], ABSENT] -> PUBLIC + [[FunctionVolatility:{DescID: 109}, ABSENT], ABSENT] -> PUBLIC + [[FunctionLeakProof:{DescID: 109}, ABSENT], ABSENT] -> PUBLIC + [[FunctionNullInputBehavior:{DescID: 109}, ABSENT], ABSENT] -> PUBLIC + [[FunctionBody:{DescID: 109}, ABSENT], ABSENT] -> PUBLIC + ops: + *scop.UndoAllInTxnImmediateMutationOpSideEffects + {} +PreCommitPhase stage 2 of 2 with 11 MutationType ops + transitions: + [[Owner:{DescID: 109}, ABSENT], PUBLIC] -> ABSENT + [[UserPrivileges:{DescID: 109, Name: admin}, ABSENT], PUBLIC] -> ABSENT + [[UserPrivileges:{DescID: 109, Name: root}, ABSENT], PUBLIC] -> ABSENT + [[Function:{DescID: 109}, ABSENT], PUBLIC] -> DROPPED + [[ObjectParent:{DescID: 109, ReferencedDescID: 101}, ABSENT], PUBLIC] -> ABSENT + [[FunctionName:{DescID: 109}, ABSENT], PUBLIC] -> ABSENT + [[FunctionVolatility:{DescID: 109}, ABSENT], PUBLIC] -> ABSENT + [[FunctionLeakProof:{DescID: 109}, ABSENT], PUBLIC] -> ABSENT + [[FunctionNullInputBehavior:{DescID: 109}, ABSENT], PUBLIC] -> ABSENT + [[FunctionBody:{DescID: 109}, ABSENT], PUBLIC] -> ABSENT + ops: + *scop.MarkDescriptorAsDropped + DescriptorID: 109 + *scop.RemoveObjectParent + ObjectID: 109 + ParentSchemaID: 101 + *scop.RemoveBackReferenceInTypes + BackReferencedDescriptorID: 109 + TypeIDs: + - 107 + - 108 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 109 + RelationIDs: + - 104 + - 104 + - 104 + - 106 + - 105 + *scop.SetJobStateOnDescriptor + DescriptorID: 104 + Initialize: true + *scop.SetJobStateOnDescriptor + DescriptorID: 105 + Initialize: true + *scop.SetJobStateOnDescriptor + DescriptorID: 106 + Initialize: true + *scop.SetJobStateOnDescriptor + DescriptorID: 107 + Initialize: true + *scop.SetJobStateOnDescriptor + DescriptorID: 108 + Initialize: true + *scop.SetJobStateOnDescriptor + DescriptorID: 109 + Initialize: true + *scop.CreateSchemaChangerJob + Authorization: + UserName: root + DescriptorIDs: + - 104 + - 105 + - 106 + - 107 + - 108 + - 109 + JobID: 1 + NonCancelable: true + RunningStatus: PostCommitNonRevertiblePhase stage 1 of 1 with 1 MutationType op pending + Statements: + - statement: DROP FUNCTION f + redactedstatement: DROP FUNCTION ‹""›.‹""›.‹f› + statementtag: DROP FUNCTION +PostCommitNonRevertiblePhase stage 1 of 1 with 8 MutationType ops + transitions: + [[Function:{DescID: 109}, ABSENT], DROPPED] -> ABSENT + ops: + *scop.DeleteDescriptor + DescriptorID: 109 + *scop.RemoveJobStateFromDescriptor + DescriptorID: 104 + JobID: 1 + *scop.RemoveJobStateFromDescriptor + DescriptorID: 105 + JobID: 1 + *scop.RemoveJobStateFromDescriptor + DescriptorID: 106 + JobID: 1 + *scop.RemoveJobStateFromDescriptor + DescriptorID: 107 + JobID: 1 + *scop.RemoveJobStateFromDescriptor + DescriptorID: 108 + JobID: 1 + *scop.RemoveJobStateFromDescriptor + DescriptorID: 109 + JobID: 1 + *scop.UpdateSchemaChangerJob + DescriptorIDsToRemove: + - 104 + - 105 + - 106 + - 107 + - 108 + - 109 + IsNonCancelable: true + JobID: 1 diff --git a/pkg/sql/schemachanger/scplan/testdata/drop_index b/pkg/sql/schemachanger/scplan/testdata/drop_index index 64a240b31d13..a9c21d1021d9 100644 --- a/pkg/sql/schemachanger/scplan/testdata/drop_index +++ b/pkg/sql/schemachanger/scplan/testdata/drop_index @@ -620,7 +620,7 @@ DROP INDEX idx3 CASCADE ops DROP INDEX idx4 CASCADE ---- -StatementPhase stage 1 of 1 with 4 MutationType ops +StatementPhase stage 1 of 1 with 5 MutationType ops transitions: [[IndexColumn:{DescID: 104, ColumnID: 2, IndexID: 8}, ABSENT], PUBLIC] -> ABSENT [[IndexColumn:{DescID: 104, ColumnID: 3, IndexID: 8}, ABSENT], PUBLIC] -> ABSENT @@ -643,10 +643,13 @@ StatementPhase stage 1 of 1 with 4 MutationType ops ops: *scop.MarkDescriptorAsDropped DescriptorID: 105 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 105 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 105 RelationIDs: - 104 + *scop.RemoveObjectParent + ObjectID: 105 + ParentSchemaID: 101 *scop.MakePublicSecondaryIndexWriteOnly IndexID: 8 TableID: 104 @@ -679,7 +682,7 @@ PreCommitPhase stage 1 of 2 with 1 MutationType op ops: *scop.UndoAllInTxnImmediateMutationOpSideEffects {} -PreCommitPhase stage 2 of 2 with 10 MutationType ops +PreCommitPhase stage 2 of 2 with 11 MutationType ops transitions: [[IndexColumn:{DescID: 104, ColumnID: 2, IndexID: 8}, ABSENT], PUBLIC] -> ABSENT [[IndexColumn:{DescID: 104, ColumnID: 3, IndexID: 8}, ABSENT], PUBLIC] -> ABSENT @@ -706,6 +709,9 @@ PreCommitPhase stage 2 of 2 with 10 MutationType ops BackReferencedID: 105 RelationIDs: - 104 + *scop.RemoveObjectParent + ObjectID: 105 + ParentSchemaID: 101 *scop.MakePublicSecondaryIndexWriteOnly IndexID: 8 TableID: 104 @@ -958,7 +964,7 @@ DROP INDEX idx4 CASCADE ops DROP INDEX v2@idx CASCADE; ---- -StatementPhase stage 1 of 1 with 5 MutationType ops +StatementPhase stage 1 of 1 with 6 MutationType ops transitions: [[IndexColumn:{DescID: 106, ColumnID: 2, IndexID: 2}, ABSENT], PUBLIC] -> ABSENT [[IndexColumn:{DescID: 106, ColumnID: 3, IndexID: 2}, ABSENT], PUBLIC] -> ABSENT @@ -990,10 +996,13 @@ StatementPhase stage 1 of 1 with 5 MutationType ops ops: *scop.MarkDescriptorAsDropped DescriptorID: 107 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 107 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 107 RelationIDs: - 106 + *scop.RemoveObjectParent + ObjectID: 107 + ParentSchemaID: 101 *scop.RemoveColumnDefaultExpression ColumnID: 2 TableID: 107 @@ -1038,7 +1047,7 @@ PreCommitPhase stage 1 of 2 with 1 MutationType op ops: *scop.UndoAllInTxnImmediateMutationOpSideEffects {} -PreCommitPhase stage 2 of 2 with 12 MutationType ops +PreCommitPhase stage 2 of 2 with 13 MutationType ops transitions: [[IndexColumn:{DescID: 106, ColumnID: 2, IndexID: 2}, ABSENT], PUBLIC] -> ABSENT [[IndexColumn:{DescID: 106, ColumnID: 3, IndexID: 2}, ABSENT], PUBLIC] -> ABSENT @@ -1074,6 +1083,9 @@ PreCommitPhase stage 2 of 2 with 12 MutationType ops BackReferencedID: 107 RelationIDs: - 106 + *scop.RemoveObjectParent + ObjectID: 107 + ParentSchemaID: 101 *scop.RemoveColumnDefaultExpression ColumnID: 2 TableID: 107 diff --git a/pkg/sql/schemachanger/scplan/testdata/drop_owned_by b/pkg/sql/schemachanger/scplan/testdata/drop_owned_by index 2ba0eb382b13..1113840fe0aa 100644 --- a/pkg/sql/schemachanger/scplan/testdata/drop_owned_by +++ b/pkg/sql/schemachanger/scplan/testdata/drop_owned_by @@ -17,7 +17,7 @@ CREATE VIEW s.v2 AS (SELECT 'a'::s.typ::string AS k, name FROM s.v1); ops DROP OWNED BY r ---- -StatementPhase stage 1 of 1 with 28 MutationType ops +StatementPhase stage 1 of 1 with 36 MutationType ops transitions: [[UserPrivileges:{DescID: 100, Name: r}, ABSENT], PUBLIC] -> ABSENT [[Namespace:{DescID: 105, Name: s, ReferencedDescID: 100}, ABSENT], PUBLIC] -> ABSENT @@ -158,8 +158,14 @@ StatementPhase stage 1 of 1 with 28 MutationType ops User: r *scop.MarkDescriptorAsDropped DescriptorID: 106 + *scop.RemoveObjectParent + ObjectID: 106 + ParentSchemaID: 101 *scop.MarkDescriptorAsDropped DescriptorID: 109 + *scop.RemoveObjectParent + ObjectID: 109 + ParentSchemaID: 101 *scop.RemoveColumnDefaultExpression ColumnID: 3 TableID: 109 @@ -170,8 +176,14 @@ StatementPhase stage 1 of 1 with 28 MutationType ops - 106 *scop.MarkDescriptorAsDropped DescriptorID: 107 + *scop.RemoveObjectParent + ObjectID: 107 + ParentSchemaID: 105 *scop.MarkDescriptorAsDropped DescriptorID: 108 + *scop.RemoveObjectParent + ObjectID: 108 + ParentSchemaID: 105 *scop.RemoveColumnDefaultExpression ColumnID: 3 TableID: 108 @@ -182,14 +194,23 @@ StatementPhase stage 1 of 1 with 28 MutationType ops - 107 *scop.MarkDescriptorAsDropped DescriptorID: 110 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 110 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 110 RelationIDs: - 108 + *scop.RemoveObjectParent + ObjectID: 110 + ParentSchemaID: 105 *scop.MarkDescriptorAsDropped DescriptorID: 111 + *scop.RemoveObjectParent + ObjectID: 111 + ParentSchemaID: 105 *scop.MarkDescriptorAsDropped DescriptorID: 112 + *scop.RemoveObjectParent + ObjectID: 112 + ParentSchemaID: 105 *scop.MarkDescriptorAsDropped DescriptorID: 113 *scop.RemoveBackReferenceInTypes @@ -197,10 +218,13 @@ StatementPhase stage 1 of 1 with 28 MutationType ops TypeIDs: - 111 - 112 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 113 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 113 RelationIDs: - 110 + *scop.RemoveObjectParent + ObjectID: 113 + ParentSchemaID: 105 *scop.DrainDescriptorName Namespace: DatabaseID: 100 @@ -383,7 +407,7 @@ PreCommitPhase stage 1 of 2 with 1 MutationType op ops: *scop.UndoAllInTxnImmediateMutationOpSideEffects {} -PreCommitPhase stage 2 of 2 with 57 MutationType ops +PreCommitPhase stage 2 of 2 with 65 MutationType ops transitions: [[UserPrivileges:{DescID: 100, Name: r}, ABSENT], PUBLIC] -> ABSENT [[Namespace:{DescID: 105, Name: s, ReferencedDescID: 100}, ABSENT], PUBLIC] -> ABSENT @@ -524,8 +548,14 @@ PreCommitPhase stage 2 of 2 with 57 MutationType ops User: r *scop.MarkDescriptorAsDropped DescriptorID: 106 + *scop.RemoveObjectParent + ObjectID: 106 + ParentSchemaID: 101 *scop.MarkDescriptorAsDropped DescriptorID: 109 + *scop.RemoveObjectParent + ObjectID: 109 + ParentSchemaID: 101 *scop.RemoveColumnDefaultExpression ColumnID: 3 TableID: 109 @@ -536,8 +566,14 @@ PreCommitPhase stage 2 of 2 with 57 MutationType ops - 106 *scop.MarkDescriptorAsDropped DescriptorID: 107 + *scop.RemoveObjectParent + ObjectID: 107 + ParentSchemaID: 105 *scop.MarkDescriptorAsDropped DescriptorID: 108 + *scop.RemoveObjectParent + ObjectID: 108 + ParentSchemaID: 105 *scop.RemoveColumnDefaultExpression ColumnID: 3 TableID: 108 @@ -552,10 +588,19 @@ PreCommitPhase stage 2 of 2 with 57 MutationType ops BackReferencedID: 110 RelationIDs: - 108 + *scop.RemoveObjectParent + ObjectID: 110 + ParentSchemaID: 105 *scop.MarkDescriptorAsDropped DescriptorID: 111 + *scop.RemoveObjectParent + ObjectID: 111 + ParentSchemaID: 105 *scop.MarkDescriptorAsDropped DescriptorID: 112 + *scop.RemoveObjectParent + ObjectID: 112 + ParentSchemaID: 105 *scop.MarkDescriptorAsDropped DescriptorID: 113 *scop.RemoveBackReferenceInTypes @@ -567,6 +612,9 @@ PreCommitPhase stage 2 of 2 with 57 MutationType ops BackReferencedID: 113 RelationIDs: - 110 + *scop.RemoveObjectParent + ObjectID: 113 + ParentSchemaID: 105 *scop.DrainDescriptorName Namespace: DatabaseID: 100 diff --git a/pkg/sql/schemachanger/scplan/testdata/drop_schema b/pkg/sql/schemachanger/scplan/testdata/drop_schema index 77bb08969ee6..21d956e26138 100644 --- a/pkg/sql/schemachanger/scplan/testdata/drop_schema +++ b/pkg/sql/schemachanger/scplan/testdata/drop_schema @@ -1131,7 +1131,7 @@ DROP SCHEMA defaultdb.SC1 CASCADE ops DROP SCHEMA defaultdb.SC1 CASCADE ---- -StatementPhase stage 1 of 1 with 31 MutationType ops +StatementPhase stage 1 of 1 with 40 MutationType ops transitions: [[Namespace:{DescID: 104, Name: sc1, ReferencedDescID: 100}, ABSENT], PUBLIC] -> ABSENT [[Owner:{DescID: 104}, ABSENT], PUBLIC] -> ABSENT @@ -1291,8 +1291,14 @@ StatementPhase stage 1 of 1 with 31 MutationType ops SchemaID: 104 *scop.MarkDescriptorAsDropped DescriptorID: 105 + *scop.RemoveObjectParent + ObjectID: 105 + ParentSchemaID: 104 *scop.MarkDescriptorAsDropped DescriptorID: 106 + *scop.RemoveObjectParent + ObjectID: 106 + ParentSchemaID: 104 *scop.RemoveTableComment TableID: 106 *scop.RemoveColumnDefaultExpression @@ -1305,33 +1311,51 @@ StatementPhase stage 1 of 1 with 31 MutationType ops - 105 *scop.MarkDescriptorAsDropped DescriptorID: 107 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 107 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 107 RelationIDs: - 106 + *scop.RemoveObjectParent + ObjectID: 107 + ParentSchemaID: 104 *scop.MarkDescriptorAsDropped DescriptorID: 108 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 108 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 108 RelationIDs: - 107 + *scop.RemoveObjectParent + ObjectID: 108 + ParentSchemaID: 104 *scop.MarkDescriptorAsDropped DescriptorID: 109 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 109 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 109 RelationIDs: - 107 - 108 + *scop.RemoveObjectParent + ObjectID: 109 + ParentSchemaID: 104 *scop.MarkDescriptorAsDropped DescriptorID: 110 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 110 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 110 RelationIDs: - 108 + *scop.RemoveObjectParent + ObjectID: 110 + ParentSchemaID: 104 *scop.MarkDescriptorAsDropped DescriptorID: 111 + *scop.RemoveObjectParent + ObjectID: 111 + ParentSchemaID: 104 *scop.MarkDescriptorAsDropped DescriptorID: 112 + *scop.RemoveObjectParent + ObjectID: 112 + ParentSchemaID: 104 *scop.MarkDescriptorAsDropped DescriptorID: 113 *scop.RemoveBackReferenceInTypes @@ -1339,10 +1363,13 @@ StatementPhase stage 1 of 1 with 31 MutationType ops TypeIDs: - 111 - 112 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 113 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 113 RelationIDs: - 110 + *scop.RemoveObjectParent + ObjectID: 113 + ParentSchemaID: 104 *scop.DrainDescriptorName Namespace: DatabaseID: 100 @@ -1554,7 +1581,7 @@ PreCommitPhase stage 1 of 2 with 1 MutationType op ops: *scop.UndoAllInTxnImmediateMutationOpSideEffects {} -PreCommitPhase stage 2 of 2 with 68 MutationType ops +PreCommitPhase stage 2 of 2 with 77 MutationType ops transitions: [[Namespace:{DescID: 104, Name: sc1, ReferencedDescID: 100}, ABSENT], PUBLIC] -> ABSENT [[Owner:{DescID: 104}, ABSENT], PUBLIC] -> ABSENT @@ -1714,8 +1741,14 @@ PreCommitPhase stage 2 of 2 with 68 MutationType ops SchemaID: 104 *scop.MarkDescriptorAsDropped DescriptorID: 105 + *scop.RemoveObjectParent + ObjectID: 105 + ParentSchemaID: 104 *scop.MarkDescriptorAsDropped DescriptorID: 106 + *scop.RemoveObjectParent + ObjectID: 106 + ParentSchemaID: 104 *scop.RemoveTableComment TableID: 106 *scop.RemoveColumnDefaultExpression @@ -1732,12 +1765,18 @@ PreCommitPhase stage 2 of 2 with 68 MutationType ops BackReferencedID: 107 RelationIDs: - 106 + *scop.RemoveObjectParent + ObjectID: 107 + ParentSchemaID: 104 *scop.MarkDescriptorAsDropped DescriptorID: 108 *scop.RemoveBackReferencesInRelations BackReferencedID: 108 RelationIDs: - 107 + *scop.RemoveObjectParent + ObjectID: 108 + ParentSchemaID: 104 *scop.MarkDescriptorAsDropped DescriptorID: 109 *scop.RemoveBackReferencesInRelations @@ -1745,16 +1784,28 @@ PreCommitPhase stage 2 of 2 with 68 MutationType ops RelationIDs: - 107 - 108 + *scop.RemoveObjectParent + ObjectID: 109 + ParentSchemaID: 104 *scop.MarkDescriptorAsDropped DescriptorID: 110 *scop.RemoveBackReferencesInRelations BackReferencedID: 110 RelationIDs: - 108 + *scop.RemoveObjectParent + ObjectID: 110 + ParentSchemaID: 104 *scop.MarkDescriptorAsDropped DescriptorID: 111 + *scop.RemoveObjectParent + ObjectID: 111 + ParentSchemaID: 104 *scop.MarkDescriptorAsDropped DescriptorID: 112 + *scop.RemoveObjectParent + ObjectID: 112 + ParentSchemaID: 104 *scop.MarkDescriptorAsDropped DescriptorID: 113 *scop.RemoveBackReferenceInTypes @@ -1766,6 +1817,9 @@ PreCommitPhase stage 2 of 2 with 68 MutationType ops BackReferencedID: 113 RelationIDs: - 110 + *scop.RemoveObjectParent + ObjectID: 113 + ParentSchemaID: 104 *scop.DrainDescriptorName Namespace: DatabaseID: 100 diff --git a/pkg/sql/schemachanger/scplan/testdata/drop_sequence b/pkg/sql/schemachanger/scplan/testdata/drop_sequence index 9814c254e270..b9968a8ff99c 100644 --- a/pkg/sql/schemachanger/scplan/testdata/drop_sequence +++ b/pkg/sql/schemachanger/scplan/testdata/drop_sequence @@ -5,7 +5,7 @@ CREATE SEQUENCE defaultdb.SQ1 ops DROP SEQUENCE defaultdb.SQ1 ---- -StatementPhase stage 1 of 1 with 2 MutationType ops +StatementPhase stage 1 of 1 with 3 MutationType ops transitions: [[Namespace:{DescID: 104, Name: sq1, ReferencedDescID: 100}, ABSENT], PUBLIC] -> ABSENT [[Owner:{DescID: 104}, ABSENT], PUBLIC] -> ABSENT @@ -16,6 +16,9 @@ StatementPhase stage 1 of 1 with 2 MutationType ops ops: *scop.MarkDescriptorAsDropped DescriptorID: 104 + *scop.RemoveObjectParent + ObjectID: 104 + ParentSchemaID: 101 *scop.DrainDescriptorName Namespace: DatabaseID: 100 @@ -33,7 +36,7 @@ PreCommitPhase stage 1 of 2 with 1 MutationType op ops: *scop.UndoAllInTxnImmediateMutationOpSideEffects {} -PreCommitPhase stage 2 of 2 with 4 MutationType ops +PreCommitPhase stage 2 of 2 with 5 MutationType ops transitions: [[Namespace:{DescID: 104, Name: sq1, ReferencedDescID: 100}, ABSENT], PUBLIC] -> ABSENT [[Owner:{DescID: 104}, ABSENT], PUBLIC] -> ABSENT @@ -44,6 +47,9 @@ PreCommitPhase stage 2 of 2 with 4 MutationType ops ops: *scop.MarkDescriptorAsDropped DescriptorID: 104 + *scop.RemoveObjectParent + ObjectID: 104 + ParentSchemaID: 101 *scop.DrainDescriptorName Namespace: DatabaseID: 100 @@ -92,7 +98,7 @@ CREATE TABLE defaultdb.blog_posts2 (id INT8 PRIMARY KEY, val INT8 DEFAULT nextva ops DROP SEQUENCE defaultdb.SQ1 CASCADE ---- -StatementPhase stage 1 of 1 with 6 MutationType ops +StatementPhase stage 1 of 1 with 7 MutationType ops transitions: [[Namespace:{DescID: 104, Name: sq1, ReferencedDescID: 100}, ABSENT], PUBLIC] -> ABSENT [[Owner:{DescID: 104}, ABSENT], PUBLIC] -> ABSENT @@ -105,6 +111,9 @@ StatementPhase stage 1 of 1 with 6 MutationType ops ops: *scop.MarkDescriptorAsDropped DescriptorID: 104 + *scop.RemoveObjectParent + ObjectID: 104 + ParentSchemaID: 101 *scop.RemoveColumnDefaultExpression ColumnID: 2 TableID: 105 @@ -140,7 +149,7 @@ PreCommitPhase stage 1 of 2 with 1 MutationType op ops: *scop.UndoAllInTxnImmediateMutationOpSideEffects {} -PreCommitPhase stage 2 of 2 with 10 MutationType ops +PreCommitPhase stage 2 of 2 with 11 MutationType ops transitions: [[Namespace:{DescID: 104, Name: sq1, ReferencedDescID: 100}, ABSENT], PUBLIC] -> ABSENT [[Owner:{DescID: 104}, ABSENT], PUBLIC] -> ABSENT @@ -153,6 +162,9 @@ PreCommitPhase stage 2 of 2 with 10 MutationType ops ops: *scop.MarkDescriptorAsDropped DescriptorID: 104 + *scop.RemoveObjectParent + ObjectID: 104 + ParentSchemaID: 101 *scop.RemoveColumnDefaultExpression ColumnID: 2 TableID: 105 diff --git a/pkg/sql/schemachanger/scplan/testdata/drop_table b/pkg/sql/schemachanger/scplan/testdata/drop_table index 842ea3011950..4d9fa3d3b8f6 100644 --- a/pkg/sql/schemachanger/scplan/testdata/drop_table +++ b/pkg/sql/schemachanger/scplan/testdata/drop_table @@ -29,7 +29,7 @@ COMMENT ON CONSTRAINT fk_customers ON defaultdb.shipments IS 'customer is not go ops DROP TABLE defaultdb.shipments CASCADE; ---- -StatementPhase stage 1 of 1 with 19 MutationType ops +StatementPhase stage 1 of 1 with 22 MutationType ops transitions: [[Namespace:{DescID: 109, Name: shipments, ReferencedDescID: 100}, ABSENT], PUBLIC] -> ABSENT [[Owner:{DescID: 109}, ABSENT], PUBLIC] -> ABSENT @@ -109,6 +109,9 @@ StatementPhase stage 1 of 1 with 19 MutationType ops ops: *scop.MarkDescriptorAsDropped DescriptorID: 109 + *scop.RemoveObjectParent + ObjectID: 109 + ParentSchemaID: 101 *scop.RemoveTableComment TableID: 109 *scop.RemoveColumnDefaultExpression @@ -142,12 +145,18 @@ StatementPhase stage 1 of 1 with 19 MutationType ops TableID: 109 *scop.MarkDescriptorAsDropped DescriptorID: 110 + *scop.RemoveObjectParent + ObjectID: 110 + ParentSchemaID: 101 *scop.MarkDescriptorAsDropped DescriptorID: 111 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 111 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 111 RelationIDs: - 109 + *scop.RemoveObjectParent + ObjectID: 111 + ParentSchemaID: 101 *scop.DrainDescriptorName Namespace: DatabaseID: 100 @@ -255,7 +264,7 @@ PreCommitPhase stage 1 of 2 with 1 MutationType op ops: *scop.UndoAllInTxnImmediateMutationOpSideEffects {} -PreCommitPhase stage 2 of 2 with 43 MutationType ops +PreCommitPhase stage 2 of 2 with 46 MutationType ops transitions: [[Namespace:{DescID: 109, Name: shipments, ReferencedDescID: 100}, ABSENT], PUBLIC] -> ABSENT [[Owner:{DescID: 109}, ABSENT], PUBLIC] -> ABSENT @@ -336,6 +345,9 @@ PreCommitPhase stage 2 of 2 with 43 MutationType ops ops: *scop.MarkDescriptorAsDropped DescriptorID: 109 + *scop.RemoveObjectParent + ObjectID: 109 + ParentSchemaID: 101 *scop.RemoveTableComment TableID: 109 *scop.RemoveColumnDefaultExpression @@ -369,12 +381,18 @@ PreCommitPhase stage 2 of 2 with 43 MutationType ops TableID: 109 *scop.MarkDescriptorAsDropped DescriptorID: 110 + *scop.RemoveObjectParent + ObjectID: 110 + ParentSchemaID: 101 *scop.MarkDescriptorAsDropped DescriptorID: 111 *scop.RemoveBackReferencesInRelations BackReferencedID: 111 RelationIDs: - 109 + *scop.RemoveObjectParent + ObjectID: 111 + ParentSchemaID: 101 *scop.DrainDescriptorName Namespace: DatabaseID: 100 @@ -1314,7 +1332,7 @@ CREATE TABLE defaultdb.greeter ( ops DROP TABLE defaultdb.greeter ---- -StatementPhase stage 1 of 1 with 9 MutationType ops +StatementPhase stage 1 of 1 with 10 MutationType ops transitions: [[Namespace:{DescID: 114, Name: greeter, ReferencedDescID: 100}, ABSENT], PUBLIC] -> ABSENT [[Owner:{DescID: 114}, ABSENT], PUBLIC] -> ABSENT @@ -1354,6 +1372,9 @@ StatementPhase stage 1 of 1 with 9 MutationType ops ops: *scop.MarkDescriptorAsDropped DescriptorID: 114 + *scop.RemoveObjectParent + ObjectID: 114 + ParentSchemaID: 101 *scop.RemoveColumnDefaultExpression ColumnID: 1 TableID: 114 @@ -1427,7 +1448,7 @@ PreCommitPhase stage 1 of 2 with 1 MutationType op ops: *scop.UndoAllInTxnImmediateMutationOpSideEffects {} -PreCommitPhase stage 2 of 2 with 20 MutationType ops +PreCommitPhase stage 2 of 2 with 21 MutationType ops transitions: [[Namespace:{DescID: 114, Name: greeter, ReferencedDescID: 100}, ABSENT], PUBLIC] -> ABSENT [[Owner:{DescID: 114}, ABSENT], PUBLIC] -> ABSENT @@ -1468,6 +1489,9 @@ PreCommitPhase stage 2 of 2 with 20 MutationType ops ops: *scop.MarkDescriptorAsDropped DescriptorID: 114 + *scop.RemoveObjectParent + ObjectID: 114 + ParentSchemaID: 101 *scop.RemoveColumnDefaultExpression ColumnID: 1 TableID: 114 diff --git a/pkg/sql/schemachanger/scplan/testdata/drop_type b/pkg/sql/schemachanger/scplan/testdata/drop_type index 00df33ab2ca9..f8f530a52d3e 100644 --- a/pkg/sql/schemachanger/scplan/testdata/drop_type +++ b/pkg/sql/schemachanger/scplan/testdata/drop_type @@ -6,7 +6,7 @@ CREATE TYPE defaultdb.ctyp AS (a INT, b INT) ops DROP TYPE defaultdb.typ ---- -StatementPhase stage 1 of 1 with 4 MutationType ops +StatementPhase stage 1 of 1 with 6 MutationType ops transitions: [[Namespace:{DescID: 104, Name: typ, ReferencedDescID: 100}, ABSENT], PUBLIC] -> ABSENT [[Owner:{DescID: 104}, ABSENT], PUBLIC] -> ABSENT @@ -26,8 +26,14 @@ StatementPhase stage 1 of 1 with 4 MutationType ops ops: *scop.MarkDescriptorAsDropped DescriptorID: 104 + *scop.RemoveObjectParent + ObjectID: 104 + ParentSchemaID: 101 *scop.MarkDescriptorAsDropped DescriptorID: 105 + *scop.RemoveObjectParent + ObjectID: 105 + ParentSchemaID: 101 *scop.DrainDescriptorName Namespace: DatabaseID: 100 @@ -60,7 +66,7 @@ PreCommitPhase stage 1 of 2 with 1 MutationType op ops: *scop.UndoAllInTxnImmediateMutationOpSideEffects {} -PreCommitPhase stage 2 of 2 with 7 MutationType ops +PreCommitPhase stage 2 of 2 with 9 MutationType ops transitions: [[Namespace:{DescID: 104, Name: typ, ReferencedDescID: 100}, ABSENT], PUBLIC] -> ABSENT [[Owner:{DescID: 104}, ABSENT], PUBLIC] -> ABSENT @@ -80,8 +86,14 @@ PreCommitPhase stage 2 of 2 with 7 MutationType ops ops: *scop.MarkDescriptorAsDropped DescriptorID: 104 + *scop.RemoveObjectParent + ObjectID: 104 + ParentSchemaID: 101 *scop.MarkDescriptorAsDropped DescriptorID: 105 + *scop.RemoveObjectParent + ObjectID: 105 + ParentSchemaID: 101 *scop.DrainDescriptorName Namespace: DatabaseID: 100 @@ -202,7 +214,7 @@ DROP TYPE defaultdb.typ ops DROP TYPE defaultdb.ctyp ---- -StatementPhase stage 1 of 1 with 6 MutationType ops +StatementPhase stage 1 of 1 with 8 MutationType ops transitions: [[Namespace:{DescID: 106, Name: ctyp, ReferencedDescID: 100}, ABSENT], PUBLIC] -> ABSENT [[Owner:{DescID: 106}, ABSENT], PUBLIC] -> ABSENT @@ -228,8 +240,14 @@ StatementPhase stage 1 of 1 with 6 MutationType ops ElementType: scpb.CompositeTypeAttrName *scop.NotImplemented ElementType: scpb.CompositeTypeAttrName + *scop.RemoveObjectParent + ObjectID: 106 + ParentSchemaID: 101 *scop.MarkDescriptorAsDropped DescriptorID: 107 + *scop.RemoveObjectParent + ObjectID: 107 + ParentSchemaID: 101 *scop.DrainDescriptorName Namespace: DatabaseID: 100 @@ -264,7 +282,7 @@ PreCommitPhase stage 1 of 2 with 1 MutationType op ops: *scop.UndoAllInTxnImmediateMutationOpSideEffects {} -PreCommitPhase stage 2 of 2 with 9 MutationType ops +PreCommitPhase stage 2 of 2 with 11 MutationType ops transitions: [[Namespace:{DescID: 106, Name: ctyp, ReferencedDescID: 100}, ABSENT], PUBLIC] -> ABSENT [[Owner:{DescID: 106}, ABSENT], PUBLIC] -> ABSENT @@ -290,8 +308,14 @@ PreCommitPhase stage 2 of 2 with 9 MutationType ops ElementType: scpb.CompositeTypeAttrName *scop.NotImplemented ElementType: scpb.CompositeTypeAttrName + *scop.RemoveObjectParent + ObjectID: 106 + ParentSchemaID: 101 *scop.MarkDescriptorAsDropped DescriptorID: 107 + *scop.RemoveObjectParent + ObjectID: 107 + ParentSchemaID: 101 *scop.DrainDescriptorName Namespace: DatabaseID: 100 diff --git a/pkg/sql/schemachanger/scplan/testdata/drop_view b/pkg/sql/schemachanger/scplan/testdata/drop_view index 333e35ad5f00..2279637ed272 100644 --- a/pkg/sql/schemachanger/scplan/testdata/drop_view +++ b/pkg/sql/schemachanger/scplan/testdata/drop_view @@ -6,7 +6,7 @@ CREATE VIEW defaultdb.v1 AS (SELECT name FROM defaultdb.t1); ops DROP VIEW defaultdb.v1 ---- -StatementPhase stage 1 of 1 with 3 MutationType ops +StatementPhase stage 1 of 1 with 4 MutationType ops transitions: [[Namespace:{DescID: 105, Name: v1, ReferencedDescID: 100}, ABSENT], PUBLIC] -> ABSENT [[Owner:{DescID: 105}, ABSENT], PUBLIC] -> ABSENT @@ -26,10 +26,13 @@ StatementPhase stage 1 of 1 with 3 MutationType ops ops: *scop.MarkDescriptorAsDropped DescriptorID: 105 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 105 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 105 RelationIDs: - 104 + *scop.RemoveObjectParent + ObjectID: 105 + ParentSchemaID: 101 *scop.DrainDescriptorName Namespace: DatabaseID: 100 @@ -56,7 +59,7 @@ PreCommitPhase stage 1 of 2 with 1 MutationType op ops: *scop.UndoAllInTxnImmediateMutationOpSideEffects {} -PreCommitPhase stage 2 of 2 with 9 MutationType ops +PreCommitPhase stage 2 of 2 with 10 MutationType ops transitions: [[Namespace:{DescID: 105, Name: v1, ReferencedDescID: 100}, ABSENT], PUBLIC] -> ABSENT [[Owner:{DescID: 105}, ABSENT], PUBLIC] -> ABSENT @@ -80,6 +83,9 @@ PreCommitPhase stage 2 of 2 with 9 MutationType ops BackReferencedID: 105 RelationIDs: - 104 + *scop.RemoveObjectParent + ObjectID: 105 + ParentSchemaID: 101 *scop.DrainDescriptorName Namespace: DatabaseID: 100 @@ -256,7 +262,7 @@ CREATE VIEW v5 AS (SELECT 'a'::defaultdb.typ::string AS k, n2, n1 from defaultdb ops DROP VIEW defaultdb.v1 CASCADE ---- -StatementPhase stage 1 of 1 with 16 MutationType ops +StatementPhase stage 1 of 1 with 21 MutationType ops transitions: [[Namespace:{DescID: 105, Name: v1, ReferencedDescID: 100}, ABSENT], PUBLIC] -> ABSENT [[Owner:{DescID: 105}, ABSENT], PUBLIC] -> ABSENT @@ -351,29 +357,41 @@ StatementPhase stage 1 of 1 with 16 MutationType ops ops: *scop.MarkDescriptorAsDropped DescriptorID: 105 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 105 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 105 RelationIDs: - 104 + *scop.RemoveObjectParent + ObjectID: 105 + ParentSchemaID: 101 *scop.MarkDescriptorAsDropped DescriptorID: 106 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 106 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 106 RelationIDs: - 105 + *scop.RemoveObjectParent + ObjectID: 106 + ParentSchemaID: 101 *scop.MarkDescriptorAsDropped DescriptorID: 107 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 107 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 107 RelationIDs: - 105 - 106 + *scop.RemoveObjectParent + ObjectID: 107 + ParentSchemaID: 101 *scop.MarkDescriptorAsDropped DescriptorID: 108 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 108 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 108 RelationIDs: - 106 + *scop.RemoveObjectParent + ObjectID: 108 + ParentSchemaID: 101 *scop.MarkDescriptorAsDropped DescriptorID: 111 *scop.RemoveBackReferenceInTypes @@ -381,10 +399,13 @@ StatementPhase stage 1 of 1 with 16 MutationType ops TypeIDs: - 109 - 110 - *scop.RemoveViewBackReferencesInRelations - BackReferencedViewID: 111 + *scop.RemoveBackReferencesInRelations + BackReferencedID: 111 RelationIDs: - 108 + *scop.RemoveObjectParent + ObjectID: 111 + ParentSchemaID: 101 *scop.DrainDescriptorName Namespace: DatabaseID: 100 @@ -510,7 +531,7 @@ PreCommitPhase stage 1 of 2 with 1 MutationType op ops: *scop.UndoAllInTxnImmediateMutationOpSideEffects {} -PreCommitPhase stage 2 of 2 with 45 MutationType ops +PreCommitPhase stage 2 of 2 with 50 MutationType ops transitions: [[Namespace:{DescID: 105, Name: v1, ReferencedDescID: 100}, ABSENT], PUBLIC] -> ABSENT [[Owner:{DescID: 105}, ABSENT], PUBLIC] -> ABSENT @@ -609,12 +630,18 @@ PreCommitPhase stage 2 of 2 with 45 MutationType ops BackReferencedID: 105 RelationIDs: - 104 + *scop.RemoveObjectParent + ObjectID: 105 + ParentSchemaID: 101 *scop.MarkDescriptorAsDropped DescriptorID: 106 *scop.RemoveBackReferencesInRelations BackReferencedID: 106 RelationIDs: - 105 + *scop.RemoveObjectParent + ObjectID: 106 + ParentSchemaID: 101 *scop.MarkDescriptorAsDropped DescriptorID: 107 *scop.RemoveBackReferencesInRelations @@ -622,12 +649,18 @@ PreCommitPhase stage 2 of 2 with 45 MutationType ops RelationIDs: - 105 - 106 + *scop.RemoveObjectParent + ObjectID: 107 + ParentSchemaID: 101 *scop.MarkDescriptorAsDropped DescriptorID: 108 *scop.RemoveBackReferencesInRelations BackReferencedID: 108 RelationIDs: - 106 + *scop.RemoveObjectParent + ObjectID: 108 + ParentSchemaID: 101 *scop.MarkDescriptorAsDropped DescriptorID: 111 *scop.RemoveBackReferenceInTypes @@ -639,6 +672,9 @@ PreCommitPhase stage 2 of 2 with 45 MutationType ops BackReferencedID: 111 RelationIDs: - 108 + *scop.RemoveObjectParent + ObjectID: 111 + ParentSchemaID: 101 *scop.DrainDescriptorName Namespace: DatabaseID: 100 diff --git a/pkg/sql/schemachanger/screl/attr.go b/pkg/sql/schemachanger/screl/attr.go index 68a50141b17e..1ac8c0bcbd74 100644 --- a/pkg/sql/schemachanger/screl/attr.go +++ b/pkg/sql/schemachanger/screl/attr.go @@ -338,6 +338,27 @@ var elementSchemaOptions = []rel.SchemaOption{ rel.EntityMapping(t((*scpb.TablePartitioning)(nil)), rel.EntityAttr(DescID, "TableID"), ), + rel.EntityMapping(t((*scpb.Function)(nil)), + rel.EntityAttr(DescID, "FunctionID"), + ), + rel.EntityMapping(t((*scpb.FunctionName)(nil)), + rel.EntityAttr(DescID, "FunctionID"), + ), + rel.EntityMapping(t((*scpb.FunctionVolatility)(nil)), + rel.EntityAttr(DescID, "FunctionID"), + ), + rel.EntityMapping(t((*scpb.FunctionLeakProof)(nil)), + rel.EntityAttr(DescID, "FunctionID"), + ), + rel.EntityMapping(t((*scpb.FunctionNullInputBehavior)(nil)), + rel.EntityAttr(DescID, "FunctionID"), + ), + rel.EntityMapping(t((*scpb.FunctionBody)(nil)), + rel.EntityAttr(DescID, "FunctionID"), + ), + rel.EntityMapping(t((*scpb.FunctionParamDefaultExpression)(nil)), + rel.EntityAttr(DescID, "FunctionID"), + ), } // Schema is the schema exported by this package covering the elements of scpb. diff --git a/pkg/sql/schemachanger/screl/scalars.go b/pkg/sql/schemachanger/screl/scalars.go index 241f344ff7d4..e74a44f9d1e2 100644 --- a/pkg/sql/schemachanger/screl/scalars.go +++ b/pkg/sql/schemachanger/screl/scalars.go @@ -120,7 +120,9 @@ func MinVersion(el scpb.Element) clusterversion.Key { return clusterversion.V23_1 case *scpb.IndexColumn, *scpb.EnumTypeValue, *scpb.TableZoneConfig: return clusterversion.V22_2UseDelRangeInGCJob - case *scpb.DatabaseData, *scpb.TableData, *scpb.IndexData, *scpb.TablePartitioning: + case *scpb.DatabaseData, *scpb.TableData, *scpb.IndexData, *scpb.TablePartitioning, + *scpb.Function, *scpb.FunctionName, *scpb.FunctionVolatility, *scpb.FunctionLeakProof, + *scpb.FunctionNullInputBehavior, *scpb.FunctionBody, *scpb.FunctionParamDefaultExpression: return clusterversion.V23_1 default: panic(errors.AssertionFailedf("unknown element %T", el)) diff --git a/pkg/sql/schemachanger/sctest/cumulative.go b/pkg/sql/schemachanger/sctest/cumulative.go index e0c753f39527..4bab53146b19 100644 --- a/pkg/sql/schemachanger/sctest/cumulative.go +++ b/pkg/sql/schemachanger/sctest/cumulative.go @@ -675,6 +675,7 @@ FROM SELECT descriptor_id, create_statement FROM crdb_internal.create_schema_statements UNION ALL SELECT descriptor_id, create_statement FROM crdb_internal.create_statements UNION ALL SELECT descriptor_id, create_statement FROM crdb_internal.create_type_statements + UNION ALL SELECT function_id as descriptor_id, create_statement FROM crdb_internal.create_function_statements ) WHERE descriptor_id IN (SELECT id FROM system.namespace) ORDER BY diff --git a/pkg/sql/schemachanger/sctest_generated_test.go b/pkg/sql/schemachanger/sctest_generated_test.go index 90d219265fc6..e22cc464c4a8 100644 --- a/pkg/sql/schemachanger/sctest_generated_test.go +++ b/pkg/sql/schemachanger/sctest_generated_test.go @@ -445,6 +445,31 @@ func TestRollback_drop_column_with_index(t *testing.T) { defer log.Scope(t).Close(t) sctest.Rollback(t, "pkg/sql/schemachanger/testdata/end_to_end/drop_column_with_index", sctest.SingleNodeCluster) } +func TestEndToEndSideEffects_drop_function(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + sctest.EndToEndSideEffects(t, "pkg/sql/schemachanger/testdata/end_to_end/drop_function", sctest.SingleNodeCluster) +} +func TestExecuteWithDMLInjection_drop_function(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + sctest.ExecuteWithDMLInjection(t, "pkg/sql/schemachanger/testdata/end_to_end/drop_function", sctest.SingleNodeCluster) +} +func TestGenerateSchemaChangeCorpus_drop_function(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + sctest.GenerateSchemaChangeCorpus(t, "pkg/sql/schemachanger/testdata/end_to_end/drop_function", sctest.SingleNodeCluster) +} +func TestPause_drop_function(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + sctest.Pause(t, "pkg/sql/schemachanger/testdata/end_to_end/drop_function", sctest.SingleNodeCluster) +} +func TestRollback_drop_function(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + sctest.Rollback(t, "pkg/sql/schemachanger/testdata/end_to_end/drop_function", sctest.SingleNodeCluster) +} func TestEndToEndSideEffects_drop_index_hash_sharded_index(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) diff --git a/pkg/sql/schemachanger/testdata/end_to_end/drop_function b/pkg/sql/schemachanger/testdata/end_to_end/drop_function new file mode 100644 index 000000000000..9ecf429a857e --- /dev/null +++ b/pkg/sql/schemachanger/testdata/end_to_end/drop_function @@ -0,0 +1,382 @@ +setup +CREATE TABLE t( + a INT PRIMARY KEY, + b INT, + C INT, + INDEX t_idx_b(b), + INDEX t_idx_c(c) +); +CREATE SEQUENCE sq1; +CREATE VIEW v AS SELECT a FROM t; +CREATE TYPE notmyworkday AS ENUM ('Monday', 'Tuesday'); +CREATE FUNCTION f(a notmyworkday) RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ + SELECT a FROM t; + SELECT b FROM t@t_idx_b; + SELECT c FROM t@t_idx_c; + SELECT a FROM v; + SELECT nextval('sq1'); +$$; +CREATE TABLE t2(a notmyworkday); +---- +... ++object {100 101 t} -> 104 ++object {100 101 sq1} -> 105 ++object {100 101 v} -> 106 ++object {100 101 notmyworkday} -> 107 ++object {100 101 _notmyworkday} -> 108 ++object {100 101 t2} -> 110 + +test +DROP FUNCTION f; +---- +begin transaction #1 +# begin StatementPhase +checking for feature: DROP FUNCTION +increment telemetry for sql.schema.drop_function +## StatementPhase stage 1 of 1 with 4 MutationType ops +upsert descriptor #101 + schema: + - functions: + - f: + - overloads: + - - argTypes: + - - family: EnumFamily + - oid: 100107 + - udtMetadata: + - arrayTypeOid: 100108 + - id: 109 + - returnType: + - family: IntFamily + - oid: 20 + - width: 64 + id: 101 + modificationTime: {} + ... + withGrantOption: "2" + version: 2 + - version: "2" + + version: "3" +upsert descriptor #104 + ... + - 1 + id: 106 + - - columnIds: + - - 1 + - id: 109 + - - columnIds: + - - 2 + - id: 109 + - indexId: 2 + - - columnIds: + - - 3 + - id: 109 + - indexId: 3 + families: + - columnIds: + ... + time: {} + unexposedParentSchemaId: 101 + - version: "3" + + version: "4" +upsert descriptor #105 + ... + createAsOfTime: + wallTime: "1640995200000000000" + - dependedOnBy: + - - byId: true + - id: 109 + families: + - columnIds: + ... + start: "1" + unexposedParentSchemaId: 101 + - version: "2" + + version: "3" +upsert descriptor #106 + ... + createAsOfTime: + wallTime: "1640995200000000000" + - dependedOnBy: + - - columnIds: + - - 1 + - id: 109 + dependsOn: + - 104 + ... + time: {} + unexposedParentSchemaId: 101 + - version: "2" + + version: "3" + viewQuery: SELECT a FROM defaultdb.public.t +upsert descriptor #107 + ... + version: 2 + referencingDescriptorIds: + - - 109 + - 110 + - version: "3" + + version: "4" +upsert descriptor #108 + ... + version: 2 + referencingDescriptorIds: + - - 109 + - 110 + - version: "3" + + version: "4" +upsert descriptor #109 + ... + oid: 20 + width: 64 + - version: "1" + + state: DROP + + version: "2" + volatility: IMMUTABLE +# end StatementPhase +# begin PreCommitPhase +## PreCommitPhase stage 1 of 2 with 1 MutationType op +undo all catalog changes within txn #1 +persist all catalog changes to storage +## PreCommitPhase stage 2 of 2 with 11 MutationType ops +upsert descriptor #101 + schema: + - functions: + - f: + - overloads: + - - argTypes: + - - family: EnumFamily + - oid: 100107 + - udtMetadata: + - arrayTypeOid: 100108 + - id: 109 + - returnType: + - family: IntFamily + - oid: 20 + - width: 64 + id: 101 + modificationTime: {} + ... + withGrantOption: "2" + version: 2 + - version: "2" + + version: "3" +upsert descriptor #104 + ... + createAsOfTime: + wallTime: "1640995200000000000" + + declarativeSchemaChangerState: + + authorization: + + userName: root + + jobId: "1" + dependedOnBy: + - columnIds: + - 1 + id: 106 + - - columnIds: + - - 1 + - id: 109 + - - columnIds: + - - 2 + - id: 109 + - indexId: 2 + - - columnIds: + - - 3 + - id: 109 + - indexId: 3 + families: + - columnIds: + ... + time: {} + unexposedParentSchemaId: 101 + - version: "3" + + version: "4" +upsert descriptor #105 + ... + createAsOfTime: + wallTime: "1640995200000000000" + - dependedOnBy: + - - byId: true + - id: 109 + + declarativeSchemaChangerState: + + authorization: + + userName: root + + jobId: "1" + families: + - columnIds: + ... + start: "1" + unexposedParentSchemaId: 101 + - version: "2" + + version: "3" +upsert descriptor #106 + ... + createAsOfTime: + wallTime: "1640995200000000000" + - dependedOnBy: + - - columnIds: + - - 1 + - id: 109 + + declarativeSchemaChangerState: + + authorization: + + userName: root + + jobId: "1" + dependsOn: + - 104 + ... + time: {} + unexposedParentSchemaId: 101 + - version: "2" + + version: "3" + viewQuery: SELECT a FROM defaultdb.public.t +upsert descriptor #107 + type: + arrayTypeId: 108 + + declarativeSchemaChangerState: + + authorization: + + userName: root + + jobId: "1" + enumMembers: + - logicalRepresentation: Monday + ... + version: 2 + referencingDescriptorIds: + - - 109 + - 110 + - version: "3" + + version: "4" +upsert descriptor #108 + ... + family: ArrayFamily + oid: 100108 + + declarativeSchemaChangerState: + + authorization: + + userName: root + + jobId: "1" + id: 108 + kind: ALIAS + ... + version: 2 + referencingDescriptorIds: + - - 109 + - 110 + - version: "3" + + version: "4" +upsert descriptor #109 + function: + + declarativeSchemaChangerState: + + authorization: + + userName: root + + currentStatuses: + + jobId: "1" + + relevantStatements: + + - statement: + + redactedStatement: DROP FUNCTION ‹""›.‹""›.‹f› + + statement: DROP FUNCTION f + + statementTag: DROP FUNCTION + + targetRanks: + + targets: + dependsOn: + - 104 + ... + oid: 20 + width: 64 + - version: "1" + + state: DROP + + version: "2" + volatility: IMMUTABLE +persist all catalog changes to storage +create job #1 (non-cancelable: true): "DROP FUNCTION \"\".\"\".f" + descriptor IDs: [104 105 106 107 108 109] +# end PreCommitPhase +commit transaction #1 +notified job registry to adopt jobs: [1] +# begin PostCommitPhase +begin transaction #2 +commit transaction #2 +begin transaction #3 +## PostCommitNonRevertiblePhase stage 1 of 1 with 8 MutationType ops +upsert descriptor #104 + ... + createAsOfTime: + wallTime: "1640995200000000000" + - declarativeSchemaChangerState: + - authorization: + - userName: root + - jobId: "1" + dependedOnBy: + - columnIds: + ... + time: {} + unexposedParentSchemaId: 101 + - version: "4" + + version: "5" +upsert descriptor #105 + ... + createAsOfTime: + wallTime: "1640995200000000000" + - declarativeSchemaChangerState: + - authorization: + - userName: root + - jobId: "1" + families: + - columnIds: + ... + start: "1" + unexposedParentSchemaId: 101 + - version: "3" + + version: "4" +upsert descriptor #106 + ... + createAsOfTime: + wallTime: "1640995200000000000" + - declarativeSchemaChangerState: + - authorization: + - userName: root + - jobId: "1" + dependsOn: + - 104 + ... + time: {} + unexposedParentSchemaId: 101 + - version: "3" + + version: "4" + viewQuery: SELECT a FROM defaultdb.public.t +upsert descriptor #107 + type: + arrayTypeId: 108 + - declarativeSchemaChangerState: + - authorization: + - userName: root + - jobId: "1" + enumMembers: + - logicalRepresentation: Monday + ... + referencingDescriptorIds: + - 110 + - version: "4" + + version: "5" +upsert descriptor #108 + ... + family: ArrayFamily + oid: 100108 + - declarativeSchemaChangerState: + - authorization: + - userName: root + - jobId: "1" + id: 108 + kind: ALIAS + ... + referencingDescriptorIds: + - 110 + - version: "4" + + version: "5" +delete descriptor #109 +persist all catalog changes to storage +update progress of schema change job #1: "all stages completed" +set schema change job #1 to non-cancellable +updated schema change job #1 descriptor IDs to [] +write *eventpb.FinishSchemaChange to event log: + sc: + descriptorId: 109 +commit transaction #3 +# end PostCommitPhase diff --git a/pkg/sql/schemachanger/testdata/end_to_end/drop_index_with_materialized_view_dep b/pkg/sql/schemachanger/testdata/end_to_end/drop_index_with_materialized_view_dep index d13523b6df9e..26927b5c3437 100644 --- a/pkg/sql/schemachanger/testdata/end_to_end/drop_index_with_materialized_view_dep +++ b/pkg/sql/schemachanger/testdata/end_to_end/drop_index_with_materialized_view_dep @@ -44,7 +44,7 @@ write *eventpb.DropIndex to event log: tag: DROP INDEX user: root tableName: defaultdb.public.v2 -## StatementPhase stage 1 of 1 with 5 MutationType ops +## StatementPhase stage 1 of 1 with 6 MutationType ops delete object namespace entry {100 101 v3} -> 106 upsert descriptor #105 ... @@ -127,7 +127,7 @@ upsert descriptor #106 ## PreCommitPhase stage 1 of 2 with 1 MutationType op undo all catalog changes within txn #1 persist all catalog changes to storage -## PreCommitPhase stage 2 of 2 with 12 MutationType ops +## PreCommitPhase stage 2 of 2 with 13 MutationType ops delete object namespace entry {100 101 v3} -> 106 upsert descriptor #105 ... diff --git a/pkg/sql/schemachanger/testdata/end_to_end/drop_table b/pkg/sql/schemachanger/testdata/end_to_end/drop_table index 4bc5b02bfe95..9bdcd3f8e1de 100644 --- a/pkg/sql/schemachanger/testdata/end_to_end/drop_table +++ b/pkg/sql/schemachanger/testdata/end_to_end/drop_table @@ -27,7 +27,7 @@ write *eventpb.DropTable to event log: tag: DROP TABLE user: root tableName: db.sc.t -## StatementPhase stage 1 of 1 with 4 MutationType ops +## StatementPhase stage 1 of 1 with 5 MutationType ops delete object namespace entry {104 106 t} -> 107 upsert descriptor #107 ... @@ -49,7 +49,7 @@ delete comment TableCommentType(objID: 107, subID: 0) ## PreCommitPhase stage 1 of 2 with 1 MutationType op undo all catalog changes within txn #1 persist all catalog changes to storage -## PreCommitPhase stage 2 of 2 with 11 MutationType ops +## PreCommitPhase stage 2 of 2 with 12 MutationType ops delete object namespace entry {104 106 t} -> 107 upsert descriptor #107 ... diff --git a/pkg/sql/schemachanger/testdata/explain/drop_function b/pkg/sql/schemachanger/testdata/explain/drop_function new file mode 100644 index 000000000000..9e9962cb918f --- /dev/null +++ b/pkg/sql/schemachanger/testdata/explain/drop_function @@ -0,0 +1,94 @@ +/* setup */ +CREATE TABLE t( + a INT PRIMARY KEY, + b INT, + C INT, + INDEX t_idx_b(b), + INDEX t_idx_c(c) +); +CREATE SEQUENCE sq1; +CREATE VIEW v AS SELECT a FROM t; +CREATE TYPE notmyworkday AS ENUM ('Monday', 'Tuesday'); +CREATE FUNCTION f(a notmyworkday) RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ + SELECT a FROM t; + SELECT b FROM t@t_idx_b; + SELECT c FROM t@t_idx_c; + SELECT a FROM v; + SELECT nextval('sq1'); +$$; +CREATE TABLE t2(a notmyworkday); + +/* test */ +EXPLAIN (ddl) DROP FUNCTION f; +---- +Schema change plan for DROP FUNCTION ‹""›.‹""›.‹f›; + ├── StatementPhase + │ └── Stage 1 of 1 in StatementPhase + │ ├── 10 elements transitioning toward ABSENT + │ │ ├── PUBLIC → ABSENT Owner:{DescID: 109} + │ │ ├── PUBLIC → ABSENT UserPrivileges:{DescID: 109, Name: admin} + │ │ ├── PUBLIC → ABSENT UserPrivileges:{DescID: 109, Name: root} + │ │ ├── PUBLIC → DROPPED Function:{DescID: 109} + │ │ ├── PUBLIC → ABSENT ObjectParent:{DescID: 109, ReferencedDescID: 101} + │ │ ├── PUBLIC → ABSENT FunctionName:{DescID: 109} + │ │ ├── PUBLIC → ABSENT FunctionVolatility:{DescID: 109} + │ │ ├── PUBLIC → ABSENT FunctionLeakProof:{DescID: 109} + │ │ ├── PUBLIC → ABSENT FunctionNullInputBehavior:{DescID: 109} + │ │ └── PUBLIC → ABSENT FunctionBody:{DescID: 109} + │ └── 4 Mutation operations + │ ├── MarkDescriptorAsDropped {"DescriptorID":109} + │ ├── RemoveObjectParent {"ObjectID":109,"ParentSchemaID":101} + │ ├── RemoveBackReferenceInTypes {"BackReferencedDescriptorID":109} + │ └── RemoveBackReferencesInRelations {"BackReferencedID":109} + ├── PreCommitPhase + │ ├── Stage 1 of 2 in PreCommitPhase + │ │ ├── 10 elements transitioning toward ABSENT + │ │ │ ├── ABSENT → PUBLIC Owner:{DescID: 109} + │ │ │ ├── ABSENT → PUBLIC UserPrivileges:{DescID: 109, Name: admin} + │ │ │ ├── ABSENT → PUBLIC UserPrivileges:{DescID: 109, Name: root} + │ │ │ ├── DROPPED → PUBLIC Function:{DescID: 109} + │ │ │ ├── ABSENT → PUBLIC ObjectParent:{DescID: 109, ReferencedDescID: 101} + │ │ │ ├── ABSENT → PUBLIC FunctionName:{DescID: 109} + │ │ │ ├── ABSENT → PUBLIC FunctionVolatility:{DescID: 109} + │ │ │ ├── ABSENT → PUBLIC FunctionLeakProof:{DescID: 109} + │ │ │ ├── ABSENT → PUBLIC FunctionNullInputBehavior:{DescID: 109} + │ │ │ └── ABSENT → PUBLIC FunctionBody:{DescID: 109} + │ │ └── 1 Mutation operation + │ │ └── UndoAllInTxnImmediateMutationOpSideEffects + │ └── Stage 2 of 2 in PreCommitPhase + │ ├── 10 elements transitioning toward ABSENT + │ │ ├── PUBLIC → ABSENT Owner:{DescID: 109} + │ │ ├── PUBLIC → ABSENT UserPrivileges:{DescID: 109, Name: admin} + │ │ ├── PUBLIC → ABSENT UserPrivileges:{DescID: 109, Name: root} + │ │ ├── PUBLIC → DROPPED Function:{DescID: 109} + │ │ ├── PUBLIC → ABSENT ObjectParent:{DescID: 109, ReferencedDescID: 101} + │ │ ├── PUBLIC → ABSENT FunctionName:{DescID: 109} + │ │ ├── PUBLIC → ABSENT FunctionVolatility:{DescID: 109} + │ │ ├── PUBLIC → ABSENT FunctionLeakProof:{DescID: 109} + │ │ ├── PUBLIC → ABSENT FunctionNullInputBehavior:{DescID: 109} + │ │ └── PUBLIC → ABSENT FunctionBody:{DescID: 109} + │ └── 11 Mutation operations + │ ├── MarkDescriptorAsDropped {"DescriptorID":109} + │ ├── RemoveObjectParent {"ObjectID":109,"ParentSchemaID":101} + │ ├── RemoveBackReferenceInTypes {"BackReferencedDescriptorID":109} + │ ├── RemoveBackReferencesInRelations {"BackReferencedID":109} + │ ├── SetJobStateOnDescriptor {"DescriptorID":104,"Initialize":true} + │ ├── SetJobStateOnDescriptor {"DescriptorID":105,"Initialize":true} + │ ├── SetJobStateOnDescriptor {"DescriptorID":106,"Initialize":true} + │ ├── SetJobStateOnDescriptor {"DescriptorID":107,"Initialize":true} + │ ├── SetJobStateOnDescriptor {"DescriptorID":108,"Initialize":true} + │ ├── SetJobStateOnDescriptor {"DescriptorID":109,"Initialize":true} + │ └── CreateSchemaChangerJob {"NonCancelable":true,"RunningStatus":"PostCommitNonRev..."} + └── PostCommitNonRevertiblePhase + └── Stage 1 of 1 in PostCommitNonRevertiblePhase + ├── 1 element transitioning toward ABSENT + │ └── DROPPED → ABSENT Function:{DescID: 109} + └── 8 Mutation operations + ├── DeleteDescriptor {"DescriptorID":109} + ├── RemoveJobStateFromDescriptor {"DescriptorID":104} + ├── RemoveJobStateFromDescriptor {"DescriptorID":105} + ├── RemoveJobStateFromDescriptor {"DescriptorID":106} + ├── RemoveJobStateFromDescriptor {"DescriptorID":107} + ├── RemoveJobStateFromDescriptor {"DescriptorID":108} + ├── RemoveJobStateFromDescriptor {"DescriptorID":109} + └── UpdateSchemaChangerJob {"IsNonCancelable":true,"RunningStatus":"all stages compl..."} diff --git a/pkg/sql/schemachanger/testdata/explain/drop_index_with_materialized_view_dep b/pkg/sql/schemachanger/testdata/explain/drop_index_with_materialized_view_dep index 8545dacb89a4..df43c258bc84 100644 --- a/pkg/sql/schemachanger/testdata/explain/drop_index_with_materialized_view_dep +++ b/pkg/sql/schemachanger/testdata/explain/drop_index_with_materialized_view_dep @@ -38,9 +38,10 @@ Schema change plan for DROP INDEX ‹defaultdb›.‹public›.‹v2›@‹idx │ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 106, ColumnID: 1, IndexID: 1} │ │ ├── PUBLIC → VALIDATED PrimaryIndex:{DescID: 106, IndexID: 1, ConstraintID: 1} │ │ └── PUBLIC → ABSENT IndexName:{DescID: 106, Name: v3_pkey, IndexID: 1} - │ └── 5 Mutation operations + │ └── 6 Mutation operations │ ├── MarkDescriptorAsDropped {"DescriptorID":106} - │ ├── RemoveViewBackReferencesInRelations {"BackReferencedViewID":106} + │ ├── RemoveBackReferencesInRelations {"BackReferencedID":106} + │ ├── RemoveObjectParent {"ObjectID":106,"ParentSchemaID":101} │ ├── RemoveColumnDefaultExpression {"ColumnID":2,"TableID":106} │ ├── MakePublicSecondaryIndexWriteOnly {"IndexID":2,"TableID":105} │ └── DrainDescriptorName {"Namespace":{"DatabaseID":100,"DescriptorID":106,"Name":"v3","SchemaID":101}} @@ -105,9 +106,10 @@ Schema change plan for DROP INDEX ‹defaultdb›.‹public›.‹v2›@‹idx │ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 106, ColumnID: 1, IndexID: 1} │ │ ├── PUBLIC → ABSENT PrimaryIndex:{DescID: 106, IndexID: 1, ConstraintID: 1} │ │ └── PUBLIC → ABSENT IndexName:{DescID: 106, Name: v3_pkey, IndexID: 1} - │ └── 12 Mutation operations + │ └── 13 Mutation operations │ ├── MarkDescriptorAsDropped {"DescriptorID":106} │ ├── RemoveBackReferencesInRelations {"BackReferencedID":106} + │ ├── RemoveObjectParent {"ObjectID":106,"ParentSchemaID":101} │ ├── RemoveColumnDefaultExpression {"ColumnID":2,"TableID":106} │ ├── MakePublicSecondaryIndexWriteOnly {"IndexID":2,"TableID":105} │ ├── DrainDescriptorName {"Namespace":{"DatabaseID":100,"DescriptorID":106,"Name":"v3","SchemaID":101}} diff --git a/pkg/sql/schemachanger/testdata/explain/drop_table b/pkg/sql/schemachanger/testdata/explain/drop_table index 11c25f66bb4e..e8d04dce7172 100644 --- a/pkg/sql/schemachanger/testdata/explain/drop_table +++ b/pkg/sql/schemachanger/testdata/explain/drop_table @@ -41,8 +41,9 @@ Schema change plan for DROP TABLE ‹db›.‹sc›.‹t›; │ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 107, ColumnID: 2, IndexID: 1} │ │ ├── PUBLIC → VALIDATED PrimaryIndex:{DescID: 107, IndexID: 1, ConstraintID: 1} │ │ └── PUBLIC → ABSENT IndexName:{DescID: 107, Name: t_pkey, IndexID: 1} - │ └── 4 Mutation operations + │ └── 5 Mutation operations │ ├── MarkDescriptorAsDropped {"DescriptorID":107} + │ ├── RemoveObjectParent {"ObjectID":107,"ParentSchemaID":106} │ ├── RemoveTableComment {"TableID":107} │ ├── RemoveColumnDefaultExpression {"ColumnID":3,"TableID":107} │ └── DrainDescriptorName {"Namespace":{"DatabaseID":104,"DescriptorID":107,"Name":"t","SchemaID":106}} @@ -111,8 +112,9 @@ Schema change plan for DROP TABLE ‹db›.‹sc›.‹t›; │ │ ├── PUBLIC → ABSENT IndexColumn:{DescID: 107, ColumnID: 2, IndexID: 1} │ │ ├── PUBLIC → ABSENT PrimaryIndex:{DescID: 107, IndexID: 1, ConstraintID: 1} │ │ └── PUBLIC → ABSENT IndexName:{DescID: 107, Name: t_pkey, IndexID: 1} - │ └── 11 Mutation operations + │ └── 12 Mutation operations │ ├── MarkDescriptorAsDropped {"DescriptorID":107} + │ ├── RemoveObjectParent {"ObjectID":107,"ParentSchemaID":106} │ ├── RemoveTableComment {"TableID":107} │ ├── RemoveColumnDefaultExpression {"ColumnID":3,"TableID":107} │ ├── DrainDescriptorName {"Namespace":{"DatabaseID":104,"DescriptorID":107,"Name":"t","SchemaID":106}} diff --git a/pkg/sql/schemachanger/testdata/explain_verbose/drop_function b/pkg/sql/schemachanger/testdata/explain_verbose/drop_function new file mode 100644 index 000000000000..27dc014ea258 --- /dev/null +++ b/pkg/sql/schemachanger/testdata/explain_verbose/drop_function @@ -0,0 +1,374 @@ +/* setup */ +CREATE TABLE t( + a INT PRIMARY KEY, + b INT, + C INT, + INDEX t_idx_b(b), + INDEX t_idx_c(c) +); +CREATE SEQUENCE sq1; +CREATE VIEW v AS SELECT a FROM t; +CREATE TYPE notmyworkday AS ENUM ('Monday', 'Tuesday'); +CREATE FUNCTION f(a notmyworkday) RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ + SELECT a FROM t; + SELECT b FROM t@t_idx_b; + SELECT c FROM t@t_idx_c; + SELECT a FROM v; + SELECT nextval('sq1'); +$$; +CREATE TABLE t2(a notmyworkday); + +/* test */ +EXPLAIN (ddl, verbose) DROP FUNCTION f; +---- +• Schema change plan for DROP FUNCTION ‹""›.‹""›.‹f›; +│ +├── • StatementPhase +│ │ +│ └── • Stage 1 of 1 in StatementPhase +│ │ +│ ├── • 10 elements transitioning toward ABSENT +│ │ │ +│ │ ├── • Owner:{DescID: 109} +│ │ │ │ PUBLIC → ABSENT +│ │ │ │ +│ │ │ ├── • Precedence dependency from DROPPED Function:{DescID: 109} +│ │ │ │ rule: "descriptor dropped before dependent element removal" +│ │ │ │ +│ │ │ └── • skip PUBLIC → ABSENT operations +│ │ │ rule: "skip element removal ops on descriptor drop" +│ │ │ +│ │ ├── • UserPrivileges:{DescID: 109, Name: admin} +│ │ │ │ PUBLIC → ABSENT +│ │ │ │ +│ │ │ ├── • Precedence dependency from DROPPED Function:{DescID: 109} +│ │ │ │ rule: "descriptor dropped before dependent element removal" +│ │ │ │ +│ │ │ └── • skip PUBLIC → ABSENT operations +│ │ │ rule: "skip element removal ops on descriptor drop" +│ │ │ +│ │ ├── • UserPrivileges:{DescID: 109, Name: root} +│ │ │ │ PUBLIC → ABSENT +│ │ │ │ +│ │ │ ├── • Precedence dependency from DROPPED Function:{DescID: 109} +│ │ │ │ rule: "descriptor dropped before dependent element removal" +│ │ │ │ +│ │ │ └── • skip PUBLIC → ABSENT operations +│ │ │ rule: "skip element removal ops on descriptor drop" +│ │ │ +│ │ ├── • Function:{DescID: 109} +│ │ │ PUBLIC → DROPPED +│ │ │ +│ │ ├── • ObjectParent:{DescID: 109, ReferencedDescID: 101} +│ │ │ │ PUBLIC → ABSENT +│ │ │ │ +│ │ │ └── • Precedence dependency from DROPPED Function:{DescID: 109} +│ │ │ rule: "descriptor dropped before dependent element removal" +│ │ │ +│ │ ├── • FunctionName:{DescID: 109} +│ │ │ │ PUBLIC → ABSENT +│ │ │ │ +│ │ │ ├── • Precedence dependency from DROPPED Function:{DescID: 109} +│ │ │ │ rule: "descriptor dropped before dependent element removal" +│ │ │ │ +│ │ │ └── • skip PUBLIC → ABSENT operations +│ │ │ rule: "skip element removal ops on descriptor drop" +│ │ │ +│ │ ├── • FunctionVolatility:{DescID: 109} +│ │ │ │ PUBLIC → ABSENT +│ │ │ │ +│ │ │ ├── • Precedence dependency from DROPPED Function:{DescID: 109} +│ │ │ │ rule: "descriptor dropped before dependent element removal" +│ │ │ │ +│ │ │ └── • skip PUBLIC → ABSENT operations +│ │ │ rule: "skip element removal ops on descriptor drop" +│ │ │ +│ │ ├── • FunctionLeakProof:{DescID: 109} +│ │ │ │ PUBLIC → ABSENT +│ │ │ │ +│ │ │ ├── • Precedence dependency from DROPPED Function:{DescID: 109} +│ │ │ │ rule: "descriptor dropped before dependent element removal" +│ │ │ │ +│ │ │ └── • skip PUBLIC → ABSENT operations +│ │ │ rule: "skip element removal ops on descriptor drop" +│ │ │ +│ │ ├── • FunctionNullInputBehavior:{DescID: 109} +│ │ │ │ PUBLIC → ABSENT +│ │ │ │ +│ │ │ ├── • Precedence dependency from DROPPED Function:{DescID: 109} +│ │ │ │ rule: "descriptor dropped before dependent element removal" +│ │ │ │ +│ │ │ └── • skip PUBLIC → ABSENT operations +│ │ │ rule: "skip element removal ops on descriptor drop" +│ │ │ +│ │ └── • FunctionBody:{DescID: 109} +│ │ │ PUBLIC → ABSENT +│ │ │ +│ │ └── • Precedence dependency from DROPPED Function:{DescID: 109} +│ │ rule: "descriptor dropped before dependent element removal" +│ │ +│ └── • 4 Mutation operations +│ │ +│ ├── • MarkDescriptorAsDropped +│ │ DescriptorID: 109 +│ │ +│ ├── • RemoveObjectParent +│ │ ObjectID: 109 +│ │ ParentSchemaID: 101 +│ │ +│ ├── • RemoveBackReferenceInTypes +│ │ BackReferencedDescriptorID: 109 +│ │ TypeIDs: +│ │ - 107 +│ │ - 108 +│ │ +│ └── • RemoveBackReferencesInRelations +│ BackReferencedID: 109 +│ RelationIDs: +│ - 104 +│ - 104 +│ - 104 +│ - 106 +│ - 105 +│ +├── • PreCommitPhase +│ │ +│ ├── • Stage 1 of 2 in PreCommitPhase +│ │ │ +│ │ ├── • 10 elements transitioning toward ABSENT +│ │ │ │ +│ │ │ ├── • Owner:{DescID: 109} +│ │ │ │ ABSENT → PUBLIC +│ │ │ │ +│ │ │ ├── • UserPrivileges:{DescID: 109, Name: admin} +│ │ │ │ ABSENT → PUBLIC +│ │ │ │ +│ │ │ ├── • UserPrivileges:{DescID: 109, Name: root} +│ │ │ │ ABSENT → PUBLIC +│ │ │ │ +│ │ │ ├── • Function:{DescID: 109} +│ │ │ │ DROPPED → PUBLIC +│ │ │ │ +│ │ │ ├── • ObjectParent:{DescID: 109, ReferencedDescID: 101} +│ │ │ │ ABSENT → PUBLIC +│ │ │ │ +│ │ │ ├── • FunctionName:{DescID: 109} +│ │ │ │ ABSENT → PUBLIC +│ │ │ │ +│ │ │ ├── • FunctionVolatility:{DescID: 109} +│ │ │ │ ABSENT → PUBLIC +│ │ │ │ +│ │ │ ├── • FunctionLeakProof:{DescID: 109} +│ │ │ │ ABSENT → PUBLIC +│ │ │ │ +│ │ │ ├── • FunctionNullInputBehavior:{DescID: 109} +│ │ │ │ ABSENT → PUBLIC +│ │ │ │ +│ │ │ └── • FunctionBody:{DescID: 109} +│ │ │ ABSENT → PUBLIC +│ │ │ +│ │ └── • 1 Mutation operation +│ │ │ +│ │ └── • UndoAllInTxnImmediateMutationOpSideEffects +│ │ {} +│ │ +│ └── • Stage 2 of 2 in PreCommitPhase +│ │ +│ ├── • 10 elements transitioning toward ABSENT +│ │ │ +│ │ ├── • Owner:{DescID: 109} +│ │ │ │ PUBLIC → ABSENT +│ │ │ │ +│ │ │ ├── • Precedence dependency from DROPPED Function:{DescID: 109} +│ │ │ │ rule: "descriptor dropped before dependent element removal" +│ │ │ │ +│ │ │ └── • skip PUBLIC → ABSENT operations +│ │ │ rule: "skip element removal ops on descriptor drop" +│ │ │ +│ │ ├── • UserPrivileges:{DescID: 109, Name: admin} +│ │ │ │ PUBLIC → ABSENT +│ │ │ │ +│ │ │ ├── • Precedence dependency from DROPPED Function:{DescID: 109} +│ │ │ │ rule: "descriptor dropped before dependent element removal" +│ │ │ │ +│ │ │ └── • skip PUBLIC → ABSENT operations +│ │ │ rule: "skip element removal ops on descriptor drop" +│ │ │ +│ │ ├── • UserPrivileges:{DescID: 109, Name: root} +│ │ │ │ PUBLIC → ABSENT +│ │ │ │ +│ │ │ ├── • Precedence dependency from DROPPED Function:{DescID: 109} +│ │ │ │ rule: "descriptor dropped before dependent element removal" +│ │ │ │ +│ │ │ └── • skip PUBLIC → ABSENT operations +│ │ │ rule: "skip element removal ops on descriptor drop" +│ │ │ +│ │ ├── • Function:{DescID: 109} +│ │ │ PUBLIC → DROPPED +│ │ │ +│ │ ├── • ObjectParent:{DescID: 109, ReferencedDescID: 101} +│ │ │ │ PUBLIC → ABSENT +│ │ │ │ +│ │ │ └── • Precedence dependency from DROPPED Function:{DescID: 109} +│ │ │ rule: "descriptor dropped before dependent element removal" +│ │ │ +│ │ ├── • FunctionName:{DescID: 109} +│ │ │ │ PUBLIC → ABSENT +│ │ │ │ +│ │ │ ├── • Precedence dependency from DROPPED Function:{DescID: 109} +│ │ │ │ rule: "descriptor dropped before dependent element removal" +│ │ │ │ +│ │ │ └── • skip PUBLIC → ABSENT operations +│ │ │ rule: "skip element removal ops on descriptor drop" +│ │ │ +│ │ ├── • FunctionVolatility:{DescID: 109} +│ │ │ │ PUBLIC → ABSENT +│ │ │ │ +│ │ │ ├── • Precedence dependency from DROPPED Function:{DescID: 109} +│ │ │ │ rule: "descriptor dropped before dependent element removal" +│ │ │ │ +│ │ │ └── • skip PUBLIC → ABSENT operations +│ │ │ rule: "skip element removal ops on descriptor drop" +│ │ │ +│ │ ├── • FunctionLeakProof:{DescID: 109} +│ │ │ │ PUBLIC → ABSENT +│ │ │ │ +│ │ │ ├── • Precedence dependency from DROPPED Function:{DescID: 109} +│ │ │ │ rule: "descriptor dropped before dependent element removal" +│ │ │ │ +│ │ │ └── • skip PUBLIC → ABSENT operations +│ │ │ rule: "skip element removal ops on descriptor drop" +│ │ │ +│ │ ├── • FunctionNullInputBehavior:{DescID: 109} +│ │ │ │ PUBLIC → ABSENT +│ │ │ │ +│ │ │ ├── • Precedence dependency from DROPPED Function:{DescID: 109} +│ │ │ │ rule: "descriptor dropped before dependent element removal" +│ │ │ │ +│ │ │ └── • skip PUBLIC → ABSENT operations +│ │ │ rule: "skip element removal ops on descriptor drop" +│ │ │ +│ │ └── • FunctionBody:{DescID: 109} +│ │ │ PUBLIC → ABSENT +│ │ │ +│ │ └── • Precedence dependency from DROPPED Function:{DescID: 109} +│ │ rule: "descriptor dropped before dependent element removal" +│ │ +│ └── • 11 Mutation operations +│ │ +│ ├── • MarkDescriptorAsDropped +│ │ DescriptorID: 109 +│ │ +│ ├── • RemoveObjectParent +│ │ ObjectID: 109 +│ │ ParentSchemaID: 101 +│ │ +│ ├── • RemoveBackReferenceInTypes +│ │ BackReferencedDescriptorID: 109 +│ │ TypeIDs: +│ │ - 107 +│ │ - 108 +│ │ +│ ├── • RemoveBackReferencesInRelations +│ │ BackReferencedID: 109 +│ │ RelationIDs: +│ │ - 104 +│ │ - 104 +│ │ - 104 +│ │ - 106 +│ │ - 105 +│ │ +│ ├── • SetJobStateOnDescriptor +│ │ DescriptorID: 104 +│ │ Initialize: true +│ │ +│ ├── • SetJobStateOnDescriptor +│ │ DescriptorID: 105 +│ │ Initialize: true +│ │ +│ ├── • SetJobStateOnDescriptor +│ │ DescriptorID: 106 +│ │ Initialize: true +│ │ +│ ├── • SetJobStateOnDescriptor +│ │ DescriptorID: 107 +│ │ Initialize: true +│ │ +│ ├── • SetJobStateOnDescriptor +│ │ DescriptorID: 108 +│ │ Initialize: true +│ │ +│ ├── • SetJobStateOnDescriptor +│ │ DescriptorID: 109 +│ │ Initialize: true +│ │ +│ └── • CreateSchemaChangerJob +│ Authorization: +│ UserName: root +│ DescriptorIDs: +│ - 104 +│ - 105 +│ - 106 +│ - 107 +│ - 108 +│ - 109 +│ JobID: 1 +│ NonCancelable: true +│ RunningStatus: PostCommitNonRevertiblePhase stage 1 of 1 with 1 MutationType op pending +│ Statements: +│ - statement: DROP FUNCTION f +│ redactedstatement: DROP FUNCTION ‹""›.‹""›.‹f› +│ statementtag: DROP FUNCTION +│ +└── • PostCommitNonRevertiblePhase + │ + └── • Stage 1 of 1 in PostCommitNonRevertiblePhase + │ + ├── • 1 element transitioning toward ABSENT + │ │ + │ └── • Function:{DescID: 109} + │ │ DROPPED → ABSENT + │ │ + │ └── • PreviousStagePrecedence dependency from DROPPED Function:{DescID: 109} + │ rule: "descriptor dropped in transaction before removal" + │ + └── • 8 Mutation operations + │ + ├── • DeleteDescriptor + │ DescriptorID: 109 + │ + ├── • RemoveJobStateFromDescriptor + │ DescriptorID: 104 + │ JobID: 1 + │ + ├── • RemoveJobStateFromDescriptor + │ DescriptorID: 105 + │ JobID: 1 + │ + ├── • RemoveJobStateFromDescriptor + │ DescriptorID: 106 + │ JobID: 1 + │ + ├── • RemoveJobStateFromDescriptor + │ DescriptorID: 107 + │ JobID: 1 + │ + ├── • RemoveJobStateFromDescriptor + │ DescriptorID: 108 + │ JobID: 1 + │ + ├── • RemoveJobStateFromDescriptor + │ DescriptorID: 109 + │ JobID: 1 + │ + └── • UpdateSchemaChangerJob + DescriptorIDsToRemove: + - 104 + - 105 + - 106 + - 107 + - 108 + - 109 + IsNonCancelable: true + JobID: 1 + RunningStatus: all stages completed diff --git a/pkg/sql/schemachanger/testdata/explain_verbose/drop_index_with_materialized_view_dep b/pkg/sql/schemachanger/testdata/explain_verbose/drop_index_with_materialized_view_dep index 366b5a323e56..bdeb35163bac 100644 --- a/pkg/sql/schemachanger/testdata/explain_verbose/drop_index_with_materialized_view_dep +++ b/pkg/sql/schemachanger/testdata/explain_verbose/drop_index_with_materialized_view_dep @@ -278,16 +278,20 @@ EXPLAIN (ddl, verbose) DROP INDEX idx CASCADE; │ │ └── • skip PUBLIC → ABSENT operations │ │ rule: "skip index dependents removal ops on relation drop" │ │ -│ └── • 5 Mutation operations +│ └── • 6 Mutation operations │ │ │ ├── • MarkDescriptorAsDropped │ │ DescriptorID: 106 │ │ -│ ├── • RemoveViewBackReferencesInRelations -│ │ BackReferencedViewID: 106 +│ ├── • RemoveBackReferencesInRelations +│ │ BackReferencedID: 106 │ │ RelationIDs: │ │ - 105 │ │ +│ ├── • RemoveObjectParent +│ │ ObjectID: 106 +│ │ ParentSchemaID: 101 +│ │ │ ├── • RemoveColumnDefaultExpression │ │ ColumnID: 2 │ │ TableID: 106 @@ -725,7 +729,7 @@ EXPLAIN (ddl, verbose) DROP INDEX idx CASCADE; │ │ └── • skip PUBLIC → ABSENT operations │ │ rule: "skip index dependents removal ops on relation drop" │ │ -│ └── • 12 Mutation operations +│ └── • 13 Mutation operations │ │ │ ├── • MarkDescriptorAsDropped │ │ DescriptorID: 106 @@ -735,6 +739,10 @@ EXPLAIN (ddl, verbose) DROP INDEX idx CASCADE; │ │ RelationIDs: │ │ - 105 │ │ +│ ├── • RemoveObjectParent +│ │ ObjectID: 106 +│ │ ParentSchemaID: 101 +│ │ │ ├── • RemoveColumnDefaultExpression │ │ ColumnID: 2 │ │ TableID: 106 diff --git a/pkg/sql/schemachanger/testdata/explain_verbose/drop_table b/pkg/sql/schemachanger/testdata/explain_verbose/drop_table index 6ab899cba17d..777bc98f980b 100644 --- a/pkg/sql/schemachanger/testdata/explain_verbose/drop_table +++ b/pkg/sql/schemachanger/testdata/explain_verbose/drop_table @@ -304,11 +304,15 @@ EXPLAIN (ddl, verbose) DROP TABLE db.sc.t; │ │ └── • skip PUBLIC → ABSENT operations │ │ rule: "skip index dependents removal ops on relation drop" │ │ -│ └── • 4 Mutation operations +│ └── • 5 Mutation operations │ │ │ ├── • MarkDescriptorAsDropped │ │ DescriptorID: 107 │ │ +│ ├── • RemoveObjectParent +│ │ ObjectID: 107 +│ │ ParentSchemaID: 106 +│ │ │ ├── • RemoveTableComment │ │ TableID: 107 │ │ @@ -791,11 +795,15 @@ EXPLAIN (ddl, verbose) DROP TABLE db.sc.t; │ │ └── • skip PUBLIC → ABSENT operations │ │ rule: "skip index dependents removal ops on relation drop" │ │ -│ └── • 11 Mutation operations +│ └── • 12 Mutation operations │ │ │ ├── • MarkDescriptorAsDropped │ │ DescriptorID: 107 │ │ +│ ├── • RemoveObjectParent +│ │ ObjectID: 107 +│ │ ParentSchemaID: 106 +│ │ │ ├── • RemoveTableComment │ │ TableID: 107 │ │ From f2798298086f6a2274a846dda3fbee1861f4d180 Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Fri, 27 Jan 2023 17:04:51 +0100 Subject: [PATCH 09/11] upgrades: share a column family for the new tenant columns The previous change in that area was causing the new columns to be (implicitly) added into separate column families. However, we're expecting (from the bootstrap schema) to use a single column family for them. This patch fixes that. Release note: None --- pkg/upgrade/upgrades/tenant_table_migration.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/upgrade/upgrades/tenant_table_migration.go b/pkg/upgrade/upgrades/tenant_table_migration.go index 7fedb9e38285..31c0438712b7 100644 --- a/pkg/upgrade/upgrades/tenant_table_migration.go +++ b/pkg/upgrade/upgrades/tenant_table_migration.go @@ -30,9 +30,9 @@ import ( const addTenantColumns = ` ALTER TABLE system.public.tenants ALTER COLUMN active SET NOT VISIBLE, - ADD COLUMN name STRING, - ADD COLUMN data_state INT, - ADD COLUMN service_mode INT` + ADD COLUMN name STRING FAMILY "primary", + ADD COLUMN data_state INT FAMILY "primary", + ADD COLUMN service_mode INT FAMILY "primary"` const addTenantIndex1 = `CREATE UNIQUE INDEX tenants_name_idx ON system.public.tenants (name ASC)` const addTenantIndex2 = `CREATE INDEX tenants_service_mode_idx ON system.public.tenants (service_mode ASC)` From 7038e7032cdb66797b43fec8cb77fde175a702a8 Mon Sep 17 00:00:00 2001 From: Tobias Grieger Date: Tue, 24 Jan 2023 10:38:23 +0100 Subject: [PATCH 10/11] aggmetric: Destroy -> Unlink Epic: none Release note: None --- pkg/kv/kvserver/metrics.go | 4 ++-- pkg/kv/kvserver/tenantrate/factory.go | 4 ++-- pkg/kv/kvserver/tenantrate/metrics.go | 16 ++++++++-------- pkg/util/metric/aggmetric/agg_metric_test.go | 8 ++++---- pkg/util/metric/aggmetric/counter.go | 10 +++++++--- pkg/util/metric/aggmetric/gauge.go | 20 ++++++++++---------- pkg/util/metric/aggmetric/histogram.go | 10 +++++++--- 7 files changed, 40 insertions(+), 32 deletions(-) diff --git a/pkg/kv/kvserver/metrics.go b/pkg/kv/kvserver/metrics.go index ecb7088b5170..38953afe5d50 100644 --- a/pkg/kv/kvserver/metrics.go +++ b/pkg/kv/kvserver/metrics.go @@ -2172,9 +2172,9 @@ func (sm *TenantsStorageMetrics) releaseTenant(ctx context.Context, ref *tenantM &m.SysCount, &m.AbortSpanBytes, } { - // Reset before unlinking, see Destroy. + // Reset before unlinking, see Unlink. (*gptr).Update(0) - (*gptr).Destroy() + (*gptr).Unlink() *gptr = nil } sm.tenants.Delete(int64(ref._tenantID.ToUint64())) diff --git a/pkg/kv/kvserver/tenantrate/factory.go b/pkg/kv/kvserver/tenantrate/factory.go index 250c9b6415ca..73fbcf7a3172 100644 --- a/pkg/kv/kvserver/tenantrate/factory.go +++ b/pkg/kv/kvserver/tenantrate/factory.go @@ -69,7 +69,7 @@ func NewLimiterFactory(sv *settings.Values, knobs *TestingKnobs) *LimiterFactory } // GetTenant gets or creates a limiter for the given tenant. The limiters are -// reference counted; call Destroy on the returned limiter when it is no longer +// reference counted; call Unlink on the returned limiter when it is no longer // in use. If the closer channel is non-nil, closing it will lead to any blocked // requests becoming unblocked. func (rl *LimiterFactory) GetTenant( @@ -120,7 +120,7 @@ func (rl *LimiterFactory) Release(lim Limiter) { panic(errors.AssertionFailedf("two limiters exist for tenant %v", l.tenantID)) } if rcLim.refCount--; rcLim.refCount == 0 { - l.metrics.destroy() + l.metrics.unlink() l.qp.Close("released") delete(rl.mu.tenants, l.tenantID) } diff --git a/pkg/kv/kvserver/tenantrate/metrics.go b/pkg/kv/kvserver/tenantrate/metrics.go index 9f1008999782..fd932ad0727c 100644 --- a/pkg/kv/kvserver/tenantrate/metrics.go +++ b/pkg/kv/kvserver/tenantrate/metrics.go @@ -123,12 +123,12 @@ func (m *Metrics) tenantMetrics(tenantID roachpb.TenantID) tenantMetrics { } } -func (tm *tenantMetrics) destroy() { - tm.currentBlocked.Destroy() - tm.readBatchesAdmitted.Destroy() - tm.writeBatchesAdmitted.Destroy() - tm.readRequestsAdmitted.Destroy() - tm.writeRequestsAdmitted.Destroy() - tm.readBytesAdmitted.Destroy() - tm.writeBytesAdmitted.Destroy() +func (tm *tenantMetrics) unlink() { + tm.currentBlocked.Unlink() + tm.readBatchesAdmitted.Unlink() + tm.writeBatchesAdmitted.Unlink() + tm.readRequestsAdmitted.Unlink() + tm.writeRequestsAdmitted.Unlink() + tm.readBytesAdmitted.Unlink() + tm.writeBytesAdmitted.Unlink() } diff --git a/pkg/util/metric/aggmetric/agg_metric_test.go b/pkg/util/metric/aggmetric/agg_metric_test.go index 6fb3b8e98f27..842a30504889 100644 --- a/pkg/util/metric/aggmetric/agg_metric_test.go +++ b/pkg/util/metric/aggmetric/agg_metric_test.go @@ -94,10 +94,10 @@ func TestAggMetric(t *testing.T) { }) t.Run("destroy", func(t *testing.T) { - g3.Destroy() - c2.Destroy() - f3.Destroy() - h3.Destroy() + g3.Unlink() + c2.Unlink() + f3.Unlink() + h3.Unlink() echotest.Require(t, writePrometheusMetrics(t), datapathutils.TestDataPath(t, "destroy.txt")) }) diff --git a/pkg/util/metric/aggmetric/counter.go b/pkg/util/metric/aggmetric/counter.go index 3fce1a2ec936..e36d4cb7a70a 100644 --- a/pkg/util/metric/aggmetric/counter.go +++ b/pkg/util/metric/aggmetric/counter.go @@ -106,9 +106,13 @@ func (g *Counter) ToPrometheusMetric() *io_prometheus_client.Metric { } } -// Destroy disconnects this Counter from its parents. Unlike Gauge.Destroy, it -// does not decrement its value from its parent. -func (g *Counter) Destroy() { +// Unlink unlinks this child from the parent, i.e. the parent will no longer +// track this child (i.e. won't generate labels for it, etc). However, the child +// will continue to be functional and reference the parent, meaning updates to +// it will be reflected in the aggregate stored in the parent. +// +// See tenantrate.TestUseAfterRelease. +func (g *Counter) Unlink() { g.parent.remove(g) } diff --git a/pkg/util/metric/aggmetric/gauge.go b/pkg/util/metric/aggmetric/gauge.go index edcb1ed4b6d6..da185da844f0 100644 --- a/pkg/util/metric/aggmetric/gauge.go +++ b/pkg/util/metric/aggmetric/gauge.go @@ -107,13 +107,13 @@ func (g *Gauge) ToPrometheusMetric() *io_prometheus_client.Metric { } } -// Destroy is used to destroy the gauge associated with this child. It unlinks the -// Gauge from the parent, meaning it will no longer be reported as contributing to -// it. However, this Gauge will continue to be functional and it will continue to -// update the parent. +// Unlink unlinks this child from the parent, i.e. the parent will no longer +// track this child (i.e. won't generate labels for it, etc). However, the child +// will continue to be functional and reference the parent, meaning updates to +// it will be reflected in the aggregate stored in the parent. // // See tenantrate.TestUseAfterRelease. -func (g *Gauge) Destroy() { +func (g *Gauge) Unlink() { g.parent.remove(g) } @@ -227,13 +227,13 @@ func (g *GaugeFloat64) ToPrometheusMetric() *io_prometheus_client.Metric { } } -// Destroy is used to destroy the gauge associated with this child. It unlinks the -// GaugeFloat64 from the parent, meaning it will no longer be reported as contributing to -// it. However, this Gauge will continue to be functional and it will continue to -// update the parent. +// Unlink unlinks this child from the parent, i.e. the parent will no longer +// track this child (i.e. won't generate labels for it, etc). However, the child +// will continue to be functional and reference the parent, meaning updates to +// it will be reflected in the aggregate stored in the parent. // // See tenantrate.TestUseAfterRelease. -func (g *GaugeFloat64) Destroy() { +func (g *GaugeFloat64) Unlink() { g.parent.remove(g) } diff --git a/pkg/util/metric/aggmetric/histogram.go b/pkg/util/metric/aggmetric/histogram.go index 66bb7aa3853b..68d30fab17a2 100644 --- a/pkg/util/metric/aggmetric/histogram.go +++ b/pkg/util/metric/aggmetric/histogram.go @@ -129,9 +129,13 @@ func (g *Histogram) ToPrometheusMetric() *io_prometheus_client.Metric { return g.h.ToPrometheusMetric() } -// Destroy disconnects this Histogram from its parents. Unlike Gauge.Destroy, it -// does not decrement its value from its parent. -func (g *Histogram) Destroy() { +// Unlink unlinks this child from the parent, i.e. the parent will no longer +// track this child (i.e. won't generate labels for it, etc). However, the child +// will continue to be functional and reference the parent, meaning updates to +// it will be reflected in the aggregate stored in the parent. +// +// See tenantrate.TestUseAfterRelease. +func (g *Histogram) Unlink() { g.parent.remove(g) } From 8611c84f5454d59b6d7d931b9139786d73cae8f5 Mon Sep 17 00:00:00 2001 From: Pavel Kalinnikov Date: Thu, 19 Jan 2023 17:53:49 +0000 Subject: [PATCH 11/11] kvserver: fix race on Replica.tenantLimiter When Replica is being destroyed, it can set tenantLimiter to nil. However, all accessors assume synchronization through IsInitialized atomic check and that this field can't be modified after the replica is initialized. There is no particular need in nullifying this limiter, the replica is going away soon, so this commit removes this line in order to fix the data race. Epic: none Release note: none --- pkg/kv/kvserver/replica_destroy.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/kv/kvserver/replica_destroy.go b/pkg/kv/kvserver/replica_destroy.go index a4c65b9a4780..930d4cb0353f 100644 --- a/pkg/kv/kvserver/replica_destroy.go +++ b/pkg/kv/kvserver/replica_destroy.go @@ -132,7 +132,6 @@ func (r *Replica) postDestroyRaftMuLocked(ctx context.Context, ms enginepb.MVCCS // Unhook the tenant rate limiter if we have one. if r.tenantLimiter != nil { r.store.tenantRateLimiters.Release(r.tenantLimiter) - r.tenantLimiter = nil } return nil