diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 28a3afcb72c2..9bec248b4c88 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -271,6 +271,7 @@ /pkg/cmd/skip-test/ @cockroachdb/test-eng /pkg/cmd/skiperrs/ @cockroachdb/sql-experience /pkg/cmd/skipped-tests/ @cockroachdb/test-eng +/pkg/cmd/smith/ @cockroachdb/sql-queries /pkg/cmd/smithcmp/ @cockroachdb/sql-queries /pkg/cmd/smithtest/ @cockroachdb/sql-queries /pkg/cmd/teamcity-trigger/ @cockroachdb/dev-inf diff --git a/pkg/BUILD.bazel b/pkg/BUILD.bazel index 2d08d4206923..14c4f7a34029 100644 --- a/pkg/BUILD.bazel +++ b/pkg/BUILD.bazel @@ -860,6 +860,8 @@ GO_TARGETS = [ "//pkg/cmd/skiperrs:skiperrs_lib", "//pkg/cmd/skipped-tests:skipped-tests", "//pkg/cmd/skipped-tests:skipped-tests_lib", + "//pkg/cmd/smith:smith", + "//pkg/cmd/smith:smith_lib", "//pkg/cmd/smithcmp:smithcmp", "//pkg/cmd/smithcmp:smithcmp_lib", "//pkg/cmd/smithtest:smithtest", @@ -2107,6 +2109,7 @@ GET_X_DATA_TARGETS = [ "//pkg/cmd/skip-test:get_x_data", "//pkg/cmd/skiperrs:get_x_data", "//pkg/cmd/skipped-tests:get_x_data", + "//pkg/cmd/smith:get_x_data", "//pkg/cmd/smithcmp:get_x_data", "//pkg/cmd/smithtest:get_x_data", "//pkg/cmd/teamcity-trigger:get_x_data", diff --git a/pkg/ccl/schemachangerccl/testdata/end_to_end/create_index b/pkg/ccl/schemachangerccl/testdata/end_to_end/create_index index 82039a6e3176..0b8689c8e4ee 100644 --- a/pkg/ccl/schemachangerccl/testdata/end_to_end/create_index +++ b/pkg/ccl/schemachangerccl/testdata/end_to_end/create_index @@ -285,7 +285,7 @@ notified job registry to adopt jobs: [1] begin transaction #2 commit transaction #2 begin transaction #3 -## PostCommitPhase stage 1 of 7 with 3 MutationType ops +## PostCommitPhase stage 1 of 6 with 3 MutationType ops upsert descriptor #104 ... - ABSENT @@ -314,14 +314,14 @@ upsert descriptor #104 unexposedParentSchemaId: 101 - version: "2" + version: "3" -update progress of schema change job #1: "PostCommitPhase stage 2 of 7 with 1 BackfillType op pending" +update progress of schema change job #1: "PostCommitPhase stage 2 of 6 with 1 BackfillType op pending" commit transaction #3 begin transaction #4 -## PostCommitPhase stage 2 of 7 with 1 BackfillType op +## PostCommitPhase stage 2 of 6 with 1 BackfillType op backfill indexes [2] from index #1 in table #104 commit transaction #4 begin transaction #5 -## PostCommitPhase stage 3 of 7 with 3 MutationType ops +## PostCommitPhase stage 3 of 6 with 3 MutationType ops upsert descriptor #104 ... - PUBLIC @@ -350,10 +350,10 @@ upsert descriptor #104 unexposedParentSchemaId: 101 - version: "3" + version: "4" -update progress of schema change job #1: "PostCommitPhase stage 4 of 7 with 1 MutationType op pending" +update progress of schema change job #1: "PostCommitPhase stage 4 of 6 with 1 MutationType op pending" commit transaction #5 begin transaction #6 -## PostCommitPhase stage 4 of 7 with 3 MutationType ops +## PostCommitPhase stage 4 of 6 with 3 MutationType ops upsert descriptor #104 ... - PUBLIC @@ -382,18 +382,18 @@ upsert descriptor #104 unexposedParentSchemaId: 101 - version: "4" + version: "5" -update progress of schema change job #1: "PostCommitPhase stage 5 of 7 with 1 BackfillType op pending" +update progress of schema change job #1: "PostCommitPhase stage 5 of 6 with 1 BackfillType op pending" commit transaction #6 begin transaction #7 -## PostCommitPhase stage 5 of 7 with 1 BackfillType op +## PostCommitPhase stage 5 of 6 with 1 BackfillType op merge temporary indexes [3] into backfilled indexes [2] in table #104 commit transaction #7 begin transaction #8 -## PostCommitPhase stage 6 of 7 with 1 ValidationType op +## PostCommitPhase stage 6 of 6 with 1 ValidationType op validate forward indexes [2] in table #104 commit transaction #8 begin transaction #9 -## PostCommitPhase stage 7 of 7 with 4 MutationType ops +## PostCommitNonRevertiblePhase stage 1 of 2 with 5 MutationType ops upsert descriptor #104 ... - PUBLIC @@ -401,10 +401,16 @@ upsert descriptor #104 - - MERGE_ONLY - - ABSENT - PUBLIC + - - WRITE_ONLY + - PUBLIC + - PUBLIC + + - TRANSIENT_DELETE_ONLY + - PUBLIC + - PUBLIC + - PUBLIC + - PUBLIC - - WRITE_ONLY - - PUBLIC + jobId: "1" + relevantStatements: ... BY LIST (id) (PARTITION p1 VALUES IN (1)) statementTag: CREATE INDEX @@ -448,7 +454,8 @@ upsert descriptor #104 + version: 4 + modificationTime: {} mutations: - - direction: ADD + - - direction: ADD + + - direction: DROP index: - constraintId: 2 - createdExplicitly: true @@ -485,35 +492,6 @@ upsert descriptor #104 - index: constraintId: 3 createdExplicitly: true - ... - time: {} - unexposedParentSchemaId: 101 - - version: "5" - + version: "6" -update progress of schema change job #1: "PostCommitNonRevertiblePhase stage 1 of 2 with 1 MutationType op pending" -set schema change job #1 to non-cancellable -commit transaction #9 -begin transaction #10 -## PostCommitNonRevertiblePhase stage 1 of 2 with 3 MutationType ops -upsert descriptor #104 - ... - - PUBLIC - - PUBLIC - - - WRITE_ONLY - + - TRANSIENT_DELETE_ONLY - - PUBLIC - - PUBLIC - ... - - money - version: 4 - - modificationTime: - - wallTime: "1640995200000000009" - + modificationTime: {} - mutations: - - - direction: ADD - + - direction: DROP - index: - constraintId: 3 ... version: 4 mutationId: 1 @@ -524,11 +502,12 @@ upsert descriptor #104 ... time: {} unexposedParentSchemaId: 101 - - version: "6" - + version: "7" + - version: "5" + + version: "6" update progress of schema change job #1: "PostCommitNonRevertiblePhase stage 2 of 2 with 2 MutationType ops pending" -commit transaction #10 -begin transaction #11 +set schema change job #1 to non-cancellable +commit transaction #9 +begin transaction #10 ## PostCommitNonRevertiblePhase stage 2 of 2 with 4 MutationType ops upsert descriptor #104 ... @@ -694,7 +673,7 @@ upsert descriptor #104 - money version: 4 - modificationTime: - - wallTime: "1640995200000000010" + - wallTime: "1640995200000000009" - mutations: - - direction: DROP - index: @@ -737,12 +716,12 @@ upsert descriptor #104 ... time: {} unexposedParentSchemaId: 101 - - version: "7" - + version: "8" + - version: "6" + + version: "7" write *eventpb.FinishSchemaChange to event log for descriptor 104 create job #2 (non-cancelable: true): "GC for " descriptor IDs: [104] update progress of schema change job #1: "all stages completed" -commit transaction #11 +commit transaction #10 notified job registry to adopt jobs: [2] # end PostCommitPhase diff --git a/pkg/cmd/dev/build.go b/pkg/cmd/dev/build.go index 490905f61e79..b2b9c1e1cd77 100644 --- a/pkg/cmd/dev/build.go +++ b/pkg/cmd/dev/build.go @@ -99,6 +99,9 @@ var buildTargetMapping = map[string]string{ "roachprod-stress": "//pkg/cmd/roachprod-stress:roachprod-stress", "roachtest": "//pkg/cmd/roachtest:roachtest", "short": "//pkg/cmd/cockroach-short:cockroach-short", + "smith": "//pkg/cmd/smith:smith", + "smithcmp": "//pkg/cmd/smithcmp:smithcmp", + "smithtest": "//pkg/cmd/smithtest:smithtest", "staticcheck": "@co_honnef_go_tools//cmd/staticcheck:staticcheck", "stress": stressTarget, "swagger": "@com_github_go_swagger_go_swagger//cmd/swagger:swagger", diff --git a/pkg/cmd/roachtest/tests/query_comparison_util.go b/pkg/cmd/roachtest/tests/query_comparison_util.go index 3de1198e8819..eb0e386b4d50 100644 --- a/pkg/cmd/roachtest/tests/query_comparison_util.go +++ b/pkg/cmd/roachtest/tests/query_comparison_util.go @@ -177,7 +177,7 @@ func runOneRoundQueryComparison( // Initialize a smither that generates only deterministic SELECT statements. smither, err := sqlsmith.NewSmither(conn, rnd, - sqlsmith.DisableMutations(), sqlsmith.DisableImpureFns(), sqlsmith.DisableLimits(), + sqlsmith.DisableMutations(), sqlsmith.DisableNondeterministicFns(), sqlsmith.DisableLimits(), sqlsmith.UnlikelyConstantPredicate(), sqlsmith.FavorCommonData(), sqlsmith.UnlikelyRandomNulls(), sqlsmith.DisableCrossJoins(), sqlsmith.DisableIndexHints(), sqlsmith.DisableWith(), diff --git a/pkg/cmd/roachtest/tests/tlp.go b/pkg/cmd/roachtest/tests/tlp.go index 78966bcc02e0..530f8e82e61f 100644 --- a/pkg/cmd/roachtest/tests/tlp.go +++ b/pkg/cmd/roachtest/tests/tlp.go @@ -147,7 +147,7 @@ func runOneTLP( // Initialize a smither that will never generate mutations. tlpSmither, err := sqlsmith.NewSmither(conn, rnd, - sqlsmith.DisableMutations(), sqlsmith.DisableImpureFns()) + sqlsmith.DisableMutations(), sqlsmith.DisableNondeterministicFns()) if err != nil { t.Fatal(err) } diff --git a/pkg/cmd/roachtest/tests/tpch_concurrency.go b/pkg/cmd/roachtest/tests/tpch_concurrency.go index 1eb8e8d79b09..02a8b2ddc6e9 100644 --- a/pkg/cmd/roachtest/tests/tpch_concurrency.go +++ b/pkg/cmd/roachtest/tests/tpch_concurrency.go @@ -13,7 +13,6 @@ package tests import ( "context" "fmt" - "strings" "time" "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/cluster" @@ -107,22 +106,6 @@ func registerTPCHConcurrency(r registry.Registry) { // Run each query once on each connection. for queryNum := 1; queryNum <= tpch.NumQueries; queryNum++ { t.Status("running Q", queryNum) - // To aid during the debugging later, we'll print the DistSQL - // diagram of the query. - rows, err := conn.Query("EXPLAIN (DISTSQL) " + tpch.QueriesByNumber[queryNum]) - if err != nil { - return err - } - defer rows.Close() - for rows.Next() { - var line string - if err = rows.Scan(&line); err != nil { - t.Fatal(err) - } - if strings.Contains(line, "Diagram:") { - t.Status(line) - } - } // The way --max-ops flag works is as follows: the global ops // counter is incremented **after** each worker completes a // single operation, so it is possible for all connections start diff --git a/pkg/cmd/smith/BUILD.bazel b/pkg/cmd/smith/BUILD.bazel new file mode 100644 index 000000000000..9654da21e193 --- /dev/null +++ b/pkg/cmd/smith/BUILD.bazel @@ -0,0 +1,22 @@ +load("//build/bazelutil/unused_checker:unused.bzl", "get_x_data") +load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library") + +go_library( + name = "smith_lib", + srcs = ["main.go"], + importpath = "github.com/cockroachdb/cockroach/pkg/cmd/smith", + visibility = ["//visibility:private"], + deps = [ + "//pkg/internal/sqlsmith", + "//pkg/util/randutil", + "@com_github_cockroachdb_errors//:errors", + ], +) + +go_binary( + name = "smith", + embed = [":smith_lib"], + visibility = ["//visibility:public"], +) + +get_x_data(name = "get_x_data") diff --git a/pkg/cmd/smith/main.go b/pkg/cmd/smith/main.go new file mode 100644 index 000000000000..da9bde36628d --- /dev/null +++ b/pkg/cmd/smith/main.go @@ -0,0 +1,149 @@ +// Copyright 2022 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package main + +import ( + gosql "database/sql" + "flag" + "fmt" + "os" + "sort" + + "github.com/cockroachdb/cockroach/pkg/internal/sqlsmith" + "github.com/cockroachdb/cockroach/pkg/util/randutil" + "github.com/cockroachdb/errors" +) + +// smith is a command-line wrapper around sqlsmith-go, useful for examining +// sqlsmith output when making changes to the random query generator. + +const description = ` +%[1]s prints random SQL statements to stdout, using a random SQL statement +generator inspired by SQLsmith. See https://github.com/anse1/sqlsmith for more +about the SQLsmith project. + +Usage: + [COCKROACH_RANDOM_SEED=1234] %[1]s [options] [sqlsmith-go options] + +Options: +` + +var ( + flags = flag.NewFlagSet(os.Args[0], flag.ContinueOnError) + expr = flags.Bool("expr", false, "generate expressions instead of statements") + num = flags.Int("num", 1, "number of statements / expressions to generate") + url = flags.String("url", "", "database to fetch schema from") + smitherOptMap = map[string]sqlsmith.SmitherOption{ + "DisableMutations": sqlsmith.DisableMutations(), + "DisableDDLs": sqlsmith.DisableDDLs(), + "OnlyNoDropDDLs": sqlsmith.OnlyNoDropDDLs(), + "MultiRegionDDLs": sqlsmith.MultiRegionDDLs(), + "DisableWith": sqlsmith.DisableWith(), + "DisableNondeterministicFns": sqlsmith.DisableNondeterministicFns(), + "DisableCRDBFns": sqlsmith.DisableCRDBFns(), + "SimpleDatums": sqlsmith.SimpleDatums(), + "MutationsOnly": sqlsmith.MutationsOnly(), + "InsUpdOnly": sqlsmith.InsUpdOnly(), + "DisableLimits": sqlsmith.DisableLimits(), + "AvoidConsts": sqlsmith.AvoidConsts(), + "DisableWindowFuncs": sqlsmith.DisableWindowFuncs(), + "OutputSort": sqlsmith.OutputSort(), + "UnlikelyConstantPredicate": sqlsmith.UnlikelyConstantPredicate(), + "FavorCommonData": sqlsmith.FavorCommonData(), + "UnlikelyRandomNulls": sqlsmith.UnlikelyRandomNulls(), + "DisableCrossJoins": sqlsmith.DisableCrossJoins(), + "DisableIndexHints": sqlsmith.DisableIndexHints(), + "LowProbabilityWhereClauseWithJoinTables": sqlsmith.LowProbabilityWhereClauseWithJoinTables(), + "DisableInsertSelect": sqlsmith.DisableInsertSelect(), + "CompareMode": sqlsmith.CompareMode(), + "PostgresMode": sqlsmith.PostgresMode(), + "MutatingMode": sqlsmith.MutatingMode(), + } + smitherOpts []string +) + +func usage() { + fmt.Fprintf(flags.Output(), description, os.Args[0]) + flags.PrintDefaults() + fmt.Fprint(flags.Output(), "\nSqlsmith-go options:\n") + for _, opt := range smitherOpts { + fmt.Fprintln(flags.Output(), " ", opt) + } +} + +func init() { + smitherOpts = make([]string, 0, len(smitherOptMap)) + for opt := range smitherOptMap { + smitherOpts = append(smitherOpts, opt) + } + sort.Strings(smitherOpts) + flags.Usage = usage +} + +func main() { + if err := flags.Parse(os.Args[1:]); err != nil { + if errors.Is(err, flag.ErrHelp) { + os.Exit(0) + } else { + os.Exit(1) + } + } + + rng, seed := randutil.NewPseudoRand() + fmt.Print("-- COCKROACH_RANDOM_SEED=", seed, "\n") + + var smitherOpts []sqlsmith.SmitherOption + for _, arg := range flags.Args() { + if opt, ok := smitherOptMap[arg]; ok { + fmt.Print("-- ", arg, ": ", opt, "\n") + smitherOpts = append(smitherOpts, opt) + } else { + fmt.Fprintf(flags.Output(), "unrecognized sqlsmith-go option: %v\n", arg) + usage() + os.Exit(2) + } + } + + var db *gosql.DB + if *url != "" { + var err error + db, err = gosql.Open("postgres", *url) + if err != nil { + fmt.Fprintf(flags.Output(), "could not connect to database\n") + os.Exit(3) + } + defer db.Close() + if err := db.Ping(); err != nil { + fmt.Fprintf(flags.Output(), "could not ping database\n") + os.Exit(4) + } + fmt.Println("-- connected to", *url) + } + + smither, err := sqlsmith.NewSmither(db, rng, smitherOpts...) + if err != nil { + fmt.Println(err) + os.Exit(1) + } + defer smither.Close() + + fmt.Println("-- num", *num) + if *expr { + fmt.Println("-- expr") + for i := 0; i < *num; i++ { + fmt.Print("\n", smither.GenerateExpr(), "\n") + } + } else { + for i := 0; i < *num; i++ { + fmt.Print("\n", smither.Generate(), ";\n") + } + } +} diff --git a/pkg/internal/sqlsmith/scalar.go b/pkg/internal/sqlsmith/scalar.go index 13e0ada6b03e..08fc5c77c604 100644 --- a/pkg/internal/sqlsmith/scalar.go +++ b/pkg/internal/sqlsmith/scalar.go @@ -401,7 +401,7 @@ func makeFunc(s *Smither, ctx Context, typ *types.T, refs colRefs) (tree.TypedEx return nil, false } fn := fns[s.rnd.Intn(len(fns))] - if s.disableImpureFns && fn.overload.Volatility > volatility.Immutable { + if s.disableNondeterministicFns && fn.overload.Volatility > volatility.Immutable { return nil, false } for _, ignore := range s.ignoreFNs { @@ -410,6 +410,9 @@ func makeFunc(s *Smither, ctx Context, typ *types.T, refs colRefs) (tree.TypedEx } } + // Some aggregation functions benefit from an order by clause. + var orderExpr tree.Expr + args := make(tree.TypedExprs, 0) for _, argTyp := range fn.overload.Types.Types() { // Postgres is picky about having Int4 arguments instead of Int8. @@ -421,6 +424,9 @@ func makeFunc(s *Smither, ctx Context, typ *types.T, refs colRefs) (tree.TypedEx if class == tree.AggregateClass || class == tree.WindowClass { var ok bool arg, ok = makeColRef(s, argTyp, refs) + if ok && len(args) == 0 { + orderExpr = arg + } if !ok { // If we can't find a col ref for our aggregate function, just use a // constant. @@ -469,9 +475,7 @@ func makeFunc(s *Smither, ctx Context, typ *types.T, refs colRefs) (tree.TypedEx } } - // Cast the return and arguments to prevent ambiguity during function - // implementation choosing. - return castType(tree.NewTypedFuncExpr( + funcExpr := tree.NewTypedFuncExpr( tree.ResolvableFunctionReference{FunctionReference: fn.def}, 0, /* aggQualifier */ args, @@ -480,7 +484,33 @@ func makeFunc(s *Smither, ctx Context, typ *types.T, refs colRefs) (tree.TypedEx typ, &fn.def.FunctionProperties, fn.overload, - ), typ), true + ) + + // Some aggregation functions need an order by clause to be deterministic. + if s.disableNondeterministicFns { + switch fn.def.Name { + case "array_agg", + "concat_agg", + "json_agg", + "json_object_agg", + "jsonb_agg", + "jsonb_object_agg", + "st_makeline", + "string_agg", + "xmlagg": + if orderExpr != nil { + funcExpr.AggType = tree.GeneralAgg + funcExpr.OrderBy = tree.OrderBy{{ + Expr: orderExpr, + Direction: s.randDirection(), + }} + } + } + } + + // Cast the return and arguments to prevent ambiguity during function + // implementation choosing. + return castType(funcExpr, typ), true } var windowFrameModes = []treewindow.WindowFrameMode{ diff --git a/pkg/internal/sqlsmith/schema.go b/pkg/internal/sqlsmith/schema.go index e3f8471e84ea..a090e95d5e92 100644 --- a/pkg/internal/sqlsmith/schema.go +++ b/pkg/internal/sqlsmith/schema.go @@ -54,6 +54,7 @@ type tableRefs []*tableRef // ReloadSchemas loads tables from the database. func (s *Smither) ReloadSchemas() error { if s.db == nil { + s.schemas = []*schemaRef{{SchemaName: "public"}} return nil } s.lock.Lock() diff --git a/pkg/internal/sqlsmith/sqlsmith.go b/pkg/internal/sqlsmith/sqlsmith.go index f01c7f265f18..fa8eceed510f 100644 --- a/pkg/internal/sqlsmith/sqlsmith.go +++ b/pkg/internal/sqlsmith/sqlsmith.go @@ -79,7 +79,7 @@ type Smither struct { scalarExprSampler, boolExprSampler *scalarExprSampler disableWith bool - disableImpureFns bool + disableNondeterministicFns bool disableLimits bool disableWindowFuncs bool simpleDatums bool @@ -138,9 +138,11 @@ func NewSmither(db *gosql.DB, rnd *rand.Rand, opts ...SmitherOption) (*Smither, s.scalarExprSampler = newWeightedScalarExprSampler(s.scalarExprWeights, rnd.Int63()) s.boolExprSampler = newWeightedScalarExprSampler(s.boolExprWeights, rnd.Int63()) s.enableBulkIO() - row := s.db.QueryRow("SELECT current_database()") - if err := row.Scan(&s.dbName); err != nil { - return nil, err + if s.db != nil { + row := s.db.QueryRow("SELECT current_database()") + if err := row.Scan(&s.dbName); err != nil { + return nil, err + } } return s, s.ReloadSchemas() } @@ -316,9 +318,9 @@ var DisableWith = simpleOption("disable WITH", func(s *Smither) { s.disableWith = true }) -// DisableImpureFns causes the Smither to disable impure functions. -var DisableImpureFns = simpleOption("disable impure funcs", func(s *Smither) { - s.disableImpureFns = true +// DisableNondeterministicFns causes the Smither to disable nondeterministic functions. +var DisableNondeterministicFns = simpleOption("disable nondeterministic funcs", func(s *Smither) { + s.disableNondeterministicFns = true }) // DisableCRDBFns causes the Smither to disable crdb_internal functions. @@ -433,7 +435,7 @@ var DisableInsertSelect = simpleOption("disable insert select", func(s *Smither) var CompareMode = multiOption( "compare mode", DisableMutations(), - DisableImpureFns(), + DisableNondeterministicFns(), DisableCRDBFns(), IgnoreFNs("^version"), DisableLimits(), @@ -475,3 +477,17 @@ var PostgresMode = multiOption( IgnoreFNs("^postgis_.*version"), IgnoreFNs("^postgis_.*scripts"), ) + +// MutatingMode causes the Smither to generate mutation statements in the same +// way as the query-comparison roachtests (costfuzz and +// unoptimized-query-oracle). +var MutatingMode = multiOption( + "mutating mode", + MutationsOnly(), + FavorCommonData(), + UnlikelyRandomNulls(), + DisableInsertSelect(), + DisableCrossJoins(), + SetComplexity(.05), + SetScalarComplexity(.01), +) diff --git a/pkg/sql/colexec/colbuilder/execplan.go b/pkg/sql/colexec/colbuilder/execplan.go index b28c1504bed3..b560fcfb5dff 100644 --- a/pkg/sql/colexec/colbuilder/execplan.go +++ b/pkg/sql/colexec/colbuilder/execplan.go @@ -436,6 +436,7 @@ func (r opResult) createDiskBackedSort( args.DiskQueueCfg, args.FDSemaphore, diskAccount, + flowCtx.TestingKnobs().VecFDsToAcquire, ) r.ToClose = append(r.ToClose, es.(colexecop.Closer)) return es diff --git a/pkg/sql/colexec/colexecdisk/external_sort.go b/pkg/sql/colexec/colexecdisk/external_sort.go index af250af7b7a2..728cce659880 100644 --- a/pkg/sql/colexec/colexecdisk/external_sort.go +++ b/pkg/sql/colexec/colexecdisk/external_sort.go @@ -232,6 +232,7 @@ func NewExternalSorter( diskQueueCfg colcontainer.DiskQueueCfg, fdSemaphore semaphore.Semaphore, diskAcc *mon.BoundAccount, + testingVecFDsToAcquire int, ) colexecop.Operator { if diskQueueCfg.BufferSizeBytes > 0 && maxNumberPartitions == 0 { // With the default limit of 256 file descriptors, this results in 16 @@ -243,6 +244,9 @@ func NewExternalSorter( if maxNumberPartitions < colexecop.ExternalSorterMinPartitions { maxNumberPartitions = colexecop.ExternalSorterMinPartitions } + if testingVecFDsToAcquire > 0 { + maxNumberPartitions = testingVecFDsToAcquire + } if memoryLimit == 1 { // If memory limit is 1, we're likely in a "force disk spill" // scenario, but we don't want to artificially limit batches when we diff --git a/pkg/sql/colexec/colexecdisk/hash_based_partitioner.go b/pkg/sql/colexec/colexecdisk/hash_based_partitioner.go index 57236774916c..6b4d75515b66 100644 --- a/pkg/sql/colexec/colexecdisk/hash_based_partitioner.go +++ b/pkg/sql/colexec/colexecdisk/hash_based_partitioner.go @@ -308,6 +308,9 @@ func calculateMaxNumberActivePartitions( if maxNumberActivePartitions < numRequiredActivePartitions { maxNumberActivePartitions = numRequiredActivePartitions } + if toAcquire := flowCtx.TestingKnobs().VecFDsToAcquire; toAcquire > 0 { + maxNumberActivePartitions = toAcquire + } return maxNumberActivePartitions } diff --git a/pkg/sql/colflow/BUILD.bazel b/pkg/sql/colflow/BUILD.bazel index 00a8d4cba875..3bd030a39075 100644 --- a/pkg/sql/colflow/BUILD.bazel +++ b/pkg/sql/colflow/BUILD.bazel @@ -37,6 +37,8 @@ go_library( "//pkg/sql/execinfrapb", "//pkg/sql/execstats", "//pkg/sql/flowinfra", + "//pkg/sql/pgwire/pgcode", + "//pkg/sql/pgwire/pgerror", "//pkg/sql/rowenc", "//pkg/sql/rowexec", "//pkg/sql/sessiondatapb", @@ -49,6 +51,7 @@ go_library( "//pkg/util/mon", "//pkg/util/optional", "//pkg/util/randutil", + "//pkg/util/retry", "//pkg/util/syncutil", "//pkg/util/timeutil", "//pkg/util/tracing", @@ -70,6 +73,7 @@ go_test( "main_test.go", "routers_test.go", "stats_test.go", + "vectorized_flow_deadlock_test.go", "vectorized_flow_planning_test.go", "vectorized_flow_shutdown_test.go", "vectorized_flow_space_test.go", @@ -121,6 +125,8 @@ go_test( "//pkg/testutils/testcluster", "//pkg/util/admission", "//pkg/util/buildutil", + "//pkg/util/cancelchecker", + "//pkg/util/envutil", "//pkg/util/hlc", "//pkg/util/humanizeutil", "//pkg/util/leaktest", diff --git a/pkg/sql/colflow/vectorized_flow.go b/pkg/sql/colflow/vectorized_flow.go index 0d9439ae8596..5afcd810b241 100644 --- a/pkg/sql/colflow/vectorized_flow.go +++ b/pkg/sql/colflow/vectorized_flow.go @@ -37,6 +37,8 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/execinfrapb" "github.com/cockroachdb/cockroach/pkg/sql/execstats" "github.com/cockroachdb/cockroach/pkg/sql/flowinfra" + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" "github.com/cockroachdb/cockroach/pkg/sql/rowexec" "github.com/cockroachdb/cockroach/pkg/sql/sessiondatapb" "github.com/cockroachdb/cockroach/pkg/sql/types" @@ -47,6 +49,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/util/metric" "github.com/cockroachdb/cockroach/pkg/util/mon" "github.com/cockroachdb/cockroach/pkg/util/optional" + "github.com/cockroachdb/cockroach/pkg/util/retry" "github.com/cockroachdb/cockroach/pkg/util/syncutil" "github.com/cockroachdb/cockroach/pkg/util/timeutil" "github.com/cockroachdb/errors" @@ -55,39 +58,84 @@ import ( "github.com/marusama/semaphore" ) -// countingSemaphore is a semaphore that keeps track of the semaphore count from -// its perspective. -// Note that it effectively implements the execinfra.Releasable interface but -// due to the method name conflict doesn't. -type countingSemaphore struct { +// fdCountingSemaphore is a semaphore that keeps track of the number of file +// descriptors currently used by the vectorized engine. +// +// Note that it effectively implements the execreleasable.Releasable interface +// but due to the method name conflict doesn't. +type fdCountingSemaphore struct { semaphore.Semaphore - globalCount *metric.Gauge - count int64 + globalCount *metric.Gauge + count int64 + testingAcquireMaxRetries int } -var countingSemaphorePool = sync.Pool{ +var fdCountingSemaphorePool = sync.Pool{ New: func() interface{} { - return &countingSemaphore{} + return &fdCountingSemaphore{} }, } -func newCountingSemaphore(sem semaphore.Semaphore, globalCount *metric.Gauge) *countingSemaphore { - s := countingSemaphorePool.Get().(*countingSemaphore) +func newFDCountingSemaphore( + sem semaphore.Semaphore, globalCount *metric.Gauge, testingAcquireMaxRetries int, +) *fdCountingSemaphore { + s := fdCountingSemaphorePool.Get().(*fdCountingSemaphore) s.Semaphore = sem s.globalCount = globalCount + s.testingAcquireMaxRetries = testingAcquireMaxRetries return s } -func (s *countingSemaphore) Acquire(ctx context.Context, n int) error { - if err := s.Semaphore.Acquire(ctx, n); err != nil { - return err +var errAcquireTimeout = pgerror.New( + pgcode.ConfigurationLimitExceeded, + "acquiring of file descriptors timed out, consider increasing "+ + "COCKROACH_VEC_MAX_OPEN_FDS environment variable", +) + +func (s *fdCountingSemaphore) Acquire(ctx context.Context, n int) error { + if s.TryAcquire(n) { + return nil } - atomic.AddInt64(&s.count, int64(n)) - s.globalCount.Inc(int64(n)) - return nil + // Currently there is not enough capacity in the semaphore to acquire the + // desired number, so we set up a retry loop that exponentially backs off, + // until either the semaphore opens up or we time out (most likely due to a + // deadlock). + // + // The latter situation is possible when multiple queries already hold some + // of the quota and each of them needs more to proceed resulting in a + // deadlock. We get out of such a deadlock by randomly erroring out one of + // the queries (which would release some quota back to the semaphore) making + // it possible for other queries to proceed. + // + // Note that we've already tried to acquire the quota above (which failed), + // so the initial backoff time of 100ms seems ok (we are spilling to disk + // after all, so the query is likely to experience significant latency). The + // current choice of options is such that we'll spend on the order of 25s + // in the retry loop before timing out. + maxRetries := s.testingAcquireMaxRetries + if maxRetries <= 0 { + // Make sure that the retry loop is finite. + maxRetries = 8 + } + opts := retry.Options{ + InitialBackoff: 100 * time.Millisecond, + Multiplier: 2.0, + RandomizationFactor: 0.25, + MaxRetries: maxRetries, + } + for r := retry.StartWithCtx(ctx, opts); r.Next(); { + if s.TryAcquire(n) { + return nil + } + } + if ctx.Err() != nil { + return ctx.Err() + } + log.Warning(ctx, "acquiring of file descriptors for disk-spilling timed out") + return errAcquireTimeout } -func (s *countingSemaphore) TryAcquire(n int) bool { +func (s *fdCountingSemaphore) TryAcquire(n int) bool { success := s.Semaphore.TryAcquire(n) if !success { return false @@ -97,7 +145,7 @@ func (s *countingSemaphore) TryAcquire(n int) bool { return success } -func (s *countingSemaphore) Release(n int) int { +func (s *fdCountingSemaphore) Release(n int) int { atomic.AddInt64(&s.count, int64(-n)) s.globalCount.Dec(int64(n)) return s.Semaphore.Release(n) @@ -106,12 +154,12 @@ func (s *countingSemaphore) Release(n int) int { // ReleaseToPool should be named Release and should implement the // execinfra.Releasable interface, but that would lead to a conflict with // semaphore.Semaphore.Release method. -func (s *countingSemaphore) ReleaseToPool() { +func (s *fdCountingSemaphore) ReleaseToPool() { if unreleased := atomic.LoadInt64(&s.count); unreleased != 0 { colexecerror.InternalError(errors.Newf("unexpectedly %d count on the semaphore when releasing it to the pool", unreleased)) } - *s = countingSemaphore{} - countingSemaphorePool.Put(s) + *s = fdCountingSemaphore{} + fdCountingSemaphorePool.Put(s) } type vectorizedFlow struct { @@ -130,7 +178,7 @@ type vectorizedFlow struct { // of the number of resources held in a semaphore.Semaphore requested from the // context of this flow so that these can be released unconditionally upon // Cleanup. - countingSemaphore *countingSemaphore + countingSemaphore *fdCountingSemaphore tempStorage struct { syncutil.Mutex @@ -198,8 +246,11 @@ func (f *vectorizedFlow) Setup( if err := diskQueueCfg.EnsureDefaults(); err != nil { return ctx, nil, err } - f.countingSemaphore = newCountingSemaphore(f.Cfg.VecFDSemaphore, f.Cfg.Metrics.VecOpenFDs) flowCtx := f.GetFlowCtx() + f.countingSemaphore = newFDCountingSemaphore( + f.Cfg.VecFDSemaphore, f.Cfg.Metrics.VecOpenFDs, + flowCtx.TestingKnobs().VecFDsAcquireMaxRetriesCount, + ) f.creator = newVectorizedFlowCreator( helper, vectorizedRemoteComponentCreator{}, diff --git a/pkg/sql/colflow/vectorized_flow_deadlock_test.go b/pkg/sql/colflow/vectorized_flow_deadlock_test.go new file mode 100644 index 000000000000..c823f50dc856 --- /dev/null +++ b/pkg/sql/colflow/vectorized_flow_deadlock_test.go @@ -0,0 +1,77 @@ +// Copyright 2022 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package colflow_test + +import ( + "context" + "strconv" + "strings" + "testing" + "time" + + "github.com/cockroachdb/cockroach/pkg/base" + "github.com/cockroachdb/cockroach/pkg/sql/execinfra" + "github.com/cockroachdb/cockroach/pkg/testutils/skip" + "github.com/cockroachdb/cockroach/pkg/testutils/testcluster" + "github.com/cockroachdb/cockroach/pkg/util/cancelchecker" + "github.com/cockroachdb/cockroach/pkg/util/envutil" + "github.com/cockroachdb/cockroach/pkg/util/leaktest" + "github.com/cockroachdb/cockroach/pkg/util/log" + "github.com/cockroachdb/cockroach/pkg/util/timeutil" + "github.com/stretchr/testify/require" +) + +// TestVectorizedFlowDeadlocksWhenSpilling is a regression test for the +// vectorized flow being deadlocked when multiple operators have to spill to +// disk exhausting the file descriptor limit. +func TestVectorizedFlowDeadlocksWhenSpilling(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + + skip.UnderStressRace(t, "the test is too slow under stressrace") + + vecFDsLimit := 8 + envutil.TestSetEnv(t, "COCKROACH_VEC_MAX_OPEN_FDS", strconv.Itoa(vecFDsLimit)) + serverArgs := base.TestServerArgs{ + Knobs: base.TestingKnobs{DistSQL: &execinfra.TestingKnobs{ + // Set the testing knob so that the first operator to spill would + // use up the whole FD limit. + VecFDsToAcquire: vecFDsLimit, + // Allow just one retry to speed up the test. + VecFDsAcquireMaxRetriesCount: 1, + }}, + } + tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{ServerArgs: serverArgs}) + ctx := context.Background() + defer tc.Stopper().Stop(ctx) + conn := tc.Conns[0] + + _, err := conn.ExecContext(ctx, "CREATE TABLE t (a, b) AS SELECT i, i FROM generate_series(1, 10000) AS g(i)") + require.NoError(t, err) + // Lower the workmem budget so that all buffering operators have to spill to + // disk. + _, err = conn.ExecContext(ctx, "SET distsql_workmem = '1KiB'") + require.NoError(t, err) + + queryCtx, queryCtxCancel := context.WithDeadline(ctx, timeutil.Now().Add(10*time.Second)) + defer queryCtxCancel() + // Run a query with a hash joiner feeding into a hash aggregator, with both + // operators spilling to disk. We expect that the hash aggregator won't be + // able to spill though since the FD limit has been used up, and we'd like + // to see the query timing out (when acquiring the file descriptor quota) + // rather than being canceled due to the context deadline. + query := "SELECT max(a) FROM (SELECT t1.a, t1.b FROM t AS t1 INNER HASH JOIN t AS t2 ON t1.a = t2.b) GROUP BY b" + _, err = conn.ExecContext(queryCtx, query) + // We expect an error that is different from the query cancellation (which + // is what SQL layer returns on a context cancellation). + require.NotNil(t, err) + require.False(t, strings.Contains(err.Error(), cancelchecker.QueryCanceledError.Error())) +} diff --git a/pkg/sql/execinfra/server_config.go b/pkg/sql/execinfra/server_config.go index 85bc8384f937..3c7009864a64 100644 --- a/pkg/sql/execinfra/server_config.go +++ b/pkg/sql/execinfra/server_config.go @@ -244,6 +244,15 @@ type TestingKnobs struct { // Cannot be set together with ForceDiskSpill. MemoryLimitBytes int64 + // VecFDsToAcquire, if positive, indicates the number of file descriptors + // that should be acquired by a single disk-spilling operator in the + // vectorized engine. + VecFDsToAcquire int + // VecFDsAcquireMaxRetriesCount, if positive, determines the maximum number + // of retries done when acquiring the file descriptors for a disk-spilling + // operator in the vectorized engine. + VecFDsAcquireMaxRetriesCount int + // TableReaderBatchBytesLimit, if not 0, overrides the limit that the // TableReader will set on the size of results it wants to get for individual // requests. diff --git a/pkg/sql/logictest/testdata/logic_test/information_schema b/pkg/sql/logictest/testdata/logic_test/information_schema index 234310929302..a326ecb4f0eb 100644 --- a/pkg/sql/logictest/testdata/logic_test/information_schema +++ b/pkg/sql/logictest/testdata/logic_test/information_schema @@ -4657,7 +4657,7 @@ integer_datetimes on intervalstyle postgres intervalstyle_enabled on is_superuser on -join_reader_ordering_strategy_batch_size 10 KiB +join_reader_ordering_strategy_batch_size 100 KiB large_full_scan_rows 1000 lc_collate C.UTF-8 lc_ctype C.UTF-8 diff --git a/pkg/sql/logictest/testdata/logic_test/new_schema_changer b/pkg/sql/logictest/testdata/logic_test/new_schema_changer index fd59d000e2fe..75256b622207 100644 --- a/pkg/sql/logictest/testdata/logic_test/new_schema_changer +++ b/pkg/sql/logictest/testdata/logic_test/new_schema_changer @@ -91,7 +91,7 @@ EXPLAIN (DDL, VERBOSE) ALTER TABLE foo ADD COLUMN j INT │ │ oid: 20 │ │ width: 64 │ │ -│ └── • scop.AddColumnToIndex +│ └── • AddColumnToIndex │ ColumnID: 2 │ IndexID: 1 │ Kind: 2 @@ -174,6 +174,7 @@ EXPLAIN (DDL, VERBOSE) ALTER TABLE foo ADD COLUMN j INT JobID: 1 RunningStatus: all stages completed + statement ok ALTER TABLE foo ADD COLUMN j INT @@ -1969,8 +1970,8 @@ Schema change plan for DROP INDEX ‹test›.public.‹parent›@‹idx› CASCA │ ├── RemoveAllTableComments {"TableID":266} │ ├── MakeDroppedIndexDeleteOnly {"IndexID":2,"TableID":265} │ ├── DrainDescriptorName {"Namespace":{"DatabaseID":104,"DescriptorID":266,"Name":"child","SchemaID":105}} - │ ├── scop.RemoveColumnFromIndex {"ColumnID":2,"IndexID":2,"TableID":265} - │ ├── scop.RemoveColumnFromIndex {"ColumnID":1,"IndexID":2,"Kind":1,"TableID":265} + │ ├── RemoveColumnFromIndex {"ColumnID":2,"IndexID":2,"TableID":265} + │ ├── RemoveColumnFromIndex {"ColumnID":1,"IndexID":2,"Kind":1,"TableID":265} │ ├── SetJobStateOnDescriptor {"DescriptorID":265} │ ├── SetJobStateOnDescriptor {"DescriptorID":266} │ └── UpdateSchemaChangerJob {"IsNonCancelable":true,"RunningStatus":"PostCommitNonRev..."} @@ -1994,7 +1995,6 @@ Schema change plan for DROP INDEX ‹test›.public.‹parent›@‹idx› CASCA ├── RemoveJobStateFromDescriptor {"DescriptorID":266} └── UpdateSchemaChangerJob {"IsNonCancelable":true,"RunningStatus":"all stages compl..."} - statement ok SET use_declarative_schema_changer = 'on' diff --git a/pkg/sql/logictest/testdata/logic_test/pg_catalog b/pkg/sql/logictest/testdata/logic_test/pg_catalog index a8cd5c37fd6d..cb9bcd9f2580 100644 --- a/pkg/sql/logictest/testdata/logic_test/pg_catalog +++ b/pkg/sql/logictest/testdata/logic_test/pg_catalog @@ -4186,7 +4186,7 @@ inject_retry_errors_enabled off NULL integer_datetimes on NULL NULL NULL string intervalstyle postgres NULL NULL NULL string is_superuser on NULL NULL NULL string -join_reader_ordering_strategy_batch_size 10 KiB NULL NULL NULL string +join_reader_ordering_strategy_batch_size 100 KiB NULL NULL NULL string large_full_scan_rows 1000 NULL NULL NULL string lc_collate C.UTF-8 NULL NULL NULL string lc_ctype C.UTF-8 NULL NULL NULL string @@ -4310,7 +4310,7 @@ inject_retry_errors_enabled off NULL integer_datetimes on NULL user NULL on on intervalstyle postgres NULL user NULL postgres postgres is_superuser on NULL user NULL on on -join_reader_ordering_strategy_batch_size 10 KiB NULL user NULL 10 KiB 10 KiB +join_reader_ordering_strategy_batch_size 100 KiB NULL user NULL 100 KiB 100 KiB large_full_scan_rows 1000 NULL user NULL 1000 1000 lc_collate C.UTF-8 NULL user NULL C.UTF-8 C.UTF-8 lc_ctype C.UTF-8 NULL user NULL C.UTF-8 C.UTF-8 diff --git a/pkg/sql/logictest/testdata/logic_test/show_source b/pkg/sql/logictest/testdata/logic_test/show_source index 9fd11d1270e9..e044c5b6f14b 100644 --- a/pkg/sql/logictest/testdata/logic_test/show_source +++ b/pkg/sql/logictest/testdata/logic_test/show_source @@ -78,7 +78,7 @@ inject_retry_errors_enabled off integer_datetimes on intervalstyle postgres is_superuser on -join_reader_ordering_strategy_batch_size 10 KiB +join_reader_ordering_strategy_batch_size 100 KiB large_full_scan_rows 1000 lc_collate C.UTF-8 lc_ctype C.UTF-8 diff --git a/pkg/sql/rowexec/joinreader.go b/pkg/sql/rowexec/joinreader.go index b7f21e06c2e8..c95434ae76d9 100644 --- a/pkg/sql/rowexec/joinreader.go +++ b/pkg/sql/rowexec/joinreader.go @@ -1047,9 +1047,28 @@ func (jr *joinReader) performLookup() (joinReaderState, *execinfrapb.ProducerMet log.VEvent(jr.Ctx, 1, "done joining rows") jr.strategy.prepareToEmit(jr.Ctx) + // Check if the strategy spilled to disk and reduce the batch size if it + // did. + // TODO(yuzefovich): we should probably also grow the batch size bytes limit + // dynamically if we haven't spilled and are not close to spilling (say not + // exceeding half of the memory limit of the disk-backed container), up to + // some limit. (This would only apply to the joinReaderOrderingStrategy + // since other strategies cannot spill in the first place.) Probably it'd be + // good to look at not just the current batch of input rows, but to keep + // some statistics over the last several batches to make a more informed + // decision. + if jr.strategy.spilled() && jr.batchSizeBytes > joinReaderMinBatchSize { + jr.batchSizeBytes = jr.batchSizeBytes / 2 + if jr.batchSizeBytes < joinReaderMinBatchSize { + jr.batchSizeBytes = joinReaderMinBatchSize + } + } + return jrEmittingRows, nil } +const joinReaderMinBatchSize = 10 << 10 /* 10 KiB */ + // emitRow returns the next row from jr.toEmit, if present. Otherwise it // prepares for another input batch. func (jr *joinReader) emitRow() ( diff --git a/pkg/sql/rowexec/joinreader_strategies.go b/pkg/sql/rowexec/joinreader_strategies.go index e497fcbe73d6..36fde25c1455 100644 --- a/pkg/sql/rowexec/joinreader_strategies.go +++ b/pkg/sql/rowexec/joinreader_strategies.go @@ -451,7 +451,7 @@ var partialJoinSentinel = []int{-1} // // Say the joinReader looks up rows in order: (red, x), then (blue, y). Once // (red, x) is fetched, it is handed to -// joinReaderOderingStrategy.processLookedUpRow(), which will match it against +// joinReaderOrderingStrategy.processLookedUpRow(), which will match it against // all the corresponding input rows, producing (1, x), (4, x). These two rows // are not emitted because that would violate the input ordering (well, (1, x) // could be emitted, but we're not smart enough). So, they are buffered until @@ -535,7 +535,7 @@ type joinReaderOrderingStrategy struct { testingInfoSpilled bool } -const joinReaderOrderingStrategyBatchSizeDefault = 10 << 10 /* 10 KiB */ +const joinReaderOrderingStrategyBatchSizeDefault = 100 << 10 /* 100 KiB */ // JoinReaderOrderingStrategyBatchSize determines the size of input batches used // to construct a single lookup KV batch by joinReaderOrderingStrategy. @@ -548,8 +548,6 @@ var JoinReaderOrderingStrategyBatchSize = settings.RegisterByteSizeSetting( ) func (s *joinReaderOrderingStrategy) getLookupRowsBatchSizeHint(sd *sessiondata.SessionData) int64 { - // TODO(asubiotto): Eventually we might want to adjust this batch size - // dynamically based on whether the result row container spilled or not. if sd.JoinReaderOrderingStrategyBatchSize == 0 { // In some tests the session data might not be set - use the default // value then. diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/op_funcs.go b/pkg/sql/schemachanger/scplan/internal/opgen/op_funcs.go index 742acfc2b8ce..a102fdf63b6d 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/op_funcs.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/op_funcs.go @@ -100,12 +100,19 @@ func makeTargetsWithElementMap(cs scpb.CurrentState) targetsWithElementMap { // given an element value. type opsFunc func(element scpb.Element, md *targetsWithElementMap) []scop.Op -func makeOpsFunc(el scpb.Element, fns []interface{}) (opsFunc, error) { +func makeOpsFunc(el scpb.Element, fns []interface{}) (opsFunc, scop.Type, error) { + var opType scop.Type var funcValues []reflect.Value for _, fn := range fns { - if err := checkOpFunc(el, fn); err != nil { - return nil, err + typ, err := checkOpFunc(el, fn) + if err != nil { + return nil, 0, err } + if len(funcValues) > 0 && typ != opType { + return nil, 0, errors.Errorf("conflicting operation types for %T: %s != %s", + el, opType, typ) + } + opType = typ funcValues = append(funcValues, reflect.ValueOf(fn)) } return func(element scpb.Element, md *targetsWithElementMap) []scop.Op { @@ -124,16 +131,21 @@ func makeOpsFunc(el scpb.Element, fns []interface{}) (opsFunc, error) { } } return ret - }, nil + }, opType, nil } -var opType = reflect.TypeOf((*scop.Op)(nil)).Elem() +var ( + opInterfaceType = reflect.TypeOf((*scop.Op)(nil)).Elem() + mutationOpInterfaceType = reflect.TypeOf((*scop.MutationOp)(nil)).Elem() + validationOpInterfaceType = reflect.TypeOf((*scop.ValidationOp)(nil)).Elem() + backfillOpInterfaceType = reflect.TypeOf((*scop.BackfillOp)(nil)).Elem() +) -func checkOpFunc(el scpb.Element, fn interface{}) error { +func checkOpFunc(el scpb.Element, fn interface{}) (opType scop.Type, _ error) { fnV := reflect.ValueOf(fn) fnT := fnV.Type() if fnT.Kind() != reflect.Func { - return errors.Errorf( + return 0, errors.Errorf( "%v is a %s, expected %s", fnT, fnT.Kind(), reflect.Func, ) } @@ -141,14 +153,33 @@ func checkOpFunc(el scpb.Element, fn interface{}) error { if !(fnT.NumIn() == 1 && fnT.In(0) == elType) && !(fnT.NumIn() == 2 && fnT.In(0) == elType && fnT.In(1) == reflect.TypeOf((*targetsWithElementMap)(nil))) { - return errors.Errorf( + return 0, errors.Errorf( "expected %v to be a func with one argument of type %s", fnT, elType, ) } - if fnT.NumOut() != 1 || !fnT.Out(0).Implements(opType) { + returnTypeError := func() error { return errors.Errorf( - "expected %v to be a func with one return value of type %s", fnT, opType, + "expected %v to be a func with one return value of a "+ + "pointer type which implements %s", fnT, opType, ) } - return nil + if fnT.NumOut() != 1 { + return 0, returnTypeError() + } + out := fnT.Out(0) + if out.Kind() != reflect.Ptr || !out.Implements(opInterfaceType) { + return 0, returnTypeError() + } + switch { + case out.Implements(mutationOpInterfaceType): + opType = scop.MutationType + case out.Implements(validationOpInterfaceType): + opType = scop.ValidationType + case out.Implements(backfillOpInterfaceType): + opType = scop.BackfillType + default: + return 0, errors.AssertionFailedf("%s implemented %s but does not conform to any known type", + out, opInterfaceType) + } + return opType, nil } diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/op_gen.go b/pkg/sql/schemachanger/scplan/internal/opgen/op_gen.go index df3b82111fb5..688022d636d0 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/op_gen.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/op_gen.go @@ -78,7 +78,7 @@ func (r *registry) buildGraph(cs scpb.CurrentState) (_ *scgraph.Graph, err error ops = e.ops(e.n.Element(), &md) } if err := g.AddOpEdges( - e.n.Target, e.from, e.to, e.revertible, e.minPhase, ops..., + e.n.Target, e.from, e.to, e.revertible, e.canFail, e.minPhase, ops..., ); err != nil { return nil, err } diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_alias_type.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_alias_type.go index 216a8d499cc0..da1f1441887f 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_alias_type.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_alias_type.go @@ -21,12 +21,12 @@ func init() { scpb.Status_ABSENT, equiv(scpb.Status_DROPPED), to(scpb.Status_OFFLINE, - emit(func(this *scpb.AliasType) scop.Op { + emit(func(this *scpb.AliasType) *scop.NotImplemented { return notImplemented(this) }), ), to(scpb.Status_PUBLIC, - emit(func(this *scpb.AliasType) scop.Op { + emit(func(this *scpb.AliasType) *scop.MarkDescriptorAsPublic { return &scop.MarkDescriptorAsPublic{ DescID: this.TypeID, } @@ -36,7 +36,7 @@ func init() { toAbsent( scpb.Status_PUBLIC, to(scpb.Status_OFFLINE, - emit(func(this *scpb.AliasType, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.AliasType, md *targetsWithElementMap) *scop.MarkDescriptorAsOffline { return &scop.MarkDescriptorAsOffline{ DescID: this.TypeID, Reason: statementForDropJob(this, md).Statement, @@ -45,17 +45,17 @@ func init() { ), to(scpb.Status_DROPPED, revertible(false), - emit(func(this *scpb.AliasType) scop.Op { + emit(func(this *scpb.AliasType) *scop.MarkDescriptorAsDropped { return &scop.MarkDescriptorAsDropped{ DescID: this.TypeID, } }), ), to(scpb.Status_ABSENT, - emit(func(this *scpb.AliasType, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.AliasType, md *targetsWithElementMap) *scop.LogEvent { return newLogEventOp(this, md) }), - emit(func(this *scpb.AliasType) scop.Op { + emit(func(this *scpb.AliasType) *scop.DeleteDescriptor { return &scop.DeleteDescriptor{ DescriptorID: this.TypeID, } diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_check_constraint.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_check_constraint.go index 037b86148b43..df2104452e0c 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_check_constraint.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_check_constraint.go @@ -20,7 +20,7 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_PUBLIC, - emit(func(this *scpb.CheckConstraint) scop.Op { + emit(func(this *scpb.CheckConstraint) *scop.NotImplemented { return notImplemented(this) }), ), @@ -30,13 +30,13 @@ func init() { to(scpb.Status_ABSENT, // TODO(postamar): remove revertibility constraint when possible revertible(false), - emit(func(this *scpb.CheckConstraint) scop.Op { + emit(func(this *scpb.CheckConstraint) *scop.RemoveCheckConstraint { return &scop.RemoveCheckConstraint{ TableID: this.TableID, ConstraintID: this.ConstraintID, } }), - emit(func(this *scpb.CheckConstraint) scop.Op { + emit(func(this *scpb.CheckConstraint) *scop.UpdateTableBackReferencesInTypes { if len(this.UsesTypeIDs) == 0 { return nil } @@ -45,7 +45,7 @@ func init() { BackReferencedTableID: this.TableID, } }), - emit(func(this *scpb.CheckConstraint) scop.Op { + emit(func(this *scpb.CheckConstraint) *scop.UpdateBackReferencesInSequences { if len(this.UsesSequenceIDs) == 0 { return nil } diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_column.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_column.go index e5e6bc89afb7..a44b411b3361 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_column.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_column.go @@ -21,17 +21,17 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_DELETE_ONLY, - emit(func(this *scpb.Column) scop.Op { + emit(func(this *scpb.Column) *scop.MakeAddedColumnDeleteOnly { return &scop.MakeAddedColumnDeleteOnly{ Column: *protoutil.Clone(this).(*scpb.Column), } }), - emit(func(this *scpb.Column, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.Column, md *targetsWithElementMap) *scop.LogEvent { return newLogEventOp(this, md) }), ), to(scpb.Status_WRITE_ONLY, - emit(func(this *scpb.Column) scop.Op { + emit(func(this *scpb.Column) *scop.MakeAddedColumnDeleteAndWriteOnly { return &scop.MakeAddedColumnDeleteAndWriteOnly{ TableID: this.TableID, ColumnID: this.ColumnID, @@ -39,14 +39,14 @@ func init() { }), ), to(scpb.Status_PUBLIC, - emit(func(this *scpb.Column, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.Column, md *targetsWithElementMap) *scop.MakeColumnPublic { return &scop.MakeColumnPublic{ EventBase: newLogEventBase(this, md), TableID: this.TableID, ColumnID: this.ColumnID, } }), - emit(func(this *scpb.Column) scop.Op { + emit(func(this *scpb.Column) *scop.RefreshStats { return &scop.RefreshStats{ TableID: this.TableID, } @@ -56,19 +56,19 @@ func init() { toAbsent( scpb.Status_PUBLIC, to(scpb.Status_WRITE_ONLY, - emit(func(this *scpb.Column) scop.Op { + emit(func(this *scpb.Column) *scop.MakeDroppedColumnDeleteAndWriteOnly { return &scop.MakeDroppedColumnDeleteAndWriteOnly{ TableID: this.TableID, ColumnID: this.ColumnID, } }), - emit(func(this *scpb.Column, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.Column, md *targetsWithElementMap) *scop.LogEvent { return newLogEventOp(this, md) }), ), to(scpb.Status_DELETE_ONLY, revertible(false), - emit(func(this *scpb.Column) scop.Op { + emit(func(this *scpb.Column) *scop.MakeDroppedColumnDeleteOnly { return &scop.MakeDroppedColumnDeleteOnly{ TableID: this.TableID, ColumnID: this.ColumnID, @@ -76,7 +76,7 @@ func init() { }), ), to(scpb.Status_ABSENT, - emit(func(this *scpb.Column, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.Column, md *targetsWithElementMap) *scop.MakeColumnAbsent { return &scop.MakeColumnAbsent{ EventBase: newLogEventBase(this, md), TableID: this.TableID, diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_column_comment.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_column_comment.go index 2809793f63a5..6b2e9d73fe03 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_column_comment.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_column_comment.go @@ -20,7 +20,7 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_PUBLIC, - emit(func(this *scpb.ColumnComment) scop.Op { + emit(func(this *scpb.ColumnComment) *scop.UpsertColumnComment { return &scop.UpsertColumnComment{ TableID: this.TableID, ColumnID: this.ColumnID, @@ -28,7 +28,7 @@ func init() { PGAttributeNum: this.PgAttributeNum, } }), - emit(func(this *scpb.ColumnComment, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.ColumnComment, md *targetsWithElementMap) *scop.LogEvent { return newLogEventOp(this, md) }), ), @@ -36,14 +36,14 @@ func init() { toAbsent( scpb.Status_PUBLIC, to(scpb.Status_ABSENT, - emit(func(this *scpb.ColumnComment) scop.Op { + emit(func(this *scpb.ColumnComment) *scop.RemoveColumnComment { return &scop.RemoveColumnComment{ TableID: this.TableID, ColumnID: this.ColumnID, PgAttributeNum: this.PgAttributeNum, } }), - emit(func(this *scpb.ColumnComment, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.ColumnComment, md *targetsWithElementMap) *scop.LogEvent { return newLogEventOp(this, md) }), ), diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_column_default_expression.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_column_default_expression.go index 9fe938ed97a6..679e864acd44 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_column_default_expression.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_column_default_expression.go @@ -21,12 +21,12 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_PUBLIC, - emit(func(this *scpb.ColumnDefaultExpression) scop.Op { + emit(func(this *scpb.ColumnDefaultExpression) *scop.AddColumnDefaultExpression { return &scop.AddColumnDefaultExpression{ Default: *protoutil.Clone(this).(*scpb.ColumnDefaultExpression), } }), - emit(func(this *scpb.ColumnDefaultExpression) scop.Op { + emit(func(this *scpb.ColumnDefaultExpression) *scop.UpdateTableBackReferencesInTypes { if len(this.UsesTypeIDs) == 0 { return nil } @@ -35,7 +35,7 @@ func init() { BackReferencedTableID: this.TableID, } }), - emit(func(this *scpb.ColumnDefaultExpression) scop.Op { + emit(func(this *scpb.ColumnDefaultExpression) *scop.UpdateBackReferencesInSequences { if len(this.UsesSequenceIDs) == 0 { return nil } @@ -50,13 +50,13 @@ func init() { toAbsent( scpb.Status_PUBLIC, to(scpb.Status_ABSENT, - emit(func(this *scpb.ColumnDefaultExpression) scop.Op { + emit(func(this *scpb.ColumnDefaultExpression) *scop.RemoveColumnDefaultExpression { return &scop.RemoveColumnDefaultExpression{ TableID: this.TableID, ColumnID: this.ColumnID, } }), - emit(func(this *scpb.ColumnDefaultExpression) scop.Op { + emit(func(this *scpb.ColumnDefaultExpression) *scop.UpdateTableBackReferencesInTypes { if len(this.UsesTypeIDs) == 0 { return nil } @@ -65,7 +65,7 @@ func init() { BackReferencedTableID: this.TableID, } }), - emit(func(this *scpb.ColumnDefaultExpression) scop.Op { + emit(func(this *scpb.ColumnDefaultExpression) *scop.UpdateBackReferencesInSequences { if len(this.UsesSequenceIDs) == 0 { return nil } diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_column_family.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_column_family.go index e9535ef2e42e..252ad04259e9 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_column_family.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_column_family.go @@ -21,7 +21,7 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_PUBLIC, - emit(func(this *scpb.ColumnFamily) scop.Op { + emit(func(this *scpb.ColumnFamily) *scop.AddColumnFamily { return &scop.AddColumnFamily{ TableID: this.TableID, FamilyID: this.FamilyID, @@ -34,7 +34,7 @@ func init() { scpb.Status_PUBLIC, to(scpb.Status_ABSENT, revertible(false), - emit(func(this *scpb.ColumnFamily) scop.Op { + emit(func(this *scpb.ColumnFamily) *scop.NotImplemented { return notImplemented(this) }), ), diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_column_name.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_column_name.go index 5c0eeb440a90..a7097dd33ccf 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_column_name.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_column_name.go @@ -22,7 +22,7 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_PUBLIC, - emit(func(this *scpb.ColumnName) scop.Op { + emit(func(this *scpb.ColumnName) *scop.SetColumnName { return &scop.SetColumnName{ TableID: this.TableID, ColumnID: this.ColumnID, @@ -34,7 +34,7 @@ func init() { toAbsent( scpb.Status_PUBLIC, to(scpb.Status_ABSENT, - emit(func(this *scpb.ColumnName) scop.Op { + emit(func(this *scpb.ColumnName) *scop.SetColumnName { return &scop.SetColumnName{ TableID: this.TableID, ColumnID: this.ColumnID, diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_column_on_update_expression.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_column_on_update_expression.go index 5642a7b6f6cb..942de9d2e8ad 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_column_on_update_expression.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_column_on_update_expression.go @@ -21,12 +21,12 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_PUBLIC, - emit(func(this *scpb.ColumnOnUpdateExpression) scop.Op { + emit(func(this *scpb.ColumnOnUpdateExpression) *scop.AddColumnOnUpdateExpression { return &scop.AddColumnOnUpdateExpression{ OnUpdate: *protoutil.Clone(this).(*scpb.ColumnOnUpdateExpression), } }), - emit(func(this *scpb.ColumnOnUpdateExpression) scop.Op { + emit(func(this *scpb.ColumnOnUpdateExpression) *scop.UpdateTableBackReferencesInTypes { if len(this.UsesTypeIDs) == 0 { return nil } @@ -35,7 +35,7 @@ func init() { BackReferencedTableID: this.TableID, } }), - emit(func(this *scpb.ColumnOnUpdateExpression) scop.Op { + emit(func(this *scpb.ColumnOnUpdateExpression) *scop.UpdateBackReferencesInSequences { if len(this.UsesSequenceIDs) == 0 { return nil } @@ -50,13 +50,13 @@ func init() { toAbsent( scpb.Status_PUBLIC, to(scpb.Status_ABSENT, - emit(func(this *scpb.ColumnOnUpdateExpression) scop.Op { + emit(func(this *scpb.ColumnOnUpdateExpression) *scop.RemoveColumnOnUpdateExpression { return &scop.RemoveColumnOnUpdateExpression{ TableID: this.TableID, ColumnID: this.ColumnID, } }), - emit(func(this *scpb.ColumnOnUpdateExpression) scop.Op { + emit(func(this *scpb.ColumnOnUpdateExpression) *scop.UpdateTableBackReferencesInTypes { if len(this.UsesTypeIDs) == 0 { return nil } @@ -65,7 +65,7 @@ func init() { BackReferencedTableID: this.TableID, } }), - emit(func(this *scpb.ColumnOnUpdateExpression) scop.Op { + emit(func(this *scpb.ColumnOnUpdateExpression) *scop.UpdateBackReferencesInSequences { if len(this.UsesSequenceIDs) == 0 { return nil } diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_column_type.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_column_type.go index 10b078ae0781..6bdd332ee3cc 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_column_type.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_column_type.go @@ -23,12 +23,12 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_PUBLIC, - emit(func(this *scpb.ColumnType) scop.Op { + emit(func(this *scpb.ColumnType) *scop.SetAddedColumnType { return &scop.SetAddedColumnType{ ColumnType: *protoutil.Clone(this).(*scpb.ColumnType), } }), - emit(func(this *scpb.ColumnType) scop.Op { + emit(func(this *scpb.ColumnType) *scop.UpdateTableBackReferencesInTypes { if ids := referencedTypeIDs(this); len(ids) > 0 { return &scop.UpdateTableBackReferencesInTypes{ TypeIDs: ids, @@ -43,7 +43,7 @@ func init() { scpb.Status_PUBLIC, to(scpb.Status_ABSENT, revertible(false), - emit(func(this *scpb.ColumnType) scop.Op { + emit(func(this *scpb.ColumnType) *scop.RemoveDroppedColumnType { if ids := referencedTypeIDs(this); len(ids) > 0 { return &scop.RemoveDroppedColumnType{ TableID: this.TableID, @@ -52,7 +52,7 @@ func init() { } return nil }), - emit(func(this *scpb.ColumnType) scop.Op { + emit(func(this *scpb.ColumnType) *scop.UpdateTableBackReferencesInTypes { if ids := referencedTypeIDs(this); len(ids) > 0 { return &scop.UpdateTableBackReferencesInTypes{ TypeIDs: ids, diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_constraint_comment.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_constraint_comment.go index d4320c36b864..af35a029dccb 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_constraint_comment.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_constraint_comment.go @@ -20,14 +20,14 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_PUBLIC, - emit(func(this *scpb.ConstraintComment) scop.Op { + emit(func(this *scpb.ConstraintComment) *scop.UpsertConstraintComment { return &scop.UpsertConstraintComment{ TableID: this.TableID, ConstraintID: this.ConstraintID, Comment: this.Comment, } }), - emit(func(this *scpb.ConstraintComment, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.ConstraintComment, md *targetsWithElementMap) *scop.LogEvent { return newLogEventOp(this, md) }), ), @@ -35,13 +35,13 @@ func init() { toAbsent( scpb.Status_PUBLIC, to(scpb.Status_ABSENT, - emit(func(this *scpb.ConstraintComment) scop.Op { + emit(func(this *scpb.ConstraintComment) *scop.RemoveConstraintComment { return &scop.RemoveConstraintComment{ TableID: this.TableID, ConstraintID: this.ConstraintID, } }), - emit(func(this *scpb.ConstraintComment, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.ConstraintComment, md *targetsWithElementMap) *scop.LogEvent { return newLogEventOp(this, md) }), ), diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_constraint_name.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_constraint_name.go index ed152b7af3cd..fe806801ea37 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_constraint_name.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_constraint_name.go @@ -20,7 +20,7 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_PUBLIC, - emit(func(this *scpb.ConstraintName) scop.Op { + emit(func(this *scpb.ConstraintName) *scop.NotImplemented { return notImplemented(this) }), ), @@ -30,7 +30,7 @@ func init() { to(scpb.Status_ABSENT, // TODO(postamar): remove revertibility constraint when possible revertible(false), - emit(func(this *scpb.ConstraintName) scop.Op { + emit(func(this *scpb.ConstraintName) *scop.NotImplemented { return notImplemented(this) }), ), diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_database.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_database.go index 24267f2ef24e..a5e92248ca10 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_database.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_database.go @@ -21,12 +21,12 @@ func init() { scpb.Status_ABSENT, equiv(scpb.Status_DROPPED), to(scpb.Status_OFFLINE, - emit(func(this *scpb.Database) scop.Op { + emit(func(this *scpb.Database) *scop.NotImplemented { return notImplemented(this) }), ), to(scpb.Status_PUBLIC, - emit(func(this *scpb.Database) scop.Op { + emit(func(this *scpb.Database) *scop.MarkDescriptorAsPublic { return &scop.MarkDescriptorAsPublic{ DescID: this.DatabaseID, } @@ -36,7 +36,7 @@ func init() { toAbsent( scpb.Status_PUBLIC, to(scpb.Status_OFFLINE, - emit(func(this *scpb.Database, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.Database, md *targetsWithElementMap) *scop.MarkDescriptorAsOffline { return &scop.MarkDescriptorAsOffline{ DescID: this.DatabaseID, Reason: statementForDropJob(this, md).Statement, @@ -45,23 +45,23 @@ func init() { ), to(scpb.Status_DROPPED, revertible(false), - emit(func(this *scpb.Database) scop.Op { + emit(func(this *scpb.Database) *scop.MarkDescriptorAsDropped { return &scop.MarkDescriptorAsDropped{ DescID: this.DatabaseID, } }), ), to(scpb.Status_ABSENT, - emit(func(this *scpb.Database, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.Database, md *targetsWithElementMap) *scop.LogEvent { return newLogEventOp(this, md) }), - emit(func(this *scpb.Database, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.Database, md *targetsWithElementMap) *scop.CreateGcJobForDatabase { return &scop.CreateGcJobForDatabase{ DatabaseID: this.DatabaseID, StatementForDropJob: statementForDropJob(this, md), } }), - emit(func(this *scpb.Database) scop.Op { + emit(func(this *scpb.Database) *scop.DeleteDescriptor { return &scop.DeleteDescriptor{ DescriptorID: this.DatabaseID, } diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_database_comment.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_database_comment.go index 05aafbd1dc33..de73a1fc46b1 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_database_comment.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_database_comment.go @@ -20,13 +20,13 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_PUBLIC, - emit(func(this *scpb.DatabaseComment) scop.Op { + emit(func(this *scpb.DatabaseComment) *scop.UpsertDatabaseComment { return &scop.UpsertDatabaseComment{ DatabaseID: this.DatabaseID, Comment: this.Comment, } }), - emit(func(this *scpb.DatabaseComment, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.DatabaseComment, md *targetsWithElementMap) *scop.LogEvent { return newLogEventOp(this, md) }), ), @@ -34,12 +34,12 @@ func init() { toAbsent( scpb.Status_PUBLIC, to(scpb.Status_ABSENT, - emit(func(this *scpb.DatabaseComment) scop.Op { + emit(func(this *scpb.DatabaseComment) *scop.RemoveDatabaseComment { return &scop.RemoveDatabaseComment{ DatabaseID: this.DatabaseID, } }), - emit(func(this *scpb.DatabaseComment, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.DatabaseComment, md *targetsWithElementMap) *scop.LogEvent { return newLogEventOp(this, md) }), ), diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_database_role_setting.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_database_role_setting.go index 6941e3629e0f..3244dd55e7ca 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_database_role_setting.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_database_role_setting.go @@ -20,7 +20,7 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_PUBLIC, - emit(func(this *scpb.DatabaseRoleSetting) scop.Op { + emit(func(this *scpb.DatabaseRoleSetting) *scop.NotImplemented { return notImplemented(this) }), ), @@ -30,7 +30,7 @@ func init() { to(scpb.Status_ABSENT, // TODO(postamar): remove revertibility constraint when possible revertible(false), - emit(func(this *scpb.DatabaseRoleSetting) scop.Op { + emit(func(this *scpb.DatabaseRoleSetting) *scop.RemoveDatabaseRoleSettings { return &scop.RemoveDatabaseRoleSettings{ DatabaseID: this.DatabaseID, } diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_enum_type.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_enum_type.go index b3ece53f8620..d749d40be3e6 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_enum_type.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_enum_type.go @@ -21,12 +21,12 @@ func init() { scpb.Status_ABSENT, equiv(scpb.Status_DROPPED), to(scpb.Status_OFFLINE, - emit(func(this *scpb.EnumType) scop.Op { + emit(func(this *scpb.EnumType) *scop.NotImplemented { return notImplemented(this) }), ), to(scpb.Status_PUBLIC, - emit(func(this *scpb.EnumType) scop.Op { + emit(func(this *scpb.EnumType) *scop.MarkDescriptorAsPublic { return &scop.MarkDescriptorAsPublic{ DescID: this.TypeID, } @@ -36,7 +36,7 @@ func init() { toAbsent( scpb.Status_PUBLIC, to(scpb.Status_OFFLINE, - emit(func(this *scpb.EnumType, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.EnumType, md *targetsWithElementMap) *scop.MarkDescriptorAsOffline { return &scop.MarkDescriptorAsOffline{ DescID: this.TypeID, Reason: statementForDropJob(this, md).Statement, @@ -45,17 +45,17 @@ func init() { ), to(scpb.Status_DROPPED, revertible(false), - emit(func(this *scpb.EnumType) scop.Op { + emit(func(this *scpb.EnumType) *scop.MarkDescriptorAsDropped { return &scop.MarkDescriptorAsDropped{ DescID: this.TypeID, } }), ), to(scpb.Status_ABSENT, - emit(func(this *scpb.EnumType, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.EnumType, md *targetsWithElementMap) *scop.LogEvent { return newLogEventOp(this, md) }), - emit(func(this *scpb.EnumType) scop.Op { + emit(func(this *scpb.EnumType) *scop.DeleteDescriptor { return &scop.DeleteDescriptor{ DescriptorID: this.TypeID, } diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_enum_type_value.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_enum_type_value.go index 3f6124da8745..cea682e4060b 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_enum_type_value.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_enum_type_value.go @@ -20,7 +20,7 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_PUBLIC, - emit(func(this *scpb.EnumTypeValue) scop.Op { + emit(func(this *scpb.EnumTypeValue) *scop.NotImplemented { return notImplemented(this) }), ), @@ -28,7 +28,7 @@ func init() { toAbsent( scpb.Status_PUBLIC, to(scpb.Status_ABSENT, - emit(func(this *scpb.EnumTypeValue) scop.Op { + emit(func(this *scpb.EnumTypeValue) *scop.NotImplemented { return notImplemented(this) }), ), diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_foreign_key_constraint.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_foreign_key_constraint.go index 288295dd8f66..fa71fd44bfe1 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_foreign_key_constraint.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_foreign_key_constraint.go @@ -20,7 +20,7 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_PUBLIC, - emit(func(this *scpb.ForeignKeyConstraint) scop.Op { + emit(func(this *scpb.ForeignKeyConstraint) *scop.NotImplemented { return notImplemented(this) }), ), @@ -35,14 +35,14 @@ func init() { // table do not match. We therefore have to identify the constraint // by name in the origin table descriptor to be able to remove the // back-reference. - emit(func(this *scpb.ForeignKeyConstraint) scop.Op { + emit(func(this *scpb.ForeignKeyConstraint) *scop.RemoveForeignKeyBackReference { return &scop.RemoveForeignKeyBackReference{ ReferencedTableID: this.ReferencedTableID, OriginTableID: this.TableID, OriginConstraintID: this.ConstraintID, } }), - emit(func(this *scpb.ForeignKeyConstraint) scop.Op { + emit(func(this *scpb.ForeignKeyConstraint) *scop.RemoveForeignKeyConstraint { return &scop.RemoveForeignKeyConstraint{ TableID: this.TableID, ConstraintID: this.ConstraintID, diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_index_column.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_index_column.go index 9a9a4e309650..0ae827350eaf 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_index_column.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_index_column.go @@ -19,8 +19,8 @@ func init() { opRegistry.register((*scpb.IndexColumn)(nil), toPublic( scpb.Status_ABSENT, - to(scpb.Status_PUBLIC, emit(func(column *scpb.IndexColumn) scop.Op { - return scop.AddColumnToIndex{ + to(scpb.Status_PUBLIC, emit(func(column *scpb.IndexColumn) *scop.AddColumnToIndex { + return &scop.AddColumnToIndex{ TableID: column.TableID, ColumnID: column.ColumnID, IndexID: column.IndexID, @@ -34,8 +34,8 @@ func init() { scpb.Status_PUBLIC, to(scpb.Status_ABSENT, revertible(false), - emit(func(column *scpb.IndexColumn) scop.Op { - return scop.RemoveColumnFromIndex{ + emit(func(column *scpb.IndexColumn) *scop.RemoveColumnFromIndex { + return &scop.RemoveColumnFromIndex{ TableID: column.TableID, ColumnID: column.ColumnID, IndexID: column.IndexID, diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_index_comment.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_index_comment.go index 0ec0eda4fa31..e186f52abe8a 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_index_comment.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_index_comment.go @@ -20,14 +20,14 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_PUBLIC, - emit(func(this *scpb.IndexComment) scop.Op { + emit(func(this *scpb.IndexComment) *scop.UpsertIndexComment { return &scop.UpsertIndexComment{ TableID: this.TableID, IndexID: this.IndexID, Comment: this.Comment, } }), - emit(func(this *scpb.IndexComment, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.IndexComment, md *targetsWithElementMap) *scop.LogEvent { return newLogEventOp(this, md) }), ), @@ -35,13 +35,13 @@ func init() { toAbsent( scpb.Status_PUBLIC, to(scpb.Status_ABSENT, - emit(func(this *scpb.IndexComment) scop.Op { + emit(func(this *scpb.IndexComment) *scop.RemoveIndexComment { return &scop.RemoveIndexComment{ TableID: this.TableID, IndexID: this.IndexID, } }), - emit(func(this *scpb.IndexComment, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.IndexComment, md *targetsWithElementMap) *scop.LogEvent { return newLogEventOp(this, md) }), ), diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_index_name.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_index_name.go index 264bca5aaa59..01326f3c95af 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_index_name.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_index_name.go @@ -22,7 +22,7 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_PUBLIC, - emit(func(this *scpb.IndexName) scop.Op { + emit(func(this *scpb.IndexName) *scop.SetIndexName { return &scop.SetIndexName{ TableID: this.TableID, IndexID: this.IndexID, @@ -34,7 +34,7 @@ func init() { toAbsent( scpb.Status_PUBLIC, to(scpb.Status_ABSENT, - emit(func(this *scpb.IndexName) scop.Op { + emit(func(this *scpb.IndexName) *scop.SetIndexName { return &scop.SetIndexName{ TableID: this.TableID, IndexID: this.IndexID, diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_index_partitioning.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_index_partitioning.go index e3fc17ed90f0..bea597fbe76b 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_index_partitioning.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_index_partitioning.go @@ -31,7 +31,7 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_PUBLIC, - emit(func(this *scpb.IndexPartitioning) scop.Op { + emit(func(this *scpb.IndexPartitioning) *scop.AddIndexPartitionInfo { return &scop.AddIndexPartitionInfo{ Partitioning: *protoutil.Clone(this).(*scpb.IndexPartitioning), } diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_namespace.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_namespace.go index 806edf6f9571..cc3731332cdc 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_namespace.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_namespace.go @@ -22,7 +22,7 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_PUBLIC, - emit(func(this *scpb.Namespace) scop.Op { + emit(func(this *scpb.Namespace) *scop.NotImplemented { return notImplemented(this) }), ), @@ -32,7 +32,7 @@ func init() { to(scpb.Status_ABSENT, // TODO(postamar): remove revertibility constraint when possible revertible(false), - emit(func(this *scpb.Namespace) scop.Op { + emit(func(this *scpb.Namespace) *scop.DrainDescriptorName { return &scop.DrainDescriptorName{ Namespace: *protoutil.Clone(this).(*scpb.Namespace), } 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 40defb9882c8..781b54d6fc7c 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_object_parent.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_object_parent.go @@ -20,7 +20,7 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_PUBLIC, - emit(func(this *scpb.ObjectParent) scop.Op { + emit(func(this *scpb.ObjectParent) *scop.NotImplemented { return notImplemented(this) }), ), diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_owner.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_owner.go index 0fa6f14f777d..aeee0aefb3c8 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_owner.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_owner.go @@ -20,7 +20,7 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_PUBLIC, - emit(func(this *scpb.Owner) scop.Op { + emit(func(this *scpb.Owner) *scop.NotImplemented { return notImplemented(this) }), ), @@ -30,7 +30,7 @@ func init() { to(scpb.Status_ABSENT, // TODO(postamar): remove revertibility constraint when possible revertible(false), - emit(func(this *scpb.Owner) scop.Op { + emit(func(this *scpb.Owner) *scop.NotImplemented { return notImplemented(this) }), ), diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_primary_index.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_primary_index.go index aea3bbcbbab7..d46620e4e798 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_primary_index.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_primary_index.go @@ -21,14 +21,14 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_BACKFILL_ONLY, - emit(func(this *scpb.PrimaryIndex) scop.Op { + emit(func(this *scpb.PrimaryIndex) *scop.MakeAddedIndexBackfilling { return &scop.MakeAddedIndexBackfilling{ Index: *protoutil.Clone(&this.Index).(*scpb.Index), } }), ), to(scpb.Status_BACKFILLED, - emit(func(this *scpb.PrimaryIndex) scop.Op { + emit(func(this *scpb.PrimaryIndex) *scop.BackfillIndex { return &scop.BackfillIndex{ TableID: this.TableID, SourceIndexID: this.SourceIndexID, @@ -37,7 +37,7 @@ func init() { }), ), to(scpb.Status_DELETE_ONLY, - emit(func(this *scpb.PrimaryIndex) scop.Op { + emit(func(this *scpb.PrimaryIndex) *scop.MakeBackfillingIndexDeleteOnly { return &scop.MakeBackfillingIndexDeleteOnly{ TableID: this.TableID, IndexID: this.IndexID, @@ -45,7 +45,7 @@ func init() { }), ), to(scpb.Status_MERGE_ONLY, - emit(func(this *scpb.PrimaryIndex) scop.Op { + emit(func(this *scpb.PrimaryIndex) *scop.MakeAddedIndexDeleteAndWriteOnly { return &scop.MakeAddedIndexDeleteAndWriteOnly{ TableID: this.TableID, IndexID: this.IndexID, @@ -54,7 +54,7 @@ func init() { ), equiv(scpb.Status_WRITE_ONLY), to(scpb.Status_MERGED, - emit(func(this *scpb.PrimaryIndex) scop.Op { + emit(func(this *scpb.PrimaryIndex) *scop.MergeIndex { return &scop.MergeIndex{ TableID: this.TableID, TemporaryIndexID: this.TemporaryIndexID, @@ -63,7 +63,7 @@ func init() { }), ), to(scpb.Status_VALIDATED, - emit(func(this *scpb.PrimaryIndex) scop.Op { + emit(func(this *scpb.PrimaryIndex) *scop.ValidateUniqueIndex { return &scop.ValidateUniqueIndex{ TableID: this.TableID, IndexID: this.IndexID, @@ -71,7 +71,7 @@ func init() { }), ), to(scpb.Status_PUBLIC, - emit(func(this *scpb.PrimaryIndex, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.PrimaryIndex, md *targetsWithElementMap) *scop.MakeAddedPrimaryIndexPublic { return &scop.MakeAddedPrimaryIndexPublic{ EventBase: newLogEventBase(this, md), TableID: this.TableID, @@ -83,7 +83,7 @@ func init() { toAbsent( scpb.Status_PUBLIC, to(scpb.Status_VALIDATED, - emit(func(this *scpb.PrimaryIndex) scop.Op { + emit(func(this *scpb.PrimaryIndex) *scop.MakeDroppedPrimaryIndexDeleteAndWriteOnly { // Most of this logic is taken from MakeMutationComplete(). return &scop.MakeDroppedPrimaryIndexDeleteAndWriteOnly{ TableID: this.TableID, @@ -97,7 +97,7 @@ func init() { equiv(scpb.Status_MERGE_ONLY), equiv(scpb.Status_MERGED), to(scpb.Status_DELETE_ONLY, - emit(func(this *scpb.PrimaryIndex) scop.Op { + emit(func(this *scpb.PrimaryIndex) *scop.MakeDroppedIndexDeleteOnly { return &scop.MakeDroppedIndexDeleteOnly{ TableID: this.TableID, IndexID: this.IndexID, @@ -107,14 +107,14 @@ func init() { equiv(scpb.Status_BACKFILLED), equiv(scpb.Status_BACKFILL_ONLY), to(scpb.Status_ABSENT, - emit(func(this *scpb.PrimaryIndex, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.PrimaryIndex, md *targetsWithElementMap) *scop.CreateGcJobForIndex { return &scop.CreateGcJobForIndex{ TableID: this.TableID, IndexID: this.IndexID, StatementForDropJob: statementForDropJob(this, md), } }), - emit(func(this *scpb.PrimaryIndex, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.PrimaryIndex, md *targetsWithElementMap) *scop.MakeIndexAbsent { return &scop.MakeIndexAbsent{ EventBase: newLogEventBase(this, md), TableID: this.TableID, diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_row_level_ttl.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_row_level_ttl.go index 8406ccb26fdc..9a44509898f0 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_row_level_ttl.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_row_level_ttl.go @@ -20,7 +20,7 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_PUBLIC, - emit(func(this *scpb.RowLevelTTL) scop.Op { + emit(func(this *scpb.RowLevelTTL) *scop.NotImplemented { return notImplemented(this) }), ), @@ -30,7 +30,7 @@ func init() { to(scpb.Status_ABSENT, // TODO(postamar): remove revertibility constraint when possible revertible(false), - emit(func(this *scpb.RowLevelTTL) scop.Op { + emit(func(this *scpb.RowLevelTTL) *scop.DeleteSchedule { return &scop.DeleteSchedule{ ScheduleID: this.RowLevelTTL.ScheduleID, } diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_schema.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_schema.go index bece3c58a79a..c864bd95bb2d 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_schema.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_schema.go @@ -21,12 +21,12 @@ func init() { scpb.Status_ABSENT, equiv(scpb.Status_DROPPED), to(scpb.Status_OFFLINE, - emit(func(this *scpb.Schema) scop.Op { + emit(func(this *scpb.Schema) *scop.NotImplemented { return notImplemented(this) }), ), to(scpb.Status_PUBLIC, - emit(func(this *scpb.Schema) scop.Op { + emit(func(this *scpb.Schema) *scop.MarkDescriptorAsPublic { return &scop.MarkDescriptorAsPublic{ DescID: this.SchemaID, } @@ -35,7 +35,7 @@ func init() { ), toAbsent(scpb.Status_PUBLIC, to(scpb.Status_OFFLINE, - emit(func(this *scpb.Schema, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.Schema, md *targetsWithElementMap) *scop.MarkDescriptorAsOffline { return &scop.MarkDescriptorAsOffline{ DescID: this.SchemaID, Reason: statementForDropJob(this, md).Statement, @@ -44,17 +44,17 @@ func init() { ), to(scpb.Status_DROPPED, revertible(false), - emit(func(this *scpb.Schema) scop.Op { + emit(func(this *scpb.Schema) *scop.MarkDescriptorAsDropped { return &scop.MarkDescriptorAsDropped{ DescID: this.SchemaID, } }), ), to(scpb.Status_ABSENT, - emit(func(this *scpb.Schema, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.Schema, md *targetsWithElementMap) *scop.LogEvent { return newLogEventOp(this, md) }), - emit(func(this *scpb.Schema) scop.Op { + emit(func(this *scpb.Schema) *scop.DeleteDescriptor { return &scop.DeleteDescriptor{ DescriptorID: this.SchemaID, } diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_schema_comment.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_schema_comment.go index ad424e567af0..7c4da90bc9f0 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_schema_comment.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_schema_comment.go @@ -20,13 +20,13 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_PUBLIC, - emit(func(this *scpb.SchemaComment) scop.Op { + emit(func(this *scpb.SchemaComment) *scop.UpsertSchemaComment { return &scop.UpsertSchemaComment{ SchemaID: this.SchemaID, Comment: this.Comment, } }), - emit(func(this *scpb.SchemaComment, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.SchemaComment, md *targetsWithElementMap) *scop.LogEvent { return newLogEventOp(this, md) }), ), @@ -34,7 +34,7 @@ func init() { toAbsent( scpb.Status_PUBLIC, to(scpb.Status_ABSENT, - emit(func(this *scpb.SchemaComment) scop.Op { + emit(func(this *scpb.SchemaComment) *scop.RemoveSchemaComment { return &scop.RemoveSchemaComment{ SchemaID: this.SchemaID, } diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_schema_parent.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_schema_parent.go index 4af3ff0eaaa8..ef87f37dcdde 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_schema_parent.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_schema_parent.go @@ -21,7 +21,7 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_PUBLIC, - emit(func(this *scpb.SchemaParent) scop.Op { + emit(func(this *scpb.SchemaParent) *scop.NotImplemented { return notImplemented(this) }), ), @@ -31,7 +31,7 @@ func init() { to(scpb.Status_ABSENT, // TODO(postamar): remove revertibility constraint when possible revertible(false), - emit(func(this *scpb.SchemaParent) scop.Op { + emit(func(this *scpb.SchemaParent) *scop.RemoveSchemaParent { return &scop.RemoveSchemaParent{ Parent: *protoutil.Clone(this).(*scpb.SchemaParent), } diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_secondary_index.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_secondary_index.go index b81d71107312..054c51664508 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_secondary_index.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_secondary_index.go @@ -21,7 +21,7 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_BACKFILL_ONLY, - emit(func(this *scpb.SecondaryIndex) scop.Op { + emit(func(this *scpb.SecondaryIndex) *scop.MakeAddedIndexBackfilling { return &scop.MakeAddedIndexBackfilling{ Index: *protoutil.Clone(&this.Index).(*scpb.Index), IsSecondaryIndex: true, @@ -29,7 +29,7 @@ func init() { }), ), to(scpb.Status_BACKFILLED, - emit(func(this *scpb.SecondaryIndex) scop.Op { + emit(func(this *scpb.SecondaryIndex) *scop.BackfillIndex { return &scop.BackfillIndex{ TableID: this.TableID, SourceIndexID: this.SourceIndexID, @@ -38,7 +38,7 @@ func init() { }), ), to(scpb.Status_DELETE_ONLY, - emit(func(this *scpb.SecondaryIndex) scop.Op { + emit(func(this *scpb.SecondaryIndex) *scop.MakeBackfillingIndexDeleteOnly { return &scop.MakeBackfillingIndexDeleteOnly{ TableID: this.TableID, IndexID: this.IndexID, @@ -46,7 +46,7 @@ func init() { }), ), to(scpb.Status_MERGE_ONLY, - emit(func(this *scpb.SecondaryIndex) scop.Op { + emit(func(this *scpb.SecondaryIndex) *scop.MakeAddedIndexDeleteAndWriteOnly { return &scop.MakeAddedIndexDeleteAndWriteOnly{ TableID: this.TableID, IndexID: this.IndexID, @@ -55,7 +55,7 @@ func init() { ), equiv(scpb.Status_WRITE_ONLY), to(scpb.Status_MERGED, - emit(func(this *scpb.SecondaryIndex) scop.Op { + emit(func(this *scpb.SecondaryIndex) *scop.MergeIndex { return &scop.MergeIndex{ TableID: this.TableID, TemporaryIndexID: this.TemporaryIndexID, @@ -64,7 +64,7 @@ func init() { }), ), to(scpb.Status_VALIDATED, - emit(func(this *scpb.SecondaryIndex) scop.Op { + emit(func(this *scpb.SecondaryIndex) *scop.ValidateUniqueIndex { // TODO(ajwerner): Should this say something other than // ValidateUniqueIndex for a non-unique index? return &scop.ValidateUniqueIndex{ @@ -74,7 +74,7 @@ func init() { }), ), to(scpb.Status_PUBLIC, - emit(func(this *scpb.SecondaryIndex) scop.Op { + emit(func(this *scpb.SecondaryIndex) *scop.MakeAddedSecondaryIndexPublic { return &scop.MakeAddedSecondaryIndexPublic{ TableID: this.TableID, IndexID: this.IndexID, @@ -85,7 +85,7 @@ func init() { toAbsent( scpb.Status_PUBLIC, to(scpb.Status_VALIDATED, - emit(func(this *scpb.SecondaryIndex) scop.Op { + emit(func(this *scpb.SecondaryIndex) *scop.MakeDroppedNonPrimaryIndexDeleteAndWriteOnly { // Most of this logic is taken from MakeMutationComplete(). return &scop.MakeDroppedNonPrimaryIndexDeleteAndWriteOnly{ TableID: this.TableID, @@ -99,7 +99,7 @@ func init() { equiv(scpb.Status_MERGE_ONLY), equiv(scpb.Status_MERGED), to(scpb.Status_DELETE_ONLY, - emit(func(this *scpb.SecondaryIndex) scop.Op { + emit(func(this *scpb.SecondaryIndex) *scop.MakeDroppedIndexDeleteOnly { return &scop.MakeDroppedIndexDeleteOnly{ TableID: this.TableID, IndexID: this.IndexID, @@ -109,17 +109,17 @@ func init() { equiv(scpb.Status_BACKFILLED), equiv(scpb.Status_BACKFILL_ONLY), to(scpb.Status_ABSENT, - emit(func(this *scpb.SecondaryIndex, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.SecondaryIndex, md *targetsWithElementMap) *scop.LogEvent { return newLogEventOp(this, md) }), - emit(func(this *scpb.SecondaryIndex, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.SecondaryIndex, md *targetsWithElementMap) *scop.CreateGcJobForIndex { return &scop.CreateGcJobForIndex{ TableID: this.TableID, IndexID: this.IndexID, StatementForDropJob: statementForDropJob(this, md), } }), - emit(func(this *scpb.SecondaryIndex) scop.Op { + emit(func(this *scpb.SecondaryIndex) *scop.MakeIndexAbsent { return &scop.MakeIndexAbsent{ TableID: this.TableID, IndexID: this.IndexID, diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_secondary_index_partial.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_secondary_index_partial.go index 6759026fafbd..4435993cfae2 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_secondary_index_partial.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_secondary_index_partial.go @@ -20,14 +20,14 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_PUBLIC, - emit(func(this *scpb.SecondaryIndexPartial) scop.Op { + emit(func(this *scpb.SecondaryIndexPartial) *scop.SetAddedIndexPartialPredicate { return &scop.SetAddedIndexPartialPredicate{ TableID: this.TableID, IndexID: this.IndexID, Expr: this.Expr, } }), - emit(func(this *scpb.SecondaryIndexPartial) scop.Op { + emit(func(this *scpb.SecondaryIndexPartial) *scop.UpdateTableBackReferencesInTypes { if len(this.UsesTypeIDs) == 0 { return nil } @@ -42,13 +42,13 @@ func init() { scpb.Status_PUBLIC, to(scpb.Status_ABSENT, revertible(false), - emit(func(this *scpb.SecondaryIndexPartial) scop.Op { + emit(func(this *scpb.SecondaryIndexPartial) *scop.RemoveDroppedIndexPartialPredicate { return &scop.RemoveDroppedIndexPartialPredicate{ TableID: this.TableID, IndexID: this.IndexID, } }), - emit(func(this *scpb.SecondaryIndexPartial) scop.Op { + emit(func(this *scpb.SecondaryIndexPartial) *scop.UpdateTableBackReferencesInTypes { if len(this.UsesTypeIDs) == 0 { return nil } diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_sequence.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_sequence.go index f0170bdd2a85..a3487080d13d 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_sequence.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_sequence.go @@ -22,12 +22,12 @@ func init() { scpb.Status_ABSENT, equiv(scpb.Status_DROPPED), to(scpb.Status_OFFLINE, - emit(func(this *scpb.Sequence) scop.Op { + emit(func(this *scpb.Sequence) *scop.NotImplemented { return notImplemented(this) }), ), to(scpb.Status_PUBLIC, - emit(func(this *scpb.Sequence) scop.Op { + emit(func(this *scpb.Sequence) *scop.MarkDescriptorAsPublic { return &scop.MarkDescriptorAsPublic{ DescID: this.SequenceID, } @@ -36,7 +36,7 @@ func init() { ), toAbsent(scpb.Status_PUBLIC, to(scpb.Status_OFFLINE, - emit(func(this *scpb.Sequence, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.Sequence, md *targetsWithElementMap) *scop.MarkDescriptorAsOffline { return &scop.MarkDescriptorAsOffline{ DescID: this.SequenceID, Reason: statementForDropJob(this, md).Statement, @@ -45,22 +45,22 @@ func init() { ), to(scpb.Status_DROPPED, revertible(false), - emit(func(this *scpb.Sequence) scop.Op { + emit(func(this *scpb.Sequence) *scop.MarkDescriptorAsDropped { return &scop.MarkDescriptorAsDropped{ DescID: this.SequenceID, } }), - emit(func(this *scpb.Sequence) scop.Op { + emit(func(this *scpb.Sequence) *scop.RemoveAllTableComments { return &scop.RemoveAllTableComments{ TableID: this.SequenceID, } }), ), to(scpb.Status_ABSENT, - emit(func(this *scpb.Sequence, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.Sequence, md *targetsWithElementMap) *scop.LogEvent { return newLogEventOp(this, md) }), - emit(func(this *scpb.Sequence, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.Sequence, md *targetsWithElementMap) *scop.CreateGcJobForTable { return &scop.CreateGcJobForTable{ TableID: this.SequenceID, StatementForDropJob: statementForDropJob(this, md), diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_sequence_owner.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_sequence_owner.go index 24ea29f61693..96236b3feeaf 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_sequence_owner.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_sequence_owner.go @@ -20,7 +20,7 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_PUBLIC, - emit(func(this *scpb.SequenceOwner) scop.Op { + emit(func(this *scpb.SequenceOwner) *scop.NotImplemented { return notImplemented(this) }), ), @@ -30,14 +30,14 @@ func init() { to(scpb.Status_ABSENT, // TODO(postamar): remove revertibility constraint when possible revertible(false), - emit(func(this *scpb.SequenceOwner) scop.Op { + emit(func(this *scpb.SequenceOwner) *scop.RemoveSequenceOwner { return &scop.RemoveSequenceOwner{ OwnedSequenceID: this.SequenceID, TableID: this.TableID, ColumnID: this.ColumnID, } }), - emit(func(this *scpb.SequenceOwner) scop.Op { + emit(func(this *scpb.SequenceOwner) *scop.RemoveOwnerBackReferenceInSequence { return &scop.RemoveOwnerBackReferenceInSequence{ SequenceID: this.SequenceID, } diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_table.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_table.go index 309cb2d9f355..f8d1bd2912a3 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_table.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_table.go @@ -21,12 +21,12 @@ func init() { scpb.Status_ABSENT, equiv(scpb.Status_DROPPED), to(scpb.Status_OFFLINE, - emit(func(this *scpb.Table) scop.Op { + emit(func(this *scpb.Table) *scop.NotImplemented { return notImplemented(this) }), ), to(scpb.Status_PUBLIC, - emit(func(this *scpb.Table) scop.Op { + emit(func(this *scpb.Table) *scop.MarkDescriptorAsPublic { return &scop.MarkDescriptorAsPublic{ DescID: this.TableID, } @@ -36,7 +36,7 @@ func init() { toAbsent( scpb.Status_PUBLIC, to(scpb.Status_OFFLINE, - emit(func(this *scpb.Table, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.Table, md *targetsWithElementMap) *scop.MarkDescriptorAsOffline { return &scop.MarkDescriptorAsOffline{ DescID: this.TableID, Reason: statementForDropJob(this, md).Statement, @@ -45,22 +45,22 @@ func init() { ), to(scpb.Status_DROPPED, revertible(false), - emit(func(this *scpb.Table) scop.Op { + emit(func(this *scpb.Table) *scop.MarkDescriptorAsDropped { return &scop.MarkDescriptorAsDropped{ DescID: this.TableID, } }), - emit(func(this *scpb.Table) scop.Op { + emit(func(this *scpb.Table) *scop.RemoveAllTableComments { return &scop.RemoveAllTableComments{ TableID: this.TableID, } }), ), to(scpb.Status_ABSENT, - emit(func(this *scpb.Table, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.Table, md *targetsWithElementMap) *scop.LogEvent { return newLogEventOp(this, md) }), - emit(func(this *scpb.Table, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.Table, md *targetsWithElementMap) *scop.CreateGcJobForTable { return &scop.CreateGcJobForTable{ TableID: this.TableID, StatementForDropJob: statementForDropJob(this, md), diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_table_comment.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_table_comment.go index 6e3ca1832f0f..b85ea0fa0000 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_table_comment.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_table_comment.go @@ -20,13 +20,13 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_PUBLIC, - emit(func(this *scpb.TableComment) scop.Op { + emit(func(this *scpb.TableComment) *scop.UpsertTableComment { return &scop.UpsertTableComment{ TableID: this.TableID, Comment: this.Comment, } }), - emit(func(this *scpb.TableComment, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.TableComment, md *targetsWithElementMap) *scop.LogEvent { return newLogEventOp(this, md) }), ), @@ -34,12 +34,12 @@ func init() { toAbsent( scpb.Status_PUBLIC, to(scpb.Status_ABSENT, - emit(func(this *scpb.TableComment) scop.Op { + emit(func(this *scpb.TableComment) *scop.RemoveTableComment { return &scop.RemoveTableComment{ TableID: this.TableID, } }), - emit(func(this *scpb.TableComment, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.TableComment, md *targetsWithElementMap) *scop.LogEvent { return newLogEventOp(this, md) }), ), diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_table_locality_global.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_table_locality_global.go index a1885e384eaf..b48050eaa538 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_table_locality_global.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_table_locality_global.go @@ -20,7 +20,7 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_PUBLIC, - emit(func(this *scpb.TableLocalityGlobal) scop.Op { + emit(func(this *scpb.TableLocalityGlobal) *scop.NotImplemented { return notImplemented(this) }), ), diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_table_locality_primary_region.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_table_locality_primary_region.go index 9666db9e21b8..ff245869274a 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_table_locality_primary_region.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_table_locality_primary_region.go @@ -20,7 +20,7 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_PUBLIC, - emit(func(this *scpb.TableLocalityPrimaryRegion) scop.Op { + emit(func(this *scpb.TableLocalityPrimaryRegion) *scop.NotImplemented { return notImplemented(this) }), ), diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_table_locality_regional_by_row.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_table_locality_regional_by_row.go index df231d89563c..a1f01b505597 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_table_locality_regional_by_row.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_table_locality_regional_by_row.go @@ -20,7 +20,7 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_PUBLIC, - emit(func(this *scpb.TableLocalityRegionalByRow) scop.Op { + emit(func(this *scpb.TableLocalityRegionalByRow) *scop.NotImplemented { return notImplemented(this) }), ), diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_table_locality_secondary_region.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_table_locality_secondary_region.go index d3b64f64f9de..e6f04687b5c7 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_table_locality_secondary_region.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_table_locality_secondary_region.go @@ -21,7 +21,7 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_PUBLIC, - emit(func(this *scpb.TableLocalitySecondaryRegion) scop.Op { + emit(func(this *scpb.TableLocalitySecondaryRegion) *scop.NotImplemented { return notImplemented(this) }), ), @@ -32,7 +32,7 @@ func init() { // TODO(postamar): remove revertibility constraint when possible revertible(false), // TODO(postamar): implement table locality update - emit(func(this *scpb.TableLocalitySecondaryRegion) scop.Op { + emit(func(this *scpb.TableLocalitySecondaryRegion) *scop.RemoveBackReferenceInTypes { return &scop.RemoveBackReferenceInTypes{ TypeIDs: []catid.DescID{this.RegionEnumTypeID}, BackReferencedDescID: this.TableID, diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_temporary_index.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_temporary_index.go index 0dd7a79364f8..d26309e42306 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_temporary_index.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_temporary_index.go @@ -21,7 +21,7 @@ func init() { toTransientAbsent( scpb.Status_ABSENT, to(scpb.Status_DELETE_ONLY, - emit(func(this *scpb.TemporaryIndex) scop.Op { + emit(func(this *scpb.TemporaryIndex) *scop.MakeAddedTempIndexDeleteOnly { return &scop.MakeAddedTempIndexDeleteOnly{ Index: *protoutil.Clone(&this.Index).(*scpb.Index), IsSecondaryIndex: this.IsUsingSecondaryEncoding, @@ -29,7 +29,7 @@ func init() { }), ), to(scpb.Status_WRITE_ONLY, - emit(func(this *scpb.TemporaryIndex) scop.Op { + emit(func(this *scpb.TemporaryIndex) *scop.MakeAddedIndexDeleteAndWriteOnly { return &scop.MakeAddedIndexDeleteAndWriteOnly{ TableID: this.TableID, IndexID: this.IndexID, @@ -41,7 +41,7 @@ func init() { scpb.Status_WRITE_ONLY, to(scpb.Status_DELETE_ONLY, revertible(false), - emit(func(this *scpb.TemporaryIndex) scop.Op { + emit(func(this *scpb.TemporaryIndex) *scop.MakeDroppedIndexDeleteOnly { return &scop.MakeDroppedIndexDeleteOnly{ TableID: this.TableID, IndexID: this.IndexID, @@ -49,13 +49,13 @@ func init() { }), ), to(scpb.Status_ABSENT, - emit(func(this *scpb.TemporaryIndex) scop.Op { + emit(func(this *scpb.TemporaryIndex) *scop.CreateGcJobForIndex { return &scop.CreateGcJobForIndex{ TableID: this.TableID, IndexID: this.IndexID, } }), - emit(func(this *scpb.TemporaryIndex) scop.Op { + emit(func(this *scpb.TemporaryIndex) *scop.MakeIndexAbsent { return &scop.MakeIndexAbsent{ TableID: this.TableID, IndexID: this.IndexID, diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_unique_without_index_constraint.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_unique_without_index_constraint.go index a61cebca8322..085943adf809 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_unique_without_index_constraint.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_unique_without_index_constraint.go @@ -20,7 +20,7 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_PUBLIC, - emit(func(this *scpb.UniqueWithoutIndexConstraint) scop.Op { + emit(func(this *scpb.UniqueWithoutIndexConstraint) *scop.NotImplemented { return notImplemented(this) }), ), @@ -30,7 +30,7 @@ func init() { to(scpb.Status_ABSENT, // TODO(postamar): remove revertibility constraint when possible revertible(false), - emit(func(this *scpb.UniqueWithoutIndexConstraint) scop.Op { + emit(func(this *scpb.UniqueWithoutIndexConstraint) *scop.NotImplemented { return notImplemented(this) }), ), diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_user_privileges.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_user_privileges.go index ddbdcd9d6cd5..7ac2ebb572d9 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_user_privileges.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_user_privileges.go @@ -20,7 +20,7 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_PUBLIC, - emit(func(this *scpb.UserPrivileges) scop.Op { + emit(func(this *scpb.UserPrivileges) *scop.NotImplemented { return notImplemented(this) }), ), @@ -30,7 +30,7 @@ func init() { to(scpb.Status_ABSENT, // TODO(postamar): remove revertibility constraint when possible revertible(false), - emit(func(this *scpb.UserPrivileges) scop.Op { + emit(func(this *scpb.UserPrivileges) *scop.RemoveUserPrivileges { return &scop.RemoveUserPrivileges{ DescID: this.DescriptorID, User: this.UserName, diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_view.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_view.go index a72675ff7e26..cc73526268af 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_view.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_view.go @@ -21,12 +21,12 @@ func init() { scpb.Status_ABSENT, equiv(scpb.Status_DROPPED), to(scpb.Status_OFFLINE, - emit(func(this *scpb.View) scop.Op { + emit(func(this *scpb.View) *scop.NotImplemented { return notImplemented(this) }), ), to(scpb.Status_PUBLIC, - emit(func(this *scpb.View) scop.Op { + emit(func(this *scpb.View) *scop.MarkDescriptorAsPublic { return &scop.MarkDescriptorAsPublic{ DescID: this.ViewID, } @@ -36,7 +36,7 @@ func init() { toAbsent( scpb.Status_PUBLIC, to(scpb.Status_OFFLINE, - emit(func(this *scpb.View, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.View, md *targetsWithElementMap) *scop.MarkDescriptorAsOffline { return &scop.MarkDescriptorAsOffline{ DescID: this.ViewID, Reason: statementForDropJob(this, md).Statement, @@ -45,12 +45,12 @@ func init() { ), to(scpb.Status_DROPPED, revertible(false), - emit(func(this *scpb.View) scop.Op { + emit(func(this *scpb.View) *scop.MarkDescriptorAsDropped { return &scop.MarkDescriptorAsDropped{ DescID: this.ViewID, } }), - emit(func(this *scpb.View) scop.Op { + emit(func(this *scpb.View) *scop.RemoveBackReferenceInTypes { if len(this.UsesTypeIDs) == 0 { return nil } @@ -59,7 +59,7 @@ func init() { TypeIDs: this.UsesTypeIDs, } }), - emit(func(this *scpb.View) scop.Op { + emit(func(this *scpb.View) *scop.RemoveViewBackReferencesInRelations { if len(this.UsesRelationIDs) == 0 { return nil } @@ -68,17 +68,17 @@ func init() { RelationIDs: this.UsesRelationIDs, } }), - emit(func(this *scpb.View) scop.Op { + emit(func(this *scpb.View) *scop.RemoveAllTableComments { return &scop.RemoveAllTableComments{ TableID: this.ViewID, } }), ), to(scpb.Status_ABSENT, - emit(func(this *scpb.View, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.View, md *targetsWithElementMap) *scop.LogEvent { return newLogEventOp(this, md) }), - emit(func(this *scpb.View, md *targetsWithElementMap) scop.Op { + emit(func(this *scpb.View, md *targetsWithElementMap) *scop.CreateGcJobForTable { if !this.IsMaterialized { return nil @@ -88,7 +88,7 @@ func init() { StatementForDropJob: statementForDropJob(this, md), } }), - emit(func(this *scpb.View) scop.Op { + emit(func(this *scpb.View) *scop.DeleteDescriptor { if !this.IsMaterialized { return &scop.DeleteDescriptor{ DescriptorID: this.ViewID, diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/target.go b/pkg/sql/schemachanger/scplan/internal/opgen/target.go index 60b1192ac922..be20f91cf563 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/target.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/target.go @@ -31,7 +31,9 @@ type target struct { type transition struct { from, to scpb.Status revertible bool + canFail bool ops opsFunc + opType scop.Type minPhase scop.Phase } @@ -70,35 +72,57 @@ func makeTarget(e scpb.Element, spec targetSpec) (t target, err error) { func makeTransitions(e scpb.Element, spec targetSpec) (ret []transition, err error) { tbs := makeTransitionBuildState(spec.from) + + // lastTransitionWhichCanFail tracks the index in ret of the last transition + // corresponding to a backfill or validation. We'll use this to add an + // annotation to the transitions indicating whether such an operation exists + // in the current or any subsequent transitions. + lastTransitionWhichCanFail := -1 for i, s := range spec.transitionSpecs { var t transition if s.from == scpb.Status_UNKNOWN { t.from = tbs.from t.to = s.to if err := tbs.withTransition(s, i == 0 /* isFirst */); err != nil { - return nil, errors.Wrapf(err, "invalid transition %s -> %s", t.from, t.to) + return nil, errors.Wrapf( + err, "invalid transition %s -> %s", t.from, t.to, + ) } if len(s.emitFns) > 0 { - t.ops, err = makeOpsFunc(e, s.emitFns) + t.ops, t.opType, err = makeOpsFunc(e, s.emitFns) if err != nil { - return nil, errors.Wrapf(err, "making ops func for transition %s -> %s", t.from, t.to) + return nil, errors.Wrapf( + err, "making ops func for transition %s -> %s", t.from, t.to, + ) } } } else { t.from = s.from t.to = tbs.from if err := tbs.withEquivTransition(s); err != nil { - return nil, errors.Wrapf(err, "invalid no-op transition %s -> %s", t.from, t.to) + return nil, errors.Wrapf( + err, "invalid no-op transition %s -> %s", t.from, t.to, + ) } } t.revertible = tbs.isRevertible t.minPhase = tbs.currentMinPhase + if t.opType != scop.MutationType && t.opType != 0 { + lastTransitionWhichCanFail = i + } ret = append(ret, t) } + // Mark the transitions which can fail or precede something which can fail. + for i := 0; i <= lastTransitionWhichCanFail; i++ { + ret[i].canFail = true + } + // Check that the final status has been reached. if tbs.from != spec.to { - return nil, errors.Errorf("expected %s as the final status, instead found %s", spec.to, tbs.from) + return nil, errors.Errorf( + "expected %s as the final status, instead found %s", spec.to, tbs.from, + ) } return ret, nil diff --git a/pkg/sql/schemachanger/scplan/internal/rules/dep_index_and_column.go b/pkg/sql/schemachanger/scplan/internal/rules/dep_index_and_column.go index e1af0bf1786f..e857c703b18b 100644 --- a/pkg/sql/schemachanger/scplan/internal/rules/dep_index_and_column.go +++ b/pkg/sql/schemachanger/scplan/internal/rules/dep_index_and_column.go @@ -743,6 +743,47 @@ func init() { currentStatus(to.node, scpb.Status_WRITE_ONLY), } }) + + // This is a pair of somewhat brute-force hack to ensure that we only create + // a single GC job for all the indexes of a table being dropped by a + // transaction. + registerDepRule("temp indexes reach absent at the same time as other indexes", + scgraph.SameStagePrecedence, + "index-a", "index-b", + func(from, to nodeVars) rel.Clauses { + return rel.Clauses{ + from.el.Type((*scpb.TemporaryIndex)(nil)), + to.el.Type((*scpb.PrimaryIndex)(nil), (*scpb.SecondaryIndex)(nil)), + joinOnDescID(from.el, to.el, "descID"), + targetStatus(from.target, scpb.Transient), + targetStatus(to.target, scpb.ToAbsent), + currentStatus(from.node, scpb.Status_TRANSIENT_ABSENT), + currentStatus(to.node, scpb.Status_ABSENT), + } + }) + registerDepRule("indexes reach absent at the same time as other indexes", + scgraph.SameStagePrecedence, + "index-a", "index-b", + func(from, to nodeVars) rel.Clauses { + return rel.Clauses{ + from.el.Type((*scpb.PrimaryIndex)(nil), (*scpb.SecondaryIndex)(nil)), + to.el.Type((*scpb.PrimaryIndex)(nil), (*scpb.SecondaryIndex)(nil)), + joinOnDescID(from.el, to.el, "descID"), + targetStatusEq(from.target, to.target, scpb.ToAbsent), + currentStatusEq(from.node, to.node, scpb.Status_ABSENT), + + // Use the index ID to provide an ordering for dropping the indexes + // to ensure that there is no cycle in the edges. + // + // TODO(ajwerner): It'd be nice to be able to express this in rel + // directly. + rel.Filter("indexes-id-less", "a", "b")(func(a, b scpb.Element) bool { + aID, _ := screl.GetIndexID(a) + bID, _ := screl.GetIndexID(b) + return aID < bID + }), + } + }) } // This rule ensures that columns depend on each other in increasing order. diff --git a/pkg/sql/schemachanger/scplan/internal/rules/testdata/deprules b/pkg/sql/schemachanger/scplan/internal/rules/testdata/deprules index c7938168fc2e..8d21421dd7dc 100644 --- a/pkg/sql/schemachanger/scplan/internal/rules/testdata/deprules +++ b/pkg/sql/schemachanger/scplan/internal/rules/testdata/deprules @@ -592,6 +592,35 @@ deprules - $index-node[CurrentStatus] = WRITE_ONLY - joinTargetNode($index-column, $index-column-target, $index-column-node) - joinTargetNode($index, $index-target, $index-node) +- name: temp indexes reach absent at the same time as other indexes + from: index-a-node + kind: SameStagePrecedence + to: index-b-node + query: + - $index-a[Type] = '*scpb.TemporaryIndex' + - $index-b[Type] IN ['*scpb.PrimaryIndex', '*scpb.SecondaryIndex'] + - joinOnDescID($index-a, $index-b, $descID) + - $index-a-target[TargetStatus] = TRANSIENT_ABSENT + - $index-b-target[TargetStatus] = ABSENT + - $index-a-node[CurrentStatus] = TRANSIENT_ABSENT + - $index-b-node[CurrentStatus] = ABSENT + - joinTargetNode($index-a, $index-a-target, $index-a-node) + - joinTargetNode($index-b, $index-b-target, $index-b-node) +- name: indexes reach absent at the same time as other indexes + from: index-a-node + kind: SameStagePrecedence + to: index-b-node + query: + - $index-a[Type] IN ['*scpb.PrimaryIndex', '*scpb.SecondaryIndex'] + - $index-b[Type] IN ['*scpb.PrimaryIndex', '*scpb.SecondaryIndex'] + - joinOnDescID($index-a, $index-b, $descID) + - $index-a-target[TargetStatus] = ABSENT + - $index-b-target[TargetStatus] = ABSENT + - $index-a-node[CurrentStatus] = ABSENT + - $index-b-node[CurrentStatus] = ABSENT + - indexes-id-less(scpb.Element, scpb.Element)($a, $b) + - joinTargetNode($index-a, $index-a-target, $index-a-node) + - joinTargetNode($index-b, $index-b-target, $index-b-node) - name: ensure columns are in increasing order from: later-column-node kind: SameStagePrecedence diff --git a/pkg/sql/schemachanger/scplan/internal/scgraph/edge.go b/pkg/sql/schemachanger/scplan/internal/scgraph/edge.go index 86724458a73f..132af20d015e 100644 --- a/pkg/sql/schemachanger/scplan/internal/scgraph/edge.go +++ b/pkg/sql/schemachanger/scplan/internal/scgraph/edge.go @@ -29,11 +29,20 @@ type Edge interface { // OpEdge represents an edge changing the state of a target with an op. type OpEdge struct { - from, to *screl.Node - op []scop.Op - typ scop.Type + from, to *screl.Node + op []scop.Op + typ scop.Type + + // canFail indicates that a backfill or validation operation for this + // target has yet to be run as of the originating node for this edge. + canFail bool + + // revertible indicates that no operation which destroys information + // permanently or publishes new information externally has yet been + // run for this target. revertible bool - minPhase scop.Phase + + minPhase scop.Phase } // From implements the Edge interface. @@ -45,9 +54,14 @@ func (oe *OpEdge) To() *screl.Node { return oe.to } // Op returns the scop.Op for execution that is associated with the op edge. func (oe *OpEdge) Op() []scop.Op { return oe.op } -// Revertible returns if the dependency edge is revertible +// Revertible returns if the dependency edge is revertible. func (oe *OpEdge) Revertible() bool { return oe.revertible } +// CanFail returns if the dependency edge corresponds to a target in a state +// which has yet to undergo all of its validation and backfill operations +// before reaching its targeted status. +func (oe *OpEdge) CanFail() bool { return oe.canFail } + // Type returns the types of operations associated with this edge. func (oe *OpEdge) Type() scop.Type { return oe.typ diff --git a/pkg/sql/schemachanger/scplan/internal/scgraph/graph.go b/pkg/sql/schemachanger/scplan/internal/scgraph/graph.go index 41a920f36d37..9380f6399f87 100644 --- a/pkg/sql/schemachanger/scplan/internal/scgraph/graph.go +++ b/pkg/sql/schemachanger/scplan/internal/scgraph/graph.go @@ -202,12 +202,16 @@ func (g *Graph) GetOpEdgeFrom(n *screl.Node) (*OpEdge, bool) { // AddOpEdges adds an op edges connecting the nodes for two statuses of a target. func (g *Graph) AddOpEdges( - t *scpb.Target, from, to scpb.Status, revertible bool, minPhase scop.Phase, ops ...scop.Op, + t *scpb.Target, + from, to scpb.Status, + revertible, canFail bool, + minPhase scop.Phase, + ops ...scop.Op, ) (err error) { - oe := &OpEdge{ op: ops, revertible: revertible, + canFail: canFail, minPhase: minPhase, } if oe.from, err = g.getOrCreateNode(t, from); err != nil { diff --git a/pkg/sql/schemachanger/scplan/internal/scgraph/graph_test.go b/pkg/sql/schemachanger/scplan/internal/scgraph/graph_test.go index 8d6d73a3eeaf..34cc493a39c3 100644 --- a/pkg/sql/schemachanger/scplan/internal/scgraph/graph_test.go +++ b/pkg/sql/schemachanger/scplan/internal/scgraph/graph_test.go @@ -111,12 +111,13 @@ func TestGraphRanks(t *testing.T) { require.NoError(t, err) // Setup op edges for all the nodes. for idx := range tc.addNode { + const revertible, canFail = true, false if tc.addNode[idx] { require.NoError(t, graph.AddOpEdges( &ts.Targets[idx], scpb.Status_ABSENT, scpb.Status_PUBLIC, - true, + revertible, canFail, scop.StatementPhase, &scop.MakeColumnAbsent{}, )) @@ -125,7 +126,7 @@ func TestGraphRanks(t *testing.T) { &ts.Targets[idx], scpb.Status_PUBLIC, scpb.Status_ABSENT, - true, + revertible, canFail, scop.StatementPhase, &scop.MakeColumnAbsent{}, )) diff --git a/pkg/sql/schemachanger/scplan/internal/scstage/build.go b/pkg/sql/schemachanger/scplan/internal/scstage/build.go index 67942c109b9f..99dde6fc0e23 100644 --- a/pkg/sql/schemachanger/scplan/internal/scstage/build.go +++ b/pkg/sql/schemachanger/scplan/internal/scstage/build.go @@ -110,11 +110,22 @@ func buildStages(bc buildContext) (stages []Stage) { for !bc.isStateTerminal(bs.incumbent) { // Generate a stage builder which can make progress. sb := bc.makeStageBuilder(bs) - for !sb.canMakeProgress() { + // We allow mixing of revertible and non-revertible operations if there + // are no remaining operations which can fail. That being said, we want + // to not want to build such a stage as PostCommit but rather as + // PostCommitNonRevertible. This condition is used to determine whether + // the phase needs to be advanced due to the presence of non-revertible + // operations. + shouldBeInPostCommitNonRevertible := func() bool { + return !bc.isRevertibilityIgnored && + bs.phase == scop.PostCommitPhase && + sb.hasAnyNonRevertibleOps() + } + for !sb.canMakeProgress() || shouldBeInPostCommitNonRevertible() { // When no further progress is possible, move to the next phase and try // again, until progress is possible. We haven't reached the terminal // state yet, so this is guaranteed (barring any horrible bugs). - if bs.phase == scop.PreCommitPhase { + if !sb.canMakeProgress() && bs.phase == scop.PreCommitPhase { // This is a special case. // We need to move to the post-commit phase, but this will require // creating a schema changer job, which in turn will require this @@ -197,11 +208,27 @@ func (bc buildContext) makeStageBuilderForType(bs buildState, opType scop.Type) lut: make(map[*scpb.Target]*currentTargetState, numTargets), visited: make(map[*screl.Node]uint64, numTargets), } - for i, n := range bc.nodes(bs.incumbent) { - t := sb.makeCurrentTargetState(n) - sb.current[i] = t - sb.lut[t.n.Target] = &sb.current[i] + { + nodes := bc.nodes(bs.incumbent) + + // Determine whether there are any backfill or validation operations + // remaining which should prevent the scheduling of any non-revertible + // operations. This information will be used when building the current + // set of targets below. + for _, n := range nodes { + if oe, ok := bc.g.GetOpEdgeFrom(n); ok && oe.CanFail() { + sb.anyRemainingOpsCanFail = true + break + } + } + + for i, n := range nodes { + t := sb.makeCurrentTargetState(n) + sb.current[i] = t + sb.lut[t.n.Target] = &sb.current[i] + } } + // Greedily try to make progress by going down op edges when possible. for isDone := false; !isDone; { isDone = true @@ -236,6 +263,13 @@ type stageBuilder struct { fulfilling map[*screl.Node]struct{} opEdges []*scgraph.OpEdge + // anyRemainingOpsCanFail indicates whether there are any backfill or + // validation operations remaining in the schema change. If not, then + // revertible operations can be combined with non-revertible operations + // in order to move the schema change into the PostCommitNonRevertiblePhase + // earlier than it might otherwise. + anyRemainingOpsCanFail bool + // Helper data structures used to improve performance. lut map[*scpb.Target]*currentTargetState @@ -283,9 +317,20 @@ func (sb stageBuilder) isOutgoingOpEdgeAllowed(e *scgraph.OpEdge) bool { if !e.IsPhaseSatisfied(sb.bs.phase) { return false } + // We allow non-revertible ops to be included at stages preceding + // PostCommitNonRevertible if nothing left in the schema change at this + // point can fail. The caller is responsible for detecting whether any + // non-revertible operations are included in a phase before + // PostCommitNonRevertible and adjusting the phase accordingly. This is + // critical to allow op-edges which might otherwise be revertible to be + // grouped with non-revertible operations. if !sb.bc.isRevertibilityIgnored && sb.bs.phase < scop.PostCommitNonRevertiblePhase && - !e.Revertible() { + !e.Revertible() && + // We can't act on the knowledge that nothing remaining can fail while in + // StatementPhase because we don't know about what future targets may + // show up which could fail. + (sb.bs.phase < scop.PostCommitPhase || sb.anyRemainingOpsCanFail) { return false } return true @@ -456,6 +501,17 @@ func (sb stageBuilder) build() Stage { return s } +// hasAnyNonRevertibleOps returns true if there are any ops in the current +// stage which are not revertible. +func (sb stageBuilder) hasAnyNonRevertibleOps() bool { + for _, e := range sb.opEdges { + if !e.Revertible() { + return true + } + } + return false +} + // computeExtraJobOps generates job-related operations to decorate a stage with. // // TODO(ajwerner): Rather than adding this above the opgen layer, it may be @@ -552,10 +608,14 @@ func (bc buildContext) removeJobReferenceOp(descID descpb.ID) scop.Op { } func (bc buildContext) nodes(current []scpb.Status) []*screl.Node { - nodes := make([]*screl.Node, len(bc.targetState.Targets)) + return nodes(bc.g, bc.targetState.Targets, current) +} + +func nodes(g *scgraph.Graph, targets []scpb.Target, current []scpb.Status) []*screl.Node { + nodes := make([]*screl.Node, len(targets)) for i, status := range current { - t := &bc.targetState.Targets[i] - n, ok := bc.g.GetNode(t, status) + t := &targets[i] + n, ok := g.GetNode(t, status) if !ok { panic(errors.AssertionFailedf("could not find node for element %s, target status %s, current status %s", screl.ElementString(t.Element()), t.TargetStatus, status)) diff --git a/pkg/sql/schemachanger/scplan/testdata/alter_table b/pkg/sql/schemachanger/scplan/testdata/alter_table index 3b5bf1c56346..2fab13926bfd 100644 --- a/pkg/sql/schemachanger/scplan/testdata/alter_table +++ b/pkg/sql/schemachanger/scplan/testdata/alter_table @@ -56,7 +56,7 @@ StatementPhase stage 1 of 1 with 6 MutationType ops TypeIDs: - 105 - 106 - scop.AddColumnToIndex + *scop.AddColumnToIndex ColumnID: 2 IndexID: 1 Kind: 2 @@ -202,20 +202,20 @@ StatementPhase stage 1 of 1 with 11 MutationType ops IsUnique: true SourceIndexID: 1 TableID: 104 - scop.AddColumnToIndex + *scop.AddColumnToIndex ColumnID: 1 IndexID: 3 TableID: 104 - scop.AddColumnToIndex + *scop.AddColumnToIndex ColumnID: 2 IndexID: 3 Kind: 2 TableID: 104 - scop.AddColumnToIndex + *scop.AddColumnToIndex ColumnID: 1 IndexID: 2 TableID: 104 - scop.AddColumnToIndex + *scop.AddColumnToIndex ColumnID: 2 IndexID: 2 Kind: 2 @@ -232,13 +232,13 @@ PreCommitPhase stage 1 of 1 with 2 MutationType ops DescriptorIDs: - 104 JobID: 1 - RunningStatus: PostCommitPhase stage 1 of 7 with 2 MutationType ops pending + RunningStatus: PostCommitPhase stage 1 of 6 with 2 MutationType ops pending Statements: - statement: ALTER TABLE defaultdb.foo ADD COLUMN j INT8 DEFAULT 123 redactedstatement: ALTER TABLE ‹defaultdb›.public.‹foo› ADD COLUMN ‹j› INT8 DEFAULT ‹123› statementtag: ALTER TABLE -PostCommitPhase stage 1 of 7 with 4 MutationType ops +PostCommitPhase stage 1 of 6 with 4 MutationType ops transitions: [[Column:{DescID: 104, ColumnID: 2}, PUBLIC], DELETE_ONLY] -> WRITE_ONLY [[TemporaryIndex:{DescID: 104, IndexID: 3, SourceIndexID: 1}, TRANSIENT_ABSENT], DELETE_ONLY] -> WRITE_ONLY @@ -253,7 +253,7 @@ PostCommitPhase stage 1 of 7 with 4 MutationType ops DescriptorID: 104 *scop.UpdateSchemaChangerJob JobID: 1 -PostCommitPhase stage 2 of 7 with 1 BackfillType op +PostCommitPhase stage 2 of 6 with 1 BackfillType op transitions: [[PrimaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 1, TemporaryIndexID: 3, SourceIndexID: 1}, PUBLIC], BACKFILL_ONLY] -> BACKFILLED ops: @@ -261,7 +261,7 @@ PostCommitPhase stage 2 of 7 with 1 BackfillType op IndexID: 2 SourceIndexID: 1 TableID: 104 -PostCommitPhase stage 3 of 7 with 3 MutationType ops +PostCommitPhase stage 3 of 6 with 3 MutationType ops transitions: [[PrimaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 1, TemporaryIndexID: 3, SourceIndexID: 1}, PUBLIC], BACKFILLED] -> DELETE_ONLY ops: @@ -272,7 +272,7 @@ PostCommitPhase stage 3 of 7 with 3 MutationType ops DescriptorID: 104 *scop.UpdateSchemaChangerJob JobID: 1 -PostCommitPhase stage 4 of 7 with 3 MutationType ops +PostCommitPhase stage 4 of 6 with 3 MutationType ops transitions: [[PrimaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 1, TemporaryIndexID: 3, SourceIndexID: 1}, PUBLIC], DELETE_ONLY] -> MERGE_ONLY ops: @@ -283,7 +283,7 @@ PostCommitPhase stage 4 of 7 with 3 MutationType ops DescriptorID: 104 *scop.UpdateSchemaChangerJob JobID: 1 -PostCommitPhase stage 5 of 7 with 1 BackfillType op +PostCommitPhase stage 5 of 6 with 1 BackfillType op transitions: [[PrimaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 1, TemporaryIndexID: 3, SourceIndexID: 1}, PUBLIC], MERGE_ONLY] -> MERGED ops: @@ -291,20 +291,22 @@ PostCommitPhase stage 5 of 7 with 1 BackfillType op BackfilledIndexID: 2 TableID: 104 TemporaryIndexID: 3 -PostCommitPhase stage 6 of 7 with 1 ValidationType op +PostCommitPhase stage 6 of 6 with 1 ValidationType op transitions: [[PrimaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 1, TemporaryIndexID: 3, SourceIndexID: 1}, PUBLIC], MERGED] -> VALIDATED ops: *scop.ValidateUniqueIndex IndexID: 2 TableID: 104 -PostCommitPhase stage 7 of 7 with 8 MutationType ops +PostCommitNonRevertiblePhase stage 1 of 3 with 10 MutationType ops transitions: - [[PrimaryIndex:{DescID: 104, IndexID: 1, ConstraintID: 1}, ABSENT], PUBLIC] -> VALIDATED + [[IndexColumn:{DescID: 104, ColumnID: 1, IndexID: 1}, ABSENT], PUBLIC] -> ABSENT + [[PrimaryIndex:{DescID: 104, IndexID: 1, ConstraintID: 1}, ABSENT], PUBLIC] -> WRITE_ONLY [[IndexName:{DescID: 104, Name: foo_pkey, IndexID: 1}, ABSENT], PUBLIC] -> ABSENT [[Column:{DescID: 104, ColumnID: 2}, PUBLIC], WRITE_ONLY] -> PUBLIC [[PrimaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 1, TemporaryIndexID: 3, SourceIndexID: 1}, PUBLIC], VALIDATED] -> PUBLIC [[IndexName:{DescID: 104, Name: foo_pkey, IndexID: 2}, PUBLIC], ABSENT] -> PUBLIC + [[TemporaryIndex:{DescID: 104, IndexID: 3, SourceIndexID: 1}, TRANSIENT_ABSENT], WRITE_ONLY] -> TRANSIENT_DELETE_ONLY ops: *scop.MakeDroppedPrimaryIndexDeleteAndWriteOnly IndexID: 1 @@ -317,6 +319,13 @@ PostCommitPhase stage 7 of 7 with 8 MutationType ops IndexID: 2 Name: foo_pkey TableID: 104 + *scop.MakeDroppedIndexDeleteOnly + IndexID: 3 + TableID: 104 + *scop.RemoveColumnFromIndex + ColumnID: 1 + IndexID: 1 + TableID: 104 *scop.MakeAddedPrimaryIndexPublic EventBase: Authorization: @@ -346,19 +355,10 @@ PostCommitPhase stage 7 of 7 with 8 MutationType ops *scop.UpdateSchemaChangerJob IsNonCancelable: true JobID: 1 -PostCommitNonRevertiblePhase stage 1 of 2 with 5 MutationType ops +PostCommitNonRevertiblePhase stage 2 of 3 with 3 MutationType ops transitions: - [[IndexColumn:{DescID: 104, ColumnID: 1, IndexID: 1}, ABSENT], PUBLIC] -> ABSENT - [[PrimaryIndex:{DescID: 104, IndexID: 1, ConstraintID: 1}, ABSENT], VALIDATED] -> DELETE_ONLY - [[TemporaryIndex:{DescID: 104, IndexID: 3, SourceIndexID: 1}, TRANSIENT_ABSENT], WRITE_ONLY] -> TRANSIENT_DELETE_ONLY + [[PrimaryIndex:{DescID: 104, IndexID: 1, ConstraintID: 1}, ABSENT], WRITE_ONLY] -> DELETE_ONLY ops: - scop.RemoveColumnFromIndex - ColumnID: 1 - IndexID: 1 - TableID: 104 - *scop.MakeDroppedIndexDeleteOnly - IndexID: 3 - TableID: 104 *scop.MakeDroppedIndexDeleteOnly IndexID: 1 TableID: 104 @@ -367,11 +367,17 @@ PostCommitNonRevertiblePhase stage 1 of 2 with 5 MutationType ops *scop.UpdateSchemaChangerJob IsNonCancelable: true JobID: 1 -PostCommitNonRevertiblePhase stage 2 of 2 with 6 MutationType ops +PostCommitNonRevertiblePhase stage 3 of 3 with 6 MutationType ops transitions: [[PrimaryIndex:{DescID: 104, IndexID: 1, ConstraintID: 1}, ABSENT], DELETE_ONLY] -> ABSENT [[TemporaryIndex:{DescID: 104, IndexID: 3, SourceIndexID: 1}, TRANSIENT_ABSENT], TRANSIENT_DELETE_ONLY] -> TRANSIENT_ABSENT ops: + *scop.CreateGcJobForIndex + IndexID: 3 + TableID: 104 + *scop.MakeIndexAbsent + IndexID: 3 + TableID: 104 *scop.CreateGcJobForIndex IndexID: 1 StatementForDropJob: @@ -388,12 +394,6 @@ PostCommitNonRevertiblePhase stage 2 of 2 with 6 MutationType ops SubWorkID: 1 IndexID: 1 TableID: 104 - *scop.CreateGcJobForIndex - IndexID: 3 - TableID: 104 - *scop.MakeIndexAbsent - IndexID: 3 - TableID: 104 *scop.RemoveJobStateFromDescriptor DescriptorID: 104 JobID: 1 @@ -520,32 +520,32 @@ StatementPhase stage 1 of 1 with 18 MutationType ops IsUnique: true SourceIndexID: 1 TableID: 104 - scop.AddColumnToIndex + *scop.AddColumnToIndex ColumnID: 1 IndexID: 3 TableID: 104 - scop.AddColumnToIndex + *scop.AddColumnToIndex ColumnID: 2 IndexID: 3 Kind: 2 TableID: 104 - scop.AddColumnToIndex + *scop.AddColumnToIndex ColumnID: 3 IndexID: 2 Kind: 2 Ordinal: 1 TableID: 104 - scop.AddColumnToIndex + *scop.AddColumnToIndex ColumnID: 3 IndexID: 3 Kind: 2 Ordinal: 1 TableID: 104 - scop.AddColumnToIndex + *scop.AddColumnToIndex ColumnID: 1 IndexID: 2 TableID: 104 - scop.AddColumnToIndex + *scop.AddColumnToIndex ColumnID: 2 IndexID: 2 Kind: 2 @@ -562,7 +562,7 @@ PreCommitPhase stage 1 of 1 with 2 MutationType ops DescriptorIDs: - 104 JobID: 1 - RunningStatus: PostCommitPhase stage 1 of 7 with 3 MutationType ops pending + RunningStatus: PostCommitPhase stage 1 of 6 with 3 MutationType ops pending Statements: - statement: ALTER TABLE defaultdb.foo ADD COLUMN j INT8 DEFAULT 123 redactedstatement: ALTER TABLE ‹defaultdb›.public.‹foo› ADD COLUMN ‹j› INT8 DEFAULT @@ -572,7 +572,7 @@ PreCommitPhase stage 1 of 1 with 2 MutationType ops redactedstatement: ALTER TABLE ‹defaultdb›.public.‹foo› ADD COLUMN ‹k› INT8 DEFAULT ‹456› statementtag: ALTER TABLE -PostCommitPhase stage 1 of 7 with 5 MutationType ops +PostCommitPhase stage 1 of 6 with 5 MutationType ops transitions: [[Column:{DescID: 104, ColumnID: 2}, PUBLIC], DELETE_ONLY] -> WRITE_ONLY [[TemporaryIndex:{DescID: 104, IndexID: 3, SourceIndexID: 1}, TRANSIENT_ABSENT], DELETE_ONLY] -> WRITE_ONLY @@ -591,7 +591,7 @@ PostCommitPhase stage 1 of 7 with 5 MutationType ops DescriptorID: 104 *scop.UpdateSchemaChangerJob JobID: 1 -PostCommitPhase stage 2 of 7 with 1 BackfillType op +PostCommitPhase stage 2 of 6 with 1 BackfillType op transitions: [[PrimaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 1, TemporaryIndexID: 3, SourceIndexID: 1}, PUBLIC], BACKFILL_ONLY] -> BACKFILLED ops: @@ -599,7 +599,7 @@ PostCommitPhase stage 2 of 7 with 1 BackfillType op IndexID: 2 SourceIndexID: 1 TableID: 104 -PostCommitPhase stage 3 of 7 with 3 MutationType ops +PostCommitPhase stage 3 of 6 with 3 MutationType ops transitions: [[PrimaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 1, TemporaryIndexID: 3, SourceIndexID: 1}, PUBLIC], BACKFILLED] -> DELETE_ONLY ops: @@ -610,7 +610,7 @@ PostCommitPhase stage 3 of 7 with 3 MutationType ops DescriptorID: 104 *scop.UpdateSchemaChangerJob JobID: 1 -PostCommitPhase stage 4 of 7 with 3 MutationType ops +PostCommitPhase stage 4 of 6 with 3 MutationType ops transitions: [[PrimaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 1, TemporaryIndexID: 3, SourceIndexID: 1}, PUBLIC], DELETE_ONLY] -> MERGE_ONLY ops: @@ -621,7 +621,7 @@ PostCommitPhase stage 4 of 7 with 3 MutationType ops DescriptorID: 104 *scop.UpdateSchemaChangerJob JobID: 1 -PostCommitPhase stage 5 of 7 with 1 BackfillType op +PostCommitPhase stage 5 of 6 with 1 BackfillType op transitions: [[PrimaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 1, TemporaryIndexID: 3, SourceIndexID: 1}, PUBLIC], MERGE_ONLY] -> MERGED ops: @@ -629,20 +629,22 @@ PostCommitPhase stage 5 of 7 with 1 BackfillType op BackfilledIndexID: 2 TableID: 104 TemporaryIndexID: 3 -PostCommitPhase stage 6 of 7 with 1 ValidationType op +PostCommitPhase stage 6 of 6 with 1 ValidationType op transitions: [[PrimaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 1, TemporaryIndexID: 3, SourceIndexID: 1}, PUBLIC], MERGED] -> VALIDATED ops: *scop.ValidateUniqueIndex IndexID: 2 TableID: 104 -PostCommitPhase stage 7 of 7 with 10 MutationType ops +PostCommitNonRevertiblePhase stage 1 of 3 with 12 MutationType ops transitions: - [[PrimaryIndex:{DescID: 104, IndexID: 1, ConstraintID: 1}, ABSENT], PUBLIC] -> VALIDATED + [[IndexColumn:{DescID: 104, ColumnID: 1, IndexID: 1}, ABSENT], PUBLIC] -> ABSENT + [[PrimaryIndex:{DescID: 104, IndexID: 1, ConstraintID: 1}, ABSENT], PUBLIC] -> WRITE_ONLY [[IndexName:{DescID: 104, Name: foo_pkey, IndexID: 1}, ABSENT], PUBLIC] -> ABSENT [[Column:{DescID: 104, ColumnID: 2}, PUBLIC], WRITE_ONLY] -> PUBLIC [[PrimaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 1, TemporaryIndexID: 3, SourceIndexID: 1}, PUBLIC], VALIDATED] -> PUBLIC [[IndexName:{DescID: 104, Name: foo_pkey, IndexID: 2}, PUBLIC], ABSENT] -> PUBLIC + [[TemporaryIndex:{DescID: 104, IndexID: 3, SourceIndexID: 1}, TRANSIENT_ABSENT], WRITE_ONLY] -> TRANSIENT_DELETE_ONLY [[Column:{DescID: 104, ColumnID: 3}, PUBLIC], WRITE_ONLY] -> PUBLIC ops: *scop.MakeDroppedPrimaryIndexDeleteAndWriteOnly @@ -656,6 +658,13 @@ PostCommitPhase stage 7 of 7 with 10 MutationType ops IndexID: 2 Name: foo_pkey TableID: 104 + *scop.MakeDroppedIndexDeleteOnly + IndexID: 3 + TableID: 104 + *scop.RemoveColumnFromIndex + ColumnID: 1 + IndexID: 1 + TableID: 104 *scop.MakeAddedPrimaryIndexPublic EventBase: Authorization: @@ -699,19 +708,10 @@ PostCommitPhase stage 7 of 7 with 10 MutationType ops *scop.UpdateSchemaChangerJob IsNonCancelable: true JobID: 1 -PostCommitNonRevertiblePhase stage 1 of 2 with 5 MutationType ops +PostCommitNonRevertiblePhase stage 2 of 3 with 3 MutationType ops transitions: - [[IndexColumn:{DescID: 104, ColumnID: 1, IndexID: 1}, ABSENT], PUBLIC] -> ABSENT - [[PrimaryIndex:{DescID: 104, IndexID: 1, ConstraintID: 1}, ABSENT], VALIDATED] -> DELETE_ONLY - [[TemporaryIndex:{DescID: 104, IndexID: 3, SourceIndexID: 1}, TRANSIENT_ABSENT], WRITE_ONLY] -> TRANSIENT_DELETE_ONLY + [[PrimaryIndex:{DescID: 104, IndexID: 1, ConstraintID: 1}, ABSENT], WRITE_ONLY] -> DELETE_ONLY ops: - scop.RemoveColumnFromIndex - ColumnID: 1 - IndexID: 1 - TableID: 104 - *scop.MakeDroppedIndexDeleteOnly - IndexID: 3 - TableID: 104 *scop.MakeDroppedIndexDeleteOnly IndexID: 1 TableID: 104 @@ -720,11 +720,17 @@ PostCommitNonRevertiblePhase stage 1 of 2 with 5 MutationType ops *scop.UpdateSchemaChangerJob IsNonCancelable: true JobID: 1 -PostCommitNonRevertiblePhase stage 2 of 2 with 6 MutationType ops +PostCommitNonRevertiblePhase stage 3 of 3 with 6 MutationType ops transitions: [[PrimaryIndex:{DescID: 104, IndexID: 1, ConstraintID: 1}, ABSENT], DELETE_ONLY] -> ABSENT [[TemporaryIndex:{DescID: 104, IndexID: 3, SourceIndexID: 1}, TRANSIENT_ABSENT], TRANSIENT_DELETE_ONLY] -> TRANSIENT_ABSENT ops: + *scop.CreateGcJobForIndex + IndexID: 3 + TableID: 104 + *scop.MakeIndexAbsent + IndexID: 3 + TableID: 104 *scop.CreateGcJobForIndex IndexID: 1 StatementForDropJob: @@ -741,12 +747,6 @@ PostCommitNonRevertiblePhase stage 2 of 2 with 6 MutationType ops SubWorkID: 1 IndexID: 1 TableID: 104 - *scop.CreateGcJobForIndex - IndexID: 3 - TableID: 104 - *scop.MakeIndexAbsent - IndexID: 3 - TableID: 104 *scop.RemoveJobStateFromDescriptor DescriptorID: 104 JobID: 1 @@ -821,20 +821,20 @@ StatementPhase stage 1 of 1 with 10 MutationType ops IsUnique: true SourceIndexID: 1 TableID: 104 - scop.AddColumnToIndex + *scop.AddColumnToIndex ColumnID: 1 IndexID: 3 TableID: 104 - scop.AddColumnToIndex + *scop.AddColumnToIndex ColumnID: 2 IndexID: 3 Kind: 2 TableID: 104 - scop.AddColumnToIndex + *scop.AddColumnToIndex ColumnID: 1 IndexID: 2 TableID: 104 - scop.AddColumnToIndex + *scop.AddColumnToIndex ColumnID: 2 IndexID: 2 Kind: 2 @@ -851,13 +851,13 @@ PreCommitPhase stage 1 of 1 with 2 MutationType ops DescriptorIDs: - 104 JobID: 1 - RunningStatus: PostCommitPhase stage 1 of 7 with 2 MutationType ops pending + RunningStatus: PostCommitPhase stage 1 of 6 with 2 MutationType ops pending Statements: - statement: ALTER TABLE defaultdb.foo ADD COLUMN a INT8 AS (i + 1) STORED redactedstatement: ALTER TABLE ‹defaultdb›.public.‹foo› ADD COLUMN ‹a› INT8 AS (‹i› + ‹1›) STORED statementtag: ALTER TABLE -PostCommitPhase stage 1 of 7 with 4 MutationType ops +PostCommitPhase stage 1 of 6 with 4 MutationType ops transitions: [[Column:{DescID: 104, ColumnID: 2}, PUBLIC], DELETE_ONLY] -> WRITE_ONLY [[TemporaryIndex:{DescID: 104, IndexID: 3, SourceIndexID: 1}, TRANSIENT_ABSENT], DELETE_ONLY] -> WRITE_ONLY @@ -872,7 +872,7 @@ PostCommitPhase stage 1 of 7 with 4 MutationType ops DescriptorID: 104 *scop.UpdateSchemaChangerJob JobID: 1 -PostCommitPhase stage 2 of 7 with 1 BackfillType op +PostCommitPhase stage 2 of 6 with 1 BackfillType op transitions: [[PrimaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 1, TemporaryIndexID: 3, SourceIndexID: 1}, PUBLIC], BACKFILL_ONLY] -> BACKFILLED ops: @@ -880,7 +880,7 @@ PostCommitPhase stage 2 of 7 with 1 BackfillType op IndexID: 2 SourceIndexID: 1 TableID: 104 -PostCommitPhase stage 3 of 7 with 3 MutationType ops +PostCommitPhase stage 3 of 6 with 3 MutationType ops transitions: [[PrimaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 1, TemporaryIndexID: 3, SourceIndexID: 1}, PUBLIC], BACKFILLED] -> DELETE_ONLY ops: @@ -891,7 +891,7 @@ PostCommitPhase stage 3 of 7 with 3 MutationType ops DescriptorID: 104 *scop.UpdateSchemaChangerJob JobID: 1 -PostCommitPhase stage 4 of 7 with 3 MutationType ops +PostCommitPhase stage 4 of 6 with 3 MutationType ops transitions: [[PrimaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 1, TemporaryIndexID: 3, SourceIndexID: 1}, PUBLIC], DELETE_ONLY] -> MERGE_ONLY ops: @@ -902,7 +902,7 @@ PostCommitPhase stage 4 of 7 with 3 MutationType ops DescriptorID: 104 *scop.UpdateSchemaChangerJob JobID: 1 -PostCommitPhase stage 5 of 7 with 1 BackfillType op +PostCommitPhase stage 5 of 6 with 1 BackfillType op transitions: [[PrimaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 1, TemporaryIndexID: 3, SourceIndexID: 1}, PUBLIC], MERGE_ONLY] -> MERGED ops: @@ -910,20 +910,22 @@ PostCommitPhase stage 5 of 7 with 1 BackfillType op BackfilledIndexID: 2 TableID: 104 TemporaryIndexID: 3 -PostCommitPhase stage 6 of 7 with 1 ValidationType op +PostCommitPhase stage 6 of 6 with 1 ValidationType op transitions: [[PrimaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 1, TemporaryIndexID: 3, SourceIndexID: 1}, PUBLIC], MERGED] -> VALIDATED ops: *scop.ValidateUniqueIndex IndexID: 2 TableID: 104 -PostCommitPhase stage 7 of 7 with 8 MutationType ops +PostCommitNonRevertiblePhase stage 1 of 3 with 10 MutationType ops transitions: - [[PrimaryIndex:{DescID: 104, IndexID: 1, ConstraintID: 1}, ABSENT], PUBLIC] -> VALIDATED + [[IndexColumn:{DescID: 104, ColumnID: 1, IndexID: 1}, ABSENT], PUBLIC] -> ABSENT + [[PrimaryIndex:{DescID: 104, IndexID: 1, ConstraintID: 1}, ABSENT], PUBLIC] -> WRITE_ONLY [[IndexName:{DescID: 104, Name: foo_pkey, IndexID: 1}, ABSENT], PUBLIC] -> ABSENT [[Column:{DescID: 104, ColumnID: 2}, PUBLIC], WRITE_ONLY] -> PUBLIC [[PrimaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 1, TemporaryIndexID: 3, SourceIndexID: 1}, PUBLIC], VALIDATED] -> PUBLIC [[IndexName:{DescID: 104, Name: foo_pkey, IndexID: 2}, PUBLIC], ABSENT] -> PUBLIC + [[TemporaryIndex:{DescID: 104, IndexID: 3, SourceIndexID: 1}, TRANSIENT_ABSENT], WRITE_ONLY] -> TRANSIENT_DELETE_ONLY ops: *scop.MakeDroppedPrimaryIndexDeleteAndWriteOnly IndexID: 1 @@ -936,6 +938,13 @@ PostCommitPhase stage 7 of 7 with 8 MutationType ops IndexID: 2 Name: foo_pkey TableID: 104 + *scop.MakeDroppedIndexDeleteOnly + IndexID: 3 + TableID: 104 + *scop.RemoveColumnFromIndex + ColumnID: 1 + IndexID: 1 + TableID: 104 *scop.MakeAddedPrimaryIndexPublic EventBase: Authorization: @@ -967,19 +976,10 @@ PostCommitPhase stage 7 of 7 with 8 MutationType ops *scop.UpdateSchemaChangerJob IsNonCancelable: true JobID: 1 -PostCommitNonRevertiblePhase stage 1 of 2 with 5 MutationType ops +PostCommitNonRevertiblePhase stage 2 of 3 with 3 MutationType ops transitions: - [[IndexColumn:{DescID: 104, ColumnID: 1, IndexID: 1}, ABSENT], PUBLIC] -> ABSENT - [[PrimaryIndex:{DescID: 104, IndexID: 1, ConstraintID: 1}, ABSENT], VALIDATED] -> DELETE_ONLY - [[TemporaryIndex:{DescID: 104, IndexID: 3, SourceIndexID: 1}, TRANSIENT_ABSENT], WRITE_ONLY] -> TRANSIENT_DELETE_ONLY + [[PrimaryIndex:{DescID: 104, IndexID: 1, ConstraintID: 1}, ABSENT], WRITE_ONLY] -> DELETE_ONLY ops: - scop.RemoveColumnFromIndex - ColumnID: 1 - IndexID: 1 - TableID: 104 - *scop.MakeDroppedIndexDeleteOnly - IndexID: 3 - TableID: 104 *scop.MakeDroppedIndexDeleteOnly IndexID: 1 TableID: 104 @@ -988,11 +988,17 @@ PostCommitNonRevertiblePhase stage 1 of 2 with 5 MutationType ops *scop.UpdateSchemaChangerJob IsNonCancelable: true JobID: 1 -PostCommitNonRevertiblePhase stage 2 of 2 with 6 MutationType ops +PostCommitNonRevertiblePhase stage 3 of 3 with 6 MutationType ops transitions: [[PrimaryIndex:{DescID: 104, IndexID: 1, ConstraintID: 1}, ABSENT], DELETE_ONLY] -> ABSENT [[TemporaryIndex:{DescID: 104, IndexID: 3, SourceIndexID: 1}, TRANSIENT_ABSENT], TRANSIENT_DELETE_ONLY] -> TRANSIENT_ABSENT ops: + *scop.CreateGcJobForIndex + IndexID: 3 + TableID: 104 + *scop.MakeIndexAbsent + IndexID: 3 + TableID: 104 *scop.CreateGcJobForIndex IndexID: 1 StatementForDropJob: @@ -1010,12 +1016,6 @@ PostCommitNonRevertiblePhase stage 2 of 2 with 6 MutationType ops SubWorkID: 1 IndexID: 1 TableID: 104 - *scop.CreateGcJobForIndex - IndexID: 3 - TableID: 104 - *scop.MakeIndexAbsent - IndexID: 3 - TableID: 104 *scop.RemoveJobStateFromDescriptor DescriptorID: 104 JobID: 1 @@ -1077,7 +1077,7 @@ StatementPhase stage 1 of 1 with 10 MutationType ops family: IntFamily oid: 20 width: 64 - scop.AddColumnToIndex + *scop.AddColumnToIndex ColumnID: 2 IndexID: 1 Kind: 2 @@ -1117,7 +1117,7 @@ StatementPhase stage 1 of 1 with 10 MutationType ops family: IntFamily oid: 20 width: 64 - scop.AddColumnToIndex + *scop.AddColumnToIndex ColumnID: 3 IndexID: 1 Kind: 2 diff --git a/pkg/sql/schemachanger/scplan/testdata/create_index b/pkg/sql/schemachanger/scplan/testdata/create_index index 71f5a7255fea..f3b7e87c4fbd 100644 --- a/pkg/sql/schemachanger/scplan/testdata/create_index +++ b/pkg/sql/schemachanger/scplan/testdata/create_index @@ -29,30 +29,30 @@ StatementPhase stage 1 of 1 with 8 MutationType ops SourceIndexID: 1 TableID: 104 IsSecondaryIndex: true - scop.AddColumnToIndex + *scop.AddColumnToIndex ColumnID: 1 IndexID: 3 TableID: 104 - scop.AddColumnToIndex + *scop.AddColumnToIndex ColumnID: 2 IndexID: 3 Ordinal: 1 TableID: 104 - scop.AddColumnToIndex + *scop.AddColumnToIndex ColumnID: 3 IndexID: 3 Kind: 2 TableID: 104 - scop.AddColumnToIndex + *scop.AddColumnToIndex ColumnID: 1 IndexID: 2 TableID: 104 - scop.AddColumnToIndex + *scop.AddColumnToIndex ColumnID: 2 IndexID: 2 Ordinal: 1 TableID: 104 - scop.AddColumnToIndex + *scop.AddColumnToIndex ColumnID: 3 IndexID: 2 Kind: 2 @@ -69,13 +69,13 @@ PreCommitPhase stage 1 of 1 with 2 MutationType ops DescriptorIDs: - 104 JobID: 1 - RunningStatus: PostCommitPhase stage 1 of 7 with 1 MutationType op pending + RunningStatus: PostCommitPhase stage 1 of 6 with 1 MutationType op pending Statements: - statement: CREATE INDEX id1 ON defaultdb.t1 (id, name) STORING (money) redactedstatement: CREATE INDEX ‹id1› ON ‹defaultdb›.public.‹t1› (‹id›, ‹name›) STORING (‹money›) statementtag: CREATE INDEX -PostCommitPhase stage 1 of 7 with 3 MutationType ops +PostCommitPhase stage 1 of 6 with 3 MutationType ops transitions: [[TemporaryIndex:{DescID: 104, IndexID: 3, SourceIndexID: 1}, TRANSIENT_ABSENT], DELETE_ONLY] -> WRITE_ONLY ops: @@ -86,7 +86,7 @@ PostCommitPhase stage 1 of 7 with 3 MutationType ops DescriptorID: 104 *scop.UpdateSchemaChangerJob JobID: 1 -PostCommitPhase stage 2 of 7 with 1 BackfillType op +PostCommitPhase stage 2 of 6 with 1 BackfillType op transitions: [[SecondaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 0, TemporaryIndexID: 3, SourceIndexID: 1}, PUBLIC], BACKFILL_ONLY] -> BACKFILLED ops: @@ -94,7 +94,7 @@ PostCommitPhase stage 2 of 7 with 1 BackfillType op IndexID: 2 SourceIndexID: 1 TableID: 104 -PostCommitPhase stage 3 of 7 with 3 MutationType ops +PostCommitPhase stage 3 of 6 with 3 MutationType ops transitions: [[SecondaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 0, TemporaryIndexID: 3, SourceIndexID: 1}, PUBLIC], BACKFILLED] -> DELETE_ONLY ops: @@ -105,7 +105,7 @@ PostCommitPhase stage 3 of 7 with 3 MutationType ops DescriptorID: 104 *scop.UpdateSchemaChangerJob JobID: 1 -PostCommitPhase stage 4 of 7 with 3 MutationType ops +PostCommitPhase stage 4 of 6 with 3 MutationType ops transitions: [[SecondaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 0, TemporaryIndexID: 3, SourceIndexID: 1}, PUBLIC], DELETE_ONLY] -> MERGE_ONLY ops: @@ -116,7 +116,7 @@ PostCommitPhase stage 4 of 7 with 3 MutationType ops DescriptorID: 104 *scop.UpdateSchemaChangerJob JobID: 1 -PostCommitPhase stage 5 of 7 with 1 BackfillType op +PostCommitPhase stage 5 of 6 with 1 BackfillType op transitions: [[SecondaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 0, TemporaryIndexID: 3, SourceIndexID: 1}, PUBLIC], MERGE_ONLY] -> MERGED ops: @@ -124,37 +124,29 @@ PostCommitPhase stage 5 of 7 with 1 BackfillType op BackfilledIndexID: 2 TableID: 104 TemporaryIndexID: 3 -PostCommitPhase stage 6 of 7 with 1 ValidationType op +PostCommitPhase stage 6 of 6 with 1 ValidationType op transitions: [[SecondaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 0, TemporaryIndexID: 3, SourceIndexID: 1}, PUBLIC], MERGED] -> VALIDATED ops: *scop.ValidateUniqueIndex IndexID: 2 TableID: 104 -PostCommitPhase stage 7 of 7 with 4 MutationType ops +PostCommitNonRevertiblePhase stage 1 of 2 with 5 MutationType ops transitions: [[SecondaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 0, TemporaryIndexID: 3, SourceIndexID: 1}, PUBLIC], VALIDATED] -> PUBLIC [[IndexName:{DescID: 104, Name: id1, IndexID: 2}, PUBLIC], ABSENT] -> PUBLIC + [[TemporaryIndex:{DescID: 104, IndexID: 3, SourceIndexID: 1}, TRANSIENT_ABSENT], WRITE_ONLY] -> TRANSIENT_DELETE_ONLY ops: *scop.SetIndexName IndexID: 2 Name: id1 TableID: 104 - *scop.MakeAddedSecondaryIndexPublic - IndexID: 2 - TableID: 104 - *scop.SetJobStateOnDescriptor - DescriptorID: 104 - *scop.UpdateSchemaChangerJob - IsNonCancelable: true - JobID: 1 -PostCommitNonRevertiblePhase stage 1 of 2 with 3 MutationType ops - transitions: - [[TemporaryIndex:{DescID: 104, IndexID: 3, SourceIndexID: 1}, TRANSIENT_ABSENT], WRITE_ONLY] -> TRANSIENT_DELETE_ONLY - ops: *scop.MakeDroppedIndexDeleteOnly IndexID: 3 TableID: 104 + *scop.MakeAddedSecondaryIndexPublic + IndexID: 2 + TableID: 104 *scop.SetJobStateOnDescriptor DescriptorID: 104 *scop.UpdateSchemaChangerJob @@ -272,30 +264,30 @@ StatementPhase stage 1 of 1 with 8 MutationType ops SourceIndexID: 1 TableID: 104 IsSecondaryIndex: true - scop.AddColumnToIndex + *scop.AddColumnToIndex ColumnID: 1 IndexID: 3 TableID: 104 - scop.AddColumnToIndex + *scop.AddColumnToIndex ColumnID: 2 IndexID: 3 Ordinal: 1 TableID: 104 - scop.AddColumnToIndex + *scop.AddColumnToIndex ColumnID: 3 IndexID: 3 Kind: 2 TableID: 104 - scop.AddColumnToIndex + *scop.AddColumnToIndex ColumnID: 1 IndexID: 2 TableID: 104 - scop.AddColumnToIndex + *scop.AddColumnToIndex ColumnID: 2 IndexID: 2 Ordinal: 1 TableID: 104 - scop.AddColumnToIndex + *scop.AddColumnToIndex ColumnID: 3 IndexID: 2 Kind: 2 @@ -312,14 +304,14 @@ PreCommitPhase stage 1 of 1 with 2 MutationType ops DescriptorIDs: - 104 JobID: 1 - RunningStatus: PostCommitPhase stage 1 of 7 with 1 MutationType op pending + RunningStatus: PostCommitPhase stage 1 of 6 with 1 MutationType op pending Statements: - statement: CREATE INVERTED INDEX CONCURRENTLY id1 ON defaultdb.t1 (id, name) STORING (money) redactedstatement: CREATE INVERTED INDEX CONCURRENTLY ‹id1› ON ‹defaultdb›.public.‹t1› (‹id›, ‹name›) STORING (‹money›) statementtag: CREATE INDEX -PostCommitPhase stage 1 of 7 with 3 MutationType ops +PostCommitPhase stage 1 of 6 with 3 MutationType ops transitions: [[TemporaryIndex:{DescID: 104, IndexID: 3, SourceIndexID: 1}, TRANSIENT_ABSENT], DELETE_ONLY] -> WRITE_ONLY ops: @@ -330,7 +322,7 @@ PostCommitPhase stage 1 of 7 with 3 MutationType ops DescriptorID: 104 *scop.UpdateSchemaChangerJob JobID: 1 -PostCommitPhase stage 2 of 7 with 1 BackfillType op +PostCommitPhase stage 2 of 6 with 1 BackfillType op transitions: [[SecondaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 0, TemporaryIndexID: 3, SourceIndexID: 1}, PUBLIC], BACKFILL_ONLY] -> BACKFILLED ops: @@ -338,7 +330,7 @@ PostCommitPhase stage 2 of 7 with 1 BackfillType op IndexID: 2 SourceIndexID: 1 TableID: 104 -PostCommitPhase stage 3 of 7 with 3 MutationType ops +PostCommitPhase stage 3 of 6 with 3 MutationType ops transitions: [[SecondaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 0, TemporaryIndexID: 3, SourceIndexID: 1}, PUBLIC], BACKFILLED] -> DELETE_ONLY ops: @@ -349,7 +341,7 @@ PostCommitPhase stage 3 of 7 with 3 MutationType ops DescriptorID: 104 *scop.UpdateSchemaChangerJob JobID: 1 -PostCommitPhase stage 4 of 7 with 3 MutationType ops +PostCommitPhase stage 4 of 6 with 3 MutationType ops transitions: [[SecondaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 0, TemporaryIndexID: 3, SourceIndexID: 1}, PUBLIC], DELETE_ONLY] -> MERGE_ONLY ops: @@ -360,7 +352,7 @@ PostCommitPhase stage 4 of 7 with 3 MutationType ops DescriptorID: 104 *scop.UpdateSchemaChangerJob JobID: 1 -PostCommitPhase stage 5 of 7 with 1 BackfillType op +PostCommitPhase stage 5 of 6 with 1 BackfillType op transitions: [[SecondaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 0, TemporaryIndexID: 3, SourceIndexID: 1}, PUBLIC], MERGE_ONLY] -> MERGED ops: @@ -368,37 +360,29 @@ PostCommitPhase stage 5 of 7 with 1 BackfillType op BackfilledIndexID: 2 TableID: 104 TemporaryIndexID: 3 -PostCommitPhase stage 6 of 7 with 1 ValidationType op +PostCommitPhase stage 6 of 6 with 1 ValidationType op transitions: [[SecondaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 0, TemporaryIndexID: 3, SourceIndexID: 1}, PUBLIC], MERGED] -> VALIDATED ops: *scop.ValidateUniqueIndex IndexID: 2 TableID: 104 -PostCommitPhase stage 7 of 7 with 4 MutationType ops +PostCommitNonRevertiblePhase stage 1 of 2 with 5 MutationType ops transitions: [[SecondaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 0, TemporaryIndexID: 3, SourceIndexID: 1}, PUBLIC], VALIDATED] -> PUBLIC [[IndexName:{DescID: 104, Name: id1, IndexID: 2}, PUBLIC], ABSENT] -> PUBLIC + [[TemporaryIndex:{DescID: 104, IndexID: 3, SourceIndexID: 1}, TRANSIENT_ABSENT], WRITE_ONLY] -> TRANSIENT_DELETE_ONLY ops: *scop.SetIndexName IndexID: 2 Name: id1 TableID: 104 - *scop.MakeAddedSecondaryIndexPublic - IndexID: 2 - TableID: 104 - *scop.SetJobStateOnDescriptor - DescriptorID: 104 - *scop.UpdateSchemaChangerJob - IsNonCancelable: true - JobID: 1 -PostCommitNonRevertiblePhase stage 1 of 2 with 3 MutationType ops - transitions: - [[TemporaryIndex:{DescID: 104, IndexID: 3, SourceIndexID: 1}, TRANSIENT_ABSENT], WRITE_ONLY] -> TRANSIENT_DELETE_ONLY - ops: *scop.MakeDroppedIndexDeleteOnly IndexID: 3 TableID: 104 + *scop.MakeAddedSecondaryIndexPublic + IndexID: 2 + TableID: 104 *scop.SetJobStateOnDescriptor DescriptorID: 104 *scop.UpdateSchemaChangerJob diff --git a/pkg/sql/schemachanger/scplan/testdata/drop_index b/pkg/sql/schemachanger/scplan/testdata/drop_index index ec190d9eb33f..60aca0cd1634 100644 --- a/pkg/sql/schemachanger/scplan/testdata/drop_index +++ b/pkg/sql/schemachanger/scplan/testdata/drop_index @@ -54,11 +54,11 @@ PostCommitNonRevertiblePhase stage 1 of 2 with 5 MutationType ops *scop.MakeDroppedIndexDeleteOnly IndexID: 2 TableID: 104 - scop.RemoveColumnFromIndex + *scop.RemoveColumnFromIndex ColumnID: 1 IndexID: 2 TableID: 104 - scop.RemoveColumnFromIndex + *scop.RemoveColumnFromIndex ColumnID: 3 IndexID: 2 Kind: 1 @@ -210,11 +210,11 @@ PostCommitNonRevertiblePhase stage 1 of 2 with 6 MutationType ops *scop.MakeDroppedIndexDeleteOnly IndexID: 4 TableID: 104 - scop.RemoveColumnFromIndex + *scop.RemoveColumnFromIndex ColumnID: 4 IndexID: 4 TableID: 104 - scop.RemoveColumnFromIndex + *scop.RemoveColumnFromIndex ColumnID: 3 IndexID: 4 Kind: 1 @@ -415,16 +415,16 @@ PostCommitNonRevertiblePhase stage 1 of 2 with 9 MutationType ops *scop.MakeDroppedIndexDeleteOnly IndexID: 6 TableID: 104 - scop.RemoveColumnFromIndex + *scop.RemoveColumnFromIndex ColumnID: 5 IndexID: 6 TableID: 104 - scop.RemoveColumnFromIndex + *scop.RemoveColumnFromIndex ColumnID: 1 IndexID: 6 Ordinal: 1 TableID: 104 - scop.RemoveColumnFromIndex + *scop.RemoveColumnFromIndex ColumnID: 3 IndexID: 6 Kind: 1 @@ -634,11 +634,11 @@ PostCommitNonRevertiblePhase stage 1 of 2 with 10 MutationType ops DescriptorID: 105 Name: v SchemaID: 101 - scop.RemoveColumnFromIndex + *scop.RemoveColumnFromIndex ColumnID: 2 IndexID: 8 TableID: 104 - scop.RemoveColumnFromIndex + *scop.RemoveColumnFromIndex ColumnID: 3 IndexID: 8 Kind: 1 diff --git a/pkg/sql/schemachanger/screl/scalars.go b/pkg/sql/schemachanger/screl/scalars.go index 4d7dc7c01298..673c1c46b5e7 100644 --- a/pkg/sql/schemachanger/screl/scalars.go +++ b/pkg/sql/schemachanger/screl/scalars.go @@ -31,6 +31,15 @@ func GetDescID(e scpb.Element) catid.DescID { return id.(catid.DescID) } +// GetIndexID retrieves the index ID from the element if it has one. +func GetIndexID(e scpb.Element) (catid.IndexID, bool) { + v, err := Schema.GetAttribute(IndexID, e) + if err != nil { + return 0, false + } + return v.(catid.IndexID), true +} + // AllTargetDescIDs returns all the descriptor IDs referenced in the // target state's elements. This is a superset of the IDs of the descriptors // affected by the schema change. diff --git a/pkg/sql/schemachanger/testdata/alter_table_add_column b/pkg/sql/schemachanger/testdata/alter_table_add_column index 74d03841ac14..fcb4f0b87659 100644 --- a/pkg/sql/schemachanger/testdata/alter_table_add_column +++ b/pkg/sql/schemachanger/testdata/alter_table_add_column @@ -324,7 +324,7 @@ notified job registry to adopt jobs: [1] begin transaction #2 commit transaction #2 begin transaction #3 -## PostCommitPhase stage 1 of 7 with 4 MutationType ops +## PostCommitPhase stage 1 of 6 with 4 MutationType ops upsert descriptor #106 ... - PUBLIC @@ -367,14 +367,14 @@ upsert descriptor #106 unexposedParentSchemaId: 105 - version: "2" + version: "3" -update progress of schema change job #1: "PostCommitPhase stage 2 of 7 with 1 BackfillType op pending" +update progress of schema change job #1: "PostCommitPhase stage 2 of 6 with 1 BackfillType op pending" commit transaction #3 begin transaction #4 -## PostCommitPhase stage 2 of 7 with 1 BackfillType op +## PostCommitPhase stage 2 of 6 with 1 BackfillType op backfill indexes [2] from index #1 in table #106 commit transaction #4 begin transaction #5 -## PostCommitPhase stage 3 of 7 with 3 MutationType ops +## PostCommitPhase stage 3 of 6 with 3 MutationType ops upsert descriptor #106 ... - PUBLIC @@ -403,10 +403,10 @@ upsert descriptor #106 unexposedParentSchemaId: 105 - version: "3" + version: "4" -update progress of schema change job #1: "PostCommitPhase stage 4 of 7 with 1 MutationType op pending" +update progress of schema change job #1: "PostCommitPhase stage 4 of 6 with 1 MutationType op pending" commit transaction #5 begin transaction #6 -## PostCommitPhase stage 4 of 7 with 3 MutationType ops +## PostCommitPhase stage 4 of 6 with 3 MutationType ops upsert descriptor #106 ... - PUBLIC @@ -435,18 +435,18 @@ upsert descriptor #106 unexposedParentSchemaId: 105 - version: "4" + version: "5" -update progress of schema change job #1: "PostCommitPhase stage 5 of 7 with 1 BackfillType op pending" +update progress of schema change job #1: "PostCommitPhase stage 5 of 6 with 1 BackfillType op pending" commit transaction #6 begin transaction #7 -## PostCommitPhase stage 5 of 7 with 1 BackfillType op +## PostCommitPhase stage 5 of 6 with 1 BackfillType op merge temporary indexes [3] into backfilled indexes [2] in table #106 commit transaction #7 begin transaction #8 -## PostCommitPhase stage 6 of 7 with 1 ValidationType op +## PostCommitPhase stage 6 of 6 with 1 ValidationType op validate forward indexes [2] in table #106 commit transaction #8 begin transaction #9 -## PostCommitPhase stage 7 of 7 with 8 MutationType ops +## PostCommitNonRevertiblePhase stage 1 of 3 with 10 MutationType ops upsert descriptor #106 ... oid: 20 @@ -462,10 +462,12 @@ upsert descriptor #106 createAsOfTime: wallTime: "1640995200000000000" ... + userName: root currentStatuses: - - PUBLIC - + - VALIDATED + - ABSENT + + - WRITE_ONLY + + - ABSENT + - PUBLIC - PUBLIC - PUBLIC - - WRITE_ONLY @@ -476,8 +478,9 @@ upsert descriptor #106 - PUBLIC - - MERGE_ONLY - - ABSENT - + - PUBLIC - - WRITE_ONLY + - - WRITE_ONLY + + - TRANSIENT_DELETE_ONLY + - PUBLIC - PUBLIC ... statement: ALTER TABLE db.public.tbl ADD COLUMN j INT8 NOT NULL DEFAULT 42 @@ -504,7 +507,8 @@ upsert descriptor #106 - direction: ADD - mutationId: 1 - state: DELETE_AND_WRITE_ONLY - - direction: ADD + - - direction: ADD + + - direction: DROP index: - constraintId: 2 + constraintId: 3 @@ -529,8 +533,9 @@ upsert descriptor #106 + useDeletePreservingEncoding: true version: 4 mutationId: 1 - state: DELETE_AND_WRITE_ONLY + - state: DELETE_AND_WRITE_ONLY - - direction: ADD + + state: DELETE_ONLY + - direction: DROP index: - constraintId: 3 @@ -588,28 +593,18 @@ upsert descriptor #106 - version: "5" + version: "6" adding table for stats refresh: 106 -update progress of schema change job #1: "PostCommitNonRevertiblePhase stage 1 of 2 with 3 MutationType ops pending" +update progress of schema change job #1: "PostCommitNonRevertiblePhase stage 2 of 3 with 1 MutationType op pending" set schema change job #1 to non-cancellable commit transaction #9 begin transaction #10 -## PostCommitNonRevertiblePhase stage 1 of 2 with 5 MutationType ops +## PostCommitNonRevertiblePhase stage 2 of 3 with 3 MutationType ops upsert descriptor #106 ... - userName: root currentStatuses: - - - PUBLIC - - - VALIDATED - ABSENT - + - DELETE_ONLY - + - ABSENT - - PUBLIC - - PUBLIC - ... - - PUBLIC - - PUBLIC - - WRITE_ONLY - + - TRANSIENT_DELETE_ONLY - - PUBLIC + + - DELETE_ONLY + - ABSENT - PUBLIC ... formatVersion: 3 @@ -618,17 +613,7 @@ upsert descriptor #106 - wallTime: "1640995200000000009" + modificationTime: {} mutations: - - - direction: ADD - + - direction: DROP - index: - constraintId: 3 - ... - version: 4 - mutationId: 1 - - state: DELETE_AND_WRITE_ONLY - + state: DELETE_ONLY - direction: DROP - index: ... version: 4 mutationId: 1 @@ -641,10 +626,10 @@ upsert descriptor #106 unexposedParentSchemaId: 105 - version: "6" + version: "7" -update progress of schema change job #1: "PostCommitNonRevertiblePhase stage 2 of 2 with 4 MutationType ops pending" +update progress of schema change job #1: "PostCommitNonRevertiblePhase stage 3 of 3 with 4 MutationType ops pending" commit transaction #10 begin transaction #11 -## PostCommitNonRevertiblePhase stage 2 of 2 with 6 MutationType ops +## PostCommitNonRevertiblePhase stage 3 of 3 with 6 MutationType ops upsert descriptor #106 ... createAsOfTime: diff --git a/pkg/sql/schemachanger/testdata/alter_table_add_column_default_seq b/pkg/sql/schemachanger/testdata/alter_table_add_column_default_seq index 186c9f75c281..17fcf73a0e56 100644 --- a/pkg/sql/schemachanger/testdata/alter_table_add_column_default_seq +++ b/pkg/sql/schemachanger/testdata/alter_table_add_column_default_seq @@ -371,7 +371,7 @@ notified job registry to adopt jobs: [1] begin transaction #2 commit transaction #2 begin transaction #3 -## PostCommitPhase stage 1 of 7 with 5 MutationType ops +## PostCommitPhase stage 1 of 6 with 5 MutationType ops upsert descriptor #106 ... - PUBLIC @@ -428,14 +428,14 @@ upsert descriptor #107 unexposedParentSchemaId: 105 - version: "2" + version: "3" -update progress of schema change job #1: "PostCommitPhase stage 2 of 7 with 1 BackfillType op pending" +update progress of schema change job #1: "PostCommitPhase stage 2 of 6 with 1 BackfillType op pending" commit transaction #3 begin transaction #4 -## PostCommitPhase stage 2 of 7 with 1 BackfillType op +## PostCommitPhase stage 2 of 6 with 1 BackfillType op backfill indexes [2] from index #1 in table #106 commit transaction #4 begin transaction #5 -## PostCommitPhase stage 3 of 7 with 4 MutationType ops +## PostCommitPhase stage 3 of 6 with 4 MutationType ops upsert descriptor #106 ... - PUBLIC @@ -478,10 +478,10 @@ upsert descriptor #107 unexposedParentSchemaId: 105 - version: "3" + version: "4" -update progress of schema change job #1: "PostCommitPhase stage 4 of 7 with 1 MutationType op pending" +update progress of schema change job #1: "PostCommitPhase stage 4 of 6 with 1 MutationType op pending" commit transaction #5 begin transaction #6 -## PostCommitPhase stage 4 of 7 with 4 MutationType ops +## PostCommitPhase stage 4 of 6 with 4 MutationType ops upsert descriptor #106 ... - PUBLIC @@ -524,18 +524,18 @@ upsert descriptor #107 unexposedParentSchemaId: 105 - version: "4" + version: "5" -update progress of schema change job #1: "PostCommitPhase stage 5 of 7 with 1 BackfillType op pending" +update progress of schema change job #1: "PostCommitPhase stage 5 of 6 with 1 BackfillType op pending" commit transaction #6 begin transaction #7 -## PostCommitPhase stage 5 of 7 with 1 BackfillType op +## PostCommitPhase stage 5 of 6 with 1 BackfillType op merge temporary indexes [3] into backfilled indexes [2] in table #106 commit transaction #7 begin transaction #8 -## PostCommitPhase stage 6 of 7 with 1 ValidationType op +## PostCommitPhase stage 6 of 6 with 1 ValidationType op validate forward indexes [2] in table #106 commit transaction #8 begin transaction #9 -## PostCommitPhase stage 7 of 7 with 9 MutationType ops +## PostCommitNonRevertiblePhase stage 1 of 3 with 11 MutationType ops upsert descriptor #106 ... oid: 20 @@ -553,10 +553,12 @@ upsert descriptor #106 createAsOfTime: wallTime: "1640995200000000000" ... + userName: root currentStatuses: - - PUBLIC - + - VALIDATED + - ABSENT + + - WRITE_ONLY + + - ABSENT + - PUBLIC - PUBLIC - PUBLIC - - WRITE_ONLY @@ -567,8 +569,9 @@ upsert descriptor #106 - PUBLIC - - MERGE_ONLY - - ABSENT - + - PUBLIC - - WRITE_ONLY + - - WRITE_ONLY + + - TRANSIENT_DELETE_ONLY + - PUBLIC - PUBLIC ... statement: ALTER TABLE db.public.tbl ADD COLUMN l INT8 NOT NULL DEFAULT nextval('db.public.sq1') @@ -597,7 +600,8 @@ upsert descriptor #106 - direction: ADD - mutationId: 1 - state: DELETE_AND_WRITE_ONLY - - direction: ADD + - - direction: ADD + + - direction: DROP index: - constraintId: 2 + constraintId: 3 @@ -622,8 +626,9 @@ upsert descriptor #106 + useDeletePreservingEncoding: true version: 4 mutationId: 1 - state: DELETE_AND_WRITE_ONLY + - state: DELETE_AND_WRITE_ONLY - - direction: ADD + + state: DELETE_ONLY + - direction: DROP index: - constraintId: 3 @@ -701,28 +706,18 @@ upsert descriptor #107 - version: "5" + version: "6" adding table for stats refresh: 106 -update progress of schema change job #1: "PostCommitNonRevertiblePhase stage 1 of 2 with 3 MutationType ops pending" +update progress of schema change job #1: "PostCommitNonRevertiblePhase stage 2 of 3 with 1 MutationType op pending" set schema change job #1 to non-cancellable commit transaction #9 begin transaction #10 -## PostCommitNonRevertiblePhase stage 1 of 2 with 6 MutationType ops +## PostCommitNonRevertiblePhase stage 2 of 3 with 4 MutationType ops upsert descriptor #106 ... - userName: root currentStatuses: - - - PUBLIC - - - VALIDATED - ABSENT - + - DELETE_ONLY - + - ABSENT - - PUBLIC - - PUBLIC - ... - - PUBLIC - - PUBLIC - - WRITE_ONLY - + - TRANSIENT_DELETE_ONLY - - PUBLIC + + - DELETE_ONLY + - ABSENT - PUBLIC ... formatVersion: 3 @@ -731,17 +726,7 @@ upsert descriptor #106 - wallTime: "1640995200000000009" + modificationTime: {} mutations: - - - direction: ADD - + - direction: DROP - index: - constraintId: 3 - ... - version: 4 - mutationId: 1 - - state: DELETE_AND_WRITE_ONLY - + state: DELETE_ONLY - direction: DROP - index: ... version: 4 mutationId: 1 @@ -768,10 +753,10 @@ upsert descriptor #107 unexposedParentSchemaId: 105 - version: "6" + version: "7" -update progress of schema change job #1: "PostCommitNonRevertiblePhase stage 2 of 2 with 4 MutationType ops pending" +update progress of schema change job #1: "PostCommitNonRevertiblePhase stage 3 of 3 with 4 MutationType ops pending" commit transaction #10 begin transaction #11 -## PostCommitNonRevertiblePhase stage 2 of 2 with 7 MutationType ops +## PostCommitNonRevertiblePhase stage 3 of 3 with 7 MutationType ops upsert descriptor #106 ... createAsOfTime: diff --git a/pkg/sql/schemachanger/testdata/create_index b/pkg/sql/schemachanger/testdata/create_index index 844d74de2625..282369eb1f6b 100644 --- a/pkg/sql/schemachanger/testdata/create_index +++ b/pkg/sql/schemachanger/testdata/create_index @@ -205,7 +205,7 @@ notified job registry to adopt jobs: [1] begin transaction #2 commit transaction #2 begin transaction #3 -## PostCommitPhase stage 1 of 7 with 3 MutationType ops +## PostCommitPhase stage 1 of 6 with 3 MutationType ops upsert descriptor #106 ... - BACKFILL_ONLY @@ -234,14 +234,14 @@ upsert descriptor #106 unexposedParentSchemaId: 101 - version: "2" + version: "3" -update progress of schema change job #1: "PostCommitPhase stage 2 of 7 with 1 BackfillType op pending" +update progress of schema change job #1: "PostCommitPhase stage 2 of 6 with 1 BackfillType op pending" commit transaction #3 begin transaction #4 -## PostCommitPhase stage 2 of 7 with 1 BackfillType op +## PostCommitPhase stage 2 of 6 with 1 BackfillType op backfill indexes [2] from index #1 in table #106 commit transaction #4 begin transaction #5 -## PostCommitPhase stage 3 of 7 with 3 MutationType ops +## PostCommitPhase stage 3 of 6 with 3 MutationType ops upsert descriptor #106 ... - PUBLIC @@ -270,10 +270,10 @@ upsert descriptor #106 unexposedParentSchemaId: 101 - version: "3" + version: "4" -update progress of schema change job #1: "PostCommitPhase stage 4 of 7 with 1 MutationType op pending" +update progress of schema change job #1: "PostCommitPhase stage 4 of 6 with 1 MutationType op pending" commit transaction #5 begin transaction #6 -## PostCommitPhase stage 4 of 7 with 3 MutationType ops +## PostCommitPhase stage 4 of 6 with 3 MutationType ops upsert descriptor #106 ... - PUBLIC @@ -302,28 +302,32 @@ upsert descriptor #106 unexposedParentSchemaId: 101 - version: "4" + version: "5" -update progress of schema change job #1: "PostCommitPhase stage 5 of 7 with 1 BackfillType op pending" +update progress of schema change job #1: "PostCommitPhase stage 5 of 6 with 1 BackfillType op pending" commit transaction #6 begin transaction #7 -## PostCommitPhase stage 5 of 7 with 1 BackfillType op +## PostCommitPhase stage 5 of 6 with 1 BackfillType op merge temporary indexes [3] into backfilled indexes [2] in table #106 commit transaction #7 begin transaction #8 -## PostCommitPhase stage 6 of 7 with 1 ValidationType op +## PostCommitPhase stage 6 of 6 with 1 ValidationType op validate forward indexes [2] in table #106 commit transaction #8 begin transaction #9 -## PostCommitPhase stage 7 of 7 with 4 MutationType ops +## PostCommitNonRevertiblePhase stage 1 of 2 with 5 MutationType ops upsert descriptor #106 ... - PUBLIC - PUBLIC - - MERGE_ONLY - - ABSENT + - - WRITE_ONLY + - PUBLIC + - PUBLIC + + - TRANSIENT_DELETE_ONLY + - PUBLIC + - PUBLIC - - WRITE_ONLY - - PUBLIC + jobId: "1" + relevantStatements: ... statement: CREATE INDEX idx1 ON t (v) WHERE (v = 'a') statementTag: CREATE INDEX @@ -357,7 +361,8 @@ upsert descriptor #106 + version: 4 + modificationTime: {} mutations: - - direction: ADD + - - direction: ADD + + - direction: DROP index: - constraintId: 2 - createdExplicitly: true @@ -384,35 +389,6 @@ upsert descriptor #106 - index: constraintId: 3 createdExplicitly: true - ... - time: {} - unexposedParentSchemaId: 101 - - version: "5" - + version: "6" -update progress of schema change job #1: "PostCommitNonRevertiblePhase stage 1 of 2 with 1 MutationType op pending" -set schema change job #1 to non-cancellable -commit transaction #9 -begin transaction #10 -## PostCommitNonRevertiblePhase stage 1 of 2 with 3 MutationType ops -upsert descriptor #106 - ... - - PUBLIC - - PUBLIC - - - WRITE_ONLY - + - TRANSIENT_DELETE_ONLY - - PUBLIC - - PUBLIC - ... - storeColumnNames: [] - version: 4 - - modificationTime: - - wallTime: "1640995200000000009" - + modificationTime: {} - mutations: - - - direction: ADD - + - direction: DROP - index: - constraintId: 3 ... version: 4 mutationId: 1 @@ -423,11 +399,12 @@ upsert descriptor #106 ... time: {} unexposedParentSchemaId: 101 - - version: "6" - + version: "7" + - version: "5" + + version: "6" update progress of schema change job #1: "PostCommitNonRevertiblePhase stage 2 of 2 with 2 MutationType ops pending" -commit transaction #10 -begin transaction #11 +set schema change job #1 to non-cancellable +commit transaction #9 +begin transaction #10 ## PostCommitNonRevertiblePhase stage 2 of 2 with 4 MutationType ops upsert descriptor #106 ... @@ -533,7 +510,7 @@ upsert descriptor #106 storeColumnNames: [] version: 4 - modificationTime: - - wallTime: "1640995200000000010" + - wallTime: "1640995200000000009" - mutations: - - direction: DROP - index: @@ -566,12 +543,12 @@ upsert descriptor #106 ... time: {} unexposedParentSchemaId: 101 - - version: "7" - + version: "8" + - version: "6" + + version: "7" write *eventpb.FinishSchemaChange to event log for descriptor 106 create job #2 (non-cancelable: true): "GC for " descriptor IDs: [106] update progress of schema change job #1: "all stages completed" -commit transaction #11 +commit transaction #10 notified job registry to adopt jobs: [2] # end PostCommitPhase