From a8c161c581abb7b22a18b8c937d42a4147fce358 Mon Sep 17 00:00:00 2001 From: Erik Grinaker Date: Mon, 13 Mar 2023 13:48:25 +0000 Subject: [PATCH 1/3] roachtest: maybe deflake `change-replicas/mixed-version` Epic: none Release note: None --- pkg/cmd/roachtest/tests/mixed_version_change_replicas.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/roachtest/tests/mixed_version_change_replicas.go b/pkg/cmd/roachtest/tests/mixed_version_change_replicas.go index 31eed799dc1c..d7e16843de76 100644 --- a/pkg/cmd/roachtest/tests/mixed_version_change_replicas.go +++ b/pkg/cmd/roachtest/tests/mixed_version_change_replicas.go @@ -125,10 +125,11 @@ func runChangeReplicasMixedVersion(ctx context.Context, t test.Test, c cluster.C InitialBackoff: 100 * time.Millisecond, MaxBackoff: 5 * time.Second, Multiplier: 2, - MaxRetries: 8, + MaxRetries: 12, } var rangeErrors map[int]string for r := retry.StartWithCtx(ctx, retryOpts); r.Next(); { + setReplicateQueueEnabled(false) if errCount := len(rangeErrors); errCount > 0 { t.L().Printf("%d ranges failed, retrying", errCount) } @@ -153,6 +154,10 @@ func runChangeReplicasMixedVersion(ctx context.Context, t test.Test, c cluster.C if len(rangeErrors) == 0 { break } + // The failure may be caused by conflicts with ongoing configuration + // changes by the replicate queue, so we re-enable it and let it run + // for a bit before the next retry. + setReplicateQueueEnabled(true) } if len(rangeErrors) > 0 { From be31d22ab7e3a7b75b7640809786371515a5eefc Mon Sep 17 00:00:00 2001 From: Xiang Gu Date: Mon, 13 Mar 2023 12:09:12 -0400 Subject: [PATCH 2/3] upgrade: Remove two to-be-deleted-V22_2 cluster versions This commit removed the following two cluster versions and its associated upgrade logic and tests: - V22_2UpgradeSequenceToBeReferencedByID - V22_2UpdateInvalidColumnIDsInSequenceBackReferences Informs: #96763, #96751 Release Note: None --- pkg/clusterversion/cockroach_versions.go | 17 - pkg/upgrade/upgrades/BUILD.bazel | 9 - ..._column_ids_in_sequence_back_references.go | 140 -------- ..._sequence_back_references_external_test.go | 199 ----------- ...upgrade_sequence_to_be_referenced_by_ID.go | 325 ------------------ ...ce_to_be_referenced_by_ID_external_test.go | 153 --------- pkg/upgrade/upgrades/upgrades.go | 11 - 7 files changed, 854 deletions(-) delete mode 100644 pkg/upgrade/upgrades/update_invalid_column_ids_in_sequence_back_references.go delete mode 100644 pkg/upgrade/upgrades/update_invalid_column_ids_in_sequence_back_references_external_test.go delete mode 100644 pkg/upgrade/upgrades/upgrade_sequence_to_be_referenced_by_ID.go delete mode 100644 pkg/upgrade/upgrades/upgrade_sequence_to_be_referenced_by_ID_external_test.go diff --git a/pkg/clusterversion/cockroach_versions.go b/pkg/clusterversion/cockroach_versions.go index 0c174d74e4eb..3a80887e65ec 100644 --- a/pkg/clusterversion/cockroach_versions.go +++ b/pkg/clusterversion/cockroach_versions.go @@ -203,11 +203,6 @@ const ( TODODelete_V22_2RemoveGrantPrivilege // TODODelete_V22_2MVCCRangeTombstones enables the use of MVCC range tombstones. TODODelete_V22_2MVCCRangeTombstones - // TODODelete_V22_2UpgradeSequenceToBeReferencedByID ensures that sequences are referenced - // by IDs rather than by their names. For example, a column's DEFAULT (or - // ON UPDATE) expression can be defined to be 'nextval('s')'; we want to be - // able to refer to sequence 's' by its ID, since 's' might be later renamed. - TODODelete_V22_2UpgradeSequenceToBeReferencedByID // TODODelete_V22_2SampledStmtDiagReqs enables installing statement diagnostic requests that // probabilistically collects stmt bundles, controlled by the user provided // sampling rate. @@ -273,10 +268,6 @@ const ( // schema changes to complete. After this point, no non-MVCC // AddSSTable calls will be used outside of tenant streaming. TODODelete_V22_2NoNonMVCCAddSSTable - // TODODelete_V22_2UpdateInvalidColumnIDsInSequenceBackReferences looks for invalid column - // ids in sequences' back references and attempts a best-effort-based matching - // to update those column IDs. - TODODelete_V22_2UpdateInvalidColumnIDsInSequenceBackReferences // TODODelete_V22_2TTLDistSQL uses DistSQL to distribute TTL SELECT/DELETE statements to // leaseholder nodes. TODODelete_V22_2TTLDistSQL @@ -579,10 +570,6 @@ var rawVersionsSingleton = keyedVersions{ Key: TODODelete_V22_2MVCCRangeTombstones, Version: roachpb.Version{Major: 22, Minor: 1, Internal: 16}, }, - { - Key: TODODelete_V22_2UpgradeSequenceToBeReferencedByID, - Version: roachpb.Version{Major: 22, Minor: 1, Internal: 18}, - }, { Key: TODODelete_V22_2SampledStmtDiagReqs, Version: roachpb.Version{Major: 22, Minor: 1, Internal: 20}, @@ -655,10 +642,6 @@ var rawVersionsSingleton = keyedVersions{ Key: TODODelete_V22_2NoNonMVCCAddSSTable, Version: roachpb.Version{Major: 22, Minor: 1, Internal: 62}, }, - { - Key: TODODelete_V22_2UpdateInvalidColumnIDsInSequenceBackReferences, - Version: roachpb.Version{Major: 22, Minor: 1, Internal: 66}, - }, { Key: TODODelete_V22_2TTLDistSQL, Version: roachpb.Version{Major: 22, Minor: 1, Internal: 68}, diff --git a/pkg/upgrade/upgrades/BUILD.bazel b/pkg/upgrade/upgrades/BUILD.bazel index 1d6d64276dd9..99b178582be9 100644 --- a/pkg/upgrade/upgrades/BUILD.bazel +++ b/pkg/upgrade/upgrades/BUILD.bazel @@ -29,8 +29,6 @@ go_library( "system_privileges_user_id_migration.go", "system_rbr_indexes.go", "tenant_table_migration.go", - "update_invalid_column_ids_in_sequence_back_references.go", - "upgrade_sequence_to_be_referenced_by_ID.go", "upgrades.go", "wait_for_del_range_in_gc_job.go", "wait_for_schema_changes.go", @@ -50,21 +48,16 @@ go_library( "//pkg/roachpb", "//pkg/security/username", "//pkg/settings/cluster", - "//pkg/sql", "//pkg/sql/catalog", "//pkg/sql/catalog/catalogkeys", - "//pkg/sql/catalog/descbuilder", "//pkg/sql/catalog/descidgen", "//pkg/sql/catalog/descpb", "//pkg/sql/catalog/descs", - "//pkg/sql/catalog/resolver", "//pkg/sql/catalog/schematelemetry/schematelemetrycontroller", - "//pkg/sql/catalog/seqexpr", "//pkg/sql/catalog/systemschema", "//pkg/sql/catalog/tabledesc", "//pkg/sql/enum", "//pkg/sql/isql", - "//pkg/sql/parser", "//pkg/sql/pgwire/pgcode", "//pkg/sql/pgwire/pgerror", "//pkg/sql/schemachanger/scpb", @@ -117,8 +110,6 @@ go_test( "system_privileges_user_id_migration_test.go", "system_rbr_indexes_test.go", "tenant_table_migration_test.go", - "update_invalid_column_ids_in_sequence_back_references_external_test.go", - "upgrade_sequence_to_be_referenced_by_ID_external_test.go", "wait_for_del_range_in_gc_job_test.go", "wait_for_schema_changes_test.go", "web_sessions_table_user_id_migration_test.go", diff --git a/pkg/upgrade/upgrades/update_invalid_column_ids_in_sequence_back_references.go b/pkg/upgrade/upgrades/update_invalid_column_ids_in_sequence_back_references.go deleted file mode 100644 index d17cf8d25d54..000000000000 --- a/pkg/upgrade/upgrades/update_invalid_column_ids_in_sequence_back_references.go +++ /dev/null @@ -1,140 +0,0 @@ -// 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 upgrades - -import ( - "context" - "reflect" - "sort" - - "github.com/cockroachdb/cockroach/pkg/clusterversion" - "github.com/cockroachdb/cockroach/pkg/kv" - "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" - "github.com/cockroachdb/cockroach/pkg/sql/catalog/descs" - "github.com/cockroachdb/cockroach/pkg/upgrade" - "github.com/cockroachdb/cockroach/pkg/util/log" - "github.com/cockroachdb/errors" -) - -func updateInvalidColumnIDsInSequenceBackReferences( - ctx context.Context, _ clusterversion.ClusterVersion, d upgrade.TenantDeps, -) error { - // Scan all sequences and repair those with 0-valued column IDs in their back references, - // one transaction for each repair. - var lastSeqID descpb.ID - - for { - var currSeqID descpb.ID - var done bool - if err := d.DB.DescsTxn(ctx, func( - ctx context.Context, txn descs.Txn, - ) (err error) { - currSeqID = lastSeqID - for { - done, currSeqID, err = findNextTableToUpgrade(ctx, txn, txn.KV(), currSeqID, - func(table *descpb.TableDescriptor) bool { - return table.IsSequence() - }) - if err != nil || done { - return err - } - - // Sequence `nextIdToUpgrade` might contain back reference with invalid column IDs. If so, we need to - // update them with valid column IDs. - hasUpgrade, err := maybeUpdateInvalidColumnIdsInSequenceBackReferences( - ctx, txn.KV(), currSeqID, txn.Descriptors(), - ) - if err != nil { - return err - } - if hasUpgrade { - return nil - } - } - }); err != nil { - return err - } - - // Break out of the loop if we upgraded all sequences. - if done { - break - } - lastSeqID = currSeqID - } - - return nil -} - -// maybeUpdateInvalidColumnIdsInSequenceBackReferences looks at sequence, identified -// by `idToUpgrade`, and upgrade invalid column IDs in its back references, if any. -// -// The upgrade works by reconstructing the referenced column IDs in each back reference -// in the sequence's `DependedOnBy` field. Reconstruction is possible with the help of -// the `UsesSequenceIDs` field of each column in the referencing table. -// -// A canonical example to create a corrupt sequence descriptor with invalid column IDs is -// to run the following in v21.1: -// `CREATE SEQUENCE s; CREATE TABLE t (i INT PRIMARY KEY);` -// followed by -// `ALTER TABLE t ADD COLUMN j INT DEFAULT nextval('s'), ADD COLUMN k INT DEFAULT nextval('s')` -// which erroneously added the back reference in sequence `s` before allocating a column ID -// to the newly added column, causing the reference in `s.DependedOnBy` to be -// `ref.ColumnIDs = [0, 0]` while it should instead be `ref.ColumnIDs = [2, 3]`. -// This upgrade logic will look at the `UsesSequenceIds` field of each column (`i`, `j`, and `k`) -// in `t`. In this case, exactly two columns (`j` and `k`) will have `UsesSequenceIds` that -// contains id of `s`. We thus reconstruct and update `ref.ColumnIDs` to be `[2, 3]`. -func maybeUpdateInvalidColumnIdsInSequenceBackReferences( - ctx context.Context, txn *kv.Txn, idToUpgrade descpb.ID, descriptors *descs.Collection, -) (hasUpgraded bool, err error) { - // Get the sequence descriptor that we are going to upgrade. - seqDesc, err := descriptors.MutableByID(txn).Table(ctx, idToUpgrade) - if err != nil { - return false, err - } - if !seqDesc.IsSequence() { - return false, errors.AssertionFailedf("input id to upgrade %v is is not a sequence", idToUpgrade) - } - - for i, ref := range seqDesc.DependedOnBy { - // Re-construct the expected column IDs in `ref` and update - // `ref.ColumnIDs` if the actual value is not equal to the - // expected value. - expectedColumnIDsInRef := make([]descpb.ColumnID, 0) - tableDesc, err := descriptors.MutableByID(txn).Table(ctx, ref.ID) - if err != nil { - return false, err - } - for _, col := range tableDesc.GetColumns() { - if descpb.IDs(col.UsesSequenceIds).Contains(seqDesc.ID) { - expectedColumnIDsInRef = append(expectedColumnIDsInRef, col.ID) - } - } - sort.Slice(expectedColumnIDsInRef, func(i, j int) bool { - return expectedColumnIDsInRef[i] < expectedColumnIDsInRef[j] - }) - - if !reflect.DeepEqual(ref.ColumnIDs, expectedColumnIDsInRef) { - seqDesc.DependedOnBy[i].ColumnIDs = expectedColumnIDsInRef - hasUpgraded = true - } - } - - if hasUpgraded { - // Write the updated sequence descriptor to storage. - log.Infof(ctx, "updated invalid column IDs in back references in sequence %v (%v)", - seqDesc.Name, idToUpgrade) - if err = descriptors.WriteDesc(ctx, false, seqDesc, txn); err != nil { - return false, err - } - } - - return hasUpgraded, err -} diff --git a/pkg/upgrade/upgrades/update_invalid_column_ids_in_sequence_back_references_external_test.go b/pkg/upgrade/upgrades/update_invalid_column_ids_in_sequence_back_references_external_test.go deleted file mode 100644 index ca7f2a6494d9..000000000000 --- a/pkg/upgrade/upgrades/update_invalid_column_ids_in_sequence_back_references_external_test.go +++ /dev/null @@ -1,199 +0,0 @@ -// 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 upgrades_test - -import ( - "context" - "encoding/hex" - "fmt" - "testing" - - "github.com/cockroachdb/cockroach/pkg/base" - "github.com/cockroachdb/cockroach/pkg/clusterversion" - "github.com/cockroachdb/cockroach/pkg/server" - "github.com/cockroachdb/cockroach/pkg/settings/cluster" - "github.com/cockroachdb/cockroach/pkg/sql/catalog" - "github.com/cockroachdb/cockroach/pkg/sql/catalog/descbuilder" - "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" - "github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc" - "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" - "github.com/cockroachdb/cockroach/pkg/testutils/testcluster" - "github.com/cockroachdb/cockroach/pkg/util/hlc" - "github.com/cockroachdb/cockroach/pkg/util/leaktest" - "github.com/cockroachdb/cockroach/pkg/util/log" - "github.com/stretchr/testify/require" -) - -// TestUpgradeSeqToBeReferencedByID tests that sequence references by name will be upgraded -// to be by ID in tables or views. -func TestUpdateInvalidColumnIDsInSequenceBackReferences(t *testing.T) { - defer leaktest.AfterTest(t)() - defer log.Scope(t).Close(t) - - var ( - v0 = clusterversion.ByKey(clusterversion.TODODelete_V22_2UpdateInvalidColumnIDsInSequenceBackReferences - 1) - v1 = clusterversion.ByKey(clusterversion.TODODelete_V22_2UpdateInvalidColumnIDsInSequenceBackReferences) - ) - - type TestCase struct { - seqDescsToInject []string - tblDescsToInject []string - expectedColumnIDsInSeqBackReferencesAfterUpdate map[string][][]string - } - - /* - The hex for the descriptors to inject was created by running commands - in comment above each test case in a 21.1 binary, in which we managed to - create corrupt sequence descriptors with 0-valued column ID in its - `DependedOnBy` references. - */ - testCases := []TestCase{ - { - /* - CREATE SEQUENCE s; - CREATE TABLE tbl (i INT PRIMARY KEY); - ALTER TABLE tbl ADD COLUMN j INT DEFAULT nextval('s'); - */ - seqDescsToInject: []string{ - "0aa5020a01731834203228023a0042240a0576616c756510011a0c08011040180030005014600020003000680070007800800100480052500a077072696d61727910011800220576616c7565300140004a10080010001a00200028003000380040005a007a0408002000800100880100900100980100a20106080012001800a80100b20100ba010060006a1d0a090a0561646d696e10020a080a04726f6f7410021204726f6f741801800100880103980100b201160a077072696d61727910001a0576616c756520012801b80100c20100d201080835100018002001e2011a0801100118ffffffffffffffff7f200128003204080010003801e80100f2010408001200f801008002009202009a020a08d0b197c78df6f98517b20200b80200c0021dc80200e00200", - }, - tblDescsToInject: []string{ - "0ab4020a0374626c1835203228043a0042200a016910011a0c08011040180030005014600020003000680070007800800100423a0a016a10021a0c08011040180030005014600020012a166e65787476616c2835323a3a3a524547434c41535329300050346800700078008001004803524c0a077072696d61727910011801220169300140004a10080010001a00200028003000380040005a007a0408002000800100880100900103980100a20106080012001800a80100b20100ba010060026a1d0a090a0561646d696e10020a080a04726f6f7410021204726f6f741801800102880103980100b201170a077072696d61727910001a01691a016a200120022802b80101c20100e80100f2010408001200f801008002009202009a020a08888d94b9b0f6f98517b20200b80200c0021dc80200e00200", - }, - expectedColumnIDsInSeqBackReferencesAfterUpdate: map[string][][]string{"s": {{"[2]"}}}, - }, - { - /* - CREATE SEQUENCE s; - CREATE TABLE tbl (i INT PRIMARY KEY); - ALTER TABLE tbl ADD COLUMN j INT DEFAULT nextval('s'), ADD COLUMN k INT DEFAULT nextval('s'); - */ - seqDescsToInject: []string{ - "0aa7020a01731834203228023a0042240a0576616c756510011a0c08011040180030005014600020003000680070007800800100480052500a077072696d61727910011800220576616c7565300140004a10080010001a00200028003000380040005a007a0408002000800100880100900100980100a20106080012001800a80100b20100ba010060006a1d0a090a0561646d696e10020a080a04726f6f7410021204726f6f741801800100880103980100b201160a077072696d61727910001a0576616c756520012801b80100c20100d2010a08351000180018002001e2011a0801100118ffffffffffffffff7f200128003204080010003801e80100f2010408001200f801008002009202009a020a08e0ed9d81e6f6a08617b20200b80200c0021dc80200e00200", - }, - tblDescsToInject: []string{ - "0af5020a0374626c1835203228043a0042200a016910011a0c08011040180030005014600020003000680070007800800100423a0a016a10021a0c08011040180030005014600020012a166e65787476616c2835323a3a3a524547434c4153532930005034680070007800800100423a0a016b10031a0c08011040180030005014600020012a166e65787476616c2835323a3a3a524547434c41535329300050346800700078008001004804524c0a077072696d61727910011801220169300140004a10080010001a00200028003000380040005a007a0408002000800100880100900103980100a20106080012001800a80100b20100ba010060026a1d0a090a0561646d696e10020a080a04726f6f7410021204726f6f741801800102880103980100b2011c0a077072696d61727910001a01691a016a1a016b2001200220032802b80101c20100e80100f2010408001200f801008002009202009a020a08888e9486fef6a08617b20200b80200c0021dc80200e00200", - }, - expectedColumnIDsInSeqBackReferencesAfterUpdate: map[string][][]string{"s": {{"[2, 3]"}}}, - }, - { - /* - CREATE SEQUENCE s; - CREATE TABLE tbl_1 (i INT PRIMARY KEY); - ALTER TABLE tbl_1 ADD COLUMN j INT DEFAULT nextval('s'); - - CREATE TABLE tbl_2 (i INT PRIMARY KEY); - ALTER TABLE tbl_2 ADD COLUMN j INT DEFAULT nextval('s'), ADD COLUMN k INT DEFAULT nextval('s'); - - CREATE TABLE tbl_3 (i INT PRIMARY KEY, j INT DEFAULT nextval('s')); - ALTER TABLE tbl_3 ADD COLUMN k INT DEFAULT nextval('s'); - */ - seqDescsToInject: []string{ - "0abf020a01731834203228053a0042240a0576616c756510011a0c08011040180030005014600020003000680070007800800100480052500a077072696d61727910011800220576616c7565300140004a10080010001a00200028003000380040005a007a0408002000800100880100900100980100a20106080012001800a80100b20100ba010060006a1d0a090a0561646d696e10020a080a04726f6f7410021204726f6f741801800100880103980100b201160a077072696d61727910001a0576616c756520012801b80100c20100d201080835100018002001d2010a08361000180018002001d2010a08371000180218002001e2011a0801100118ffffffffffffffff7f200128003204080010003801e80100f2010408001200f801008002009202009a020a08b0d8cd8fe5f2a18617b20200b80200c0021dc80200e00200", - }, - tblDescsToInject: []string{ - "0ab6020a0574626c5f311835203228043a0042200a016910011a0c08011040180030005014600020003000680070007800800100423a0a016a10021a0c08011040180030005014600020012a166e65787476616c2835323a3a3a524547434c41535329300050346800700078008001004803524c0a077072696d61727910011801220169300140004a10080010001a00200028003000380040005a007a0408002000800100880100900103980100a20106080012001800a80100b20100ba010060026a1d0a090a0561646d696e10020a080a04726f6f7410021204726f6f741801800102880103980100b201170a077072696d61727910001a01691a016a200120022802b80101c20100e80100f2010408001200f801008002009202009a020a08d0c5e6ffc5f3a18617b20200b80200c0021dc80200e00200", - "0af7020a0574626c5f321836203228043a0042200a016910011a0c08011040180030005014600020003000680070007800800100423a0a016a10021a0c08011040180030005014600020012a166e65787476616c2835323a3a3a524547434c4153532930005034680070007800800100423a0a016b10031a0c08011040180030005014600020012a166e65787476616c2835323a3a3a524547434c41535329300050346800700078008001004804524c0a077072696d61727910011801220169300140004a10080010001a00200028003000380040005a007a0408002000800100880100900103980100a20106080012001800a80100b20100ba010060026a1d0a090a0561646d696e10020a080a04726f6f7410021204726f6f741801800102880103980100b2011c0a077072696d61727910001a01691a016a1a016b2001200220032802b80101c20100e80100f2010408001200f801008002009202009a020a08b8d3ffc9d4f3a18617b20200b80200c0021dc80200e00200", - "0af7020a0574626c5f331837203228043a0042200a016910011a0c08011040180030005014600020003000680070007800800100423a0a016a10021a0c08011040180030005014600020012a166e65787476616c2835323a3a3a524547434c4153532930005034680070007800800100423a0a016b10031a0c08011040180030005014600020012a166e65787476616c2835323a3a3a524547434c41535329300050346800700078008001004804524c0a077072696d61727910011801220169300140004a10080010001a00200028003000380040005a007a0408002000800100880100900103980100a20106080012001800a80100b20100ba010060026a1d0a090a0561646d696e10020a080a04726f6f7410021204726f6f741801800102880103980100b2011c0a077072696d61727910001a01691a016a1a016b2001200220032802b80101c20100e80100f2010408001200f801008002009202009a020a08c8b8f5bea1f9a18617b20200b80200c0021dc80200e00200", - }, - expectedColumnIDsInSeqBackReferencesAfterUpdate: map[string][][]string{"s": {{"[2]"}, {"[2, 3]"}, {"[2, 3]"}}}, - }, - { - /* - CREATE SEQUENCE s1; - CREATE SEQUENCE s2; - - CREATE TABLE tbl (i INT PRIMARY KEY); - ALTER TABLE tbl ADD COLUMN j INT DEFAULT nextval('s1'); - ALTER TABLE tbl ADD COLUMN k INT DEFAULT nextval('s2'); - */ - seqDescsToInject: []string{ - "0aa6020a0273311834203228023a0042240a0576616c756510011a0c08011040180030005014600020003000680070007800800100480052500a077072696d61727910011800220576616c7565300140004a10080010001a00200028003000380040005a007a0408002000800100880100900100980100a20106080012001800a80100b20100ba010060006a1d0a090a0561646d696e10020a080a04726f6f7410021204726f6f741801800100880103980100b201160a077072696d61727910001a0576616c756520012801b80100c20100d201080836100018002001e2011a0801100118ffffffffffffffff7f200128003204080010003801e80100f2010408001200f801008002009202009a020a08b8e6cacbf597f28617b20200b80200c0021dc80200e00200", - "0aa6020a0273321835203228023a0042240a0576616c756510011a0c08011040180030005014600020003000680070007800800100480052500a077072696d61727910011800220576616c7565300140004a10080010001a00200028003000380040005a007a0408002000800100880100900100980100a20106080012001800a80100b20100ba010060006a1d0a090a0561646d696e10020a080a04726f6f7410021204726f6f741801800100880103980100b201160a077072696d61727910001a0576616c756520012801b80100c20100d201080836100018002001e2011a0801100118ffffffffffffffff7f200128003204080010003801e80100f2010408001200f801008002009202009a020a08f0b2baff8498f28617b20200b80200c0021dc80200e00200", - }, - tblDescsToInject: []string{ - "0af5020a0374626c1836203228073a0042200a016910011a0c08011040180030005014600020003000680070007800800100423a0a016a10021a0c08011040180030005014600020012a166e65787476616c2835323a3a3a524547434c4153532930005034680070007800800100423a0a016b10031a0c08011040180030005014600020012a166e65787476616c2835333a3a3a524547434c41535329300050356800700078008001004804524c0a077072696d61727910011801220169300140004a10080010001a00200028003000380040005a007a0408002000800100880100900103980100a20106080012001800a80100b20100ba010060026a1d0a090a0561646d696e10020a080a04726f6f7410021204726f6f741801800103880103980100b2011c0a077072696d61727910001a01691a016a1a016b2001200220032802b80101c20100e80100f2010408001200f801008002009202009a020a08d0fc82eb819cf28617b20200b80200c0021dc80200e00200", - }, - expectedColumnIDsInSeqBackReferencesAfterUpdate: map[string][][]string{"s1": {{"[2]"}}, "s2": {{"[3]"}}}, - }, - } - - run := func(testCase TestCase) { - ctx := context.Background() - settings := cluster.MakeTestingClusterSettingsWithVersions(v1, v0, false /* initializeVersion */) - require.NoError(t, clusterversion.Initialize(ctx, v0, &settings.SV)) - - tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{ - ServerArgs: base.TestServerArgs{ - Settings: settings, - Knobs: base.TestingKnobs{ - Server: &server.TestingKnobs{ - DisableAutomaticVersionUpgrade: make(chan struct{}), - BinaryVersionOverride: v0, - }, - }, - }, - }) - defer tc.Stopper().Stop(ctx) - - sqlDB := tc.ServerConn(0) - tdb := sqlutils.MakeSQLRunner(sqlDB) - - var parentID, parentSchemaID descpb.ID - tdb.Exec(t, "CREATE TABLE temp_tbl()") - tdb.QueryRow(t, `SELECT "parentID", "parentSchemaID" FROM system.namespace WHERE name = 'temp_tbl'`). - Scan(&parentID, &parentSchemaID) - - // A function that decode a table descriptor from a hex-encoded string and insert it into the test cluster. - decodeTableDescriptorAndInsert := func(hexEncodedDescriptor string) { - decodedDescriptor, err := hex.DecodeString(hexEncodedDescriptor) - require.NoError(t, err) - b, err := descbuilder.FromBytesAndMVCCTimestamp(decodedDescriptor, hlc.Timestamp{WallTime: 1}) - require.NoError(t, err) - require.NotNil(t, b) - require.Equal(t, catalog.Table, b.DescriptorType()) - require.NoError(t, b.RunPostDeserializationChanges()) - // Modify this descriptor's parentID and parentSchemaID - tableDesc := b.(tabledesc.TableDescriptorBuilder).BuildCreatedMutableTable() - tableDesc.ParentID = parentID - tableDesc.UnexposedParentSchemaID = parentSchemaID - // Insert the descriptor into test cluster. - require.NoError(t, sqlutils.InjectDescriptors( - ctx, sqlDB, []*descpb.Descriptor{tableDesc.DescriptorProto()}, true, /* force */ - )) - } - - // Decode and insert the sequence descriptors and table descriptors. - for _, seq := range testCase.seqDescsToInject { - decodeTableDescriptorAndInsert(seq) - } - for _, tbl := range testCase.tblDescsToInject { - decodeTableDescriptorAndInsert(tbl) - } - - // Upgrade to the new cluster version. - tdb.Exec(t, `SET CLUSTER SETTING version = $1`, v1.String()) - tdb.CheckQueryResultsRetry(t, "SHOW CLUSTER SETTING version", - [][]string{{v1.String()}}) - - // Assert the upgrade logic correctly updated the invalid column IDs in the sequences' - // back references. - for seqName, expectedOutput := range testCase.expectedColumnIDsInSeqBackReferencesAfterUpdate { - query := fmt.Sprintf("select jsonb_array_elements(crdb_internal.pb_to_json('cockroach.sql.sqlbase.Descriptor', descriptor)->'table'->'dependedOnBy')->'columnIds' from system.descriptor where id = '%v'::REGCLASS;", seqName) - tdb.CheckQueryResults(t, query, expectedOutput) - } - } - - for _, testCase := range testCases { - run(testCase) - } - -} diff --git a/pkg/upgrade/upgrades/upgrade_sequence_to_be_referenced_by_ID.go b/pkg/upgrade/upgrades/upgrade_sequence_to_be_referenced_by_ID.go deleted file mode 100644 index 2976f7309cf1..000000000000 --- a/pkg/upgrade/upgrades/upgrade_sequence_to_be_referenced_by_ID.go +++ /dev/null @@ -1,325 +0,0 @@ -// 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 upgrades - -import ( - "context" - - "github.com/cockroachdb/cockroach/pkg/clusterversion" - "github.com/cockroachdb/cockroach/pkg/kv" - "github.com/cockroachdb/cockroach/pkg/sql" - "github.com/cockroachdb/cockroach/pkg/sql/catalog" - "github.com/cockroachdb/cockroach/pkg/sql/catalog/descbuilder" - "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" - "github.com/cockroachdb/cockroach/pkg/sql/catalog/descs" - "github.com/cockroachdb/cockroach/pkg/sql/catalog/resolver" - "github.com/cockroachdb/cockroach/pkg/sql/catalog/seqexpr" - "github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc" - "github.com/cockroachdb/cockroach/pkg/sql/isql" - "github.com/cockroachdb/cockroach/pkg/sql/parser" - "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" - "github.com/cockroachdb/cockroach/pkg/upgrade" - "github.com/cockroachdb/cockroach/pkg/util/hlc" - "github.com/cockroachdb/errors" -) - -func upgradeSequenceToBeReferencedByID( - ctx context.Context, _ clusterversion.ClusterVersion, d upgrade.TenantDeps, -) error { - var lastUpgradedID descpb.ID - // Upgrade each table/view, one at a time, until we exhaust all of them. - for { - done, idToUpgrade, err := findNextTableToUpgrade( - ctx, d.DB.Executor(), nil, /* kvTxn */ - lastUpgradedID, func(table *descpb.TableDescriptor) bool { - return table.IsTable() || table.IsView() - }) - if err != nil || done { - return err - } - - // Table/View `idToUpgrade` might contain reference to sequences by name. If so, we need to upgrade - // those references to be by ID. - err = maybeUpgradeSeqReferencesInTableOrView(ctx, idToUpgrade, d) - if err != nil { - return err - } - lastUpgradedID = idToUpgrade - } -} - -// Find the next table descriptor ID that is > `lastUpgradedID` -// and satisfy the `tableSelector`. -// If no such ID exists, `done` will be true. -func findNextTableToUpgrade( - ctx context.Context, - ex isql.Executor, - kvTxn *kv.Txn, - lastUpgradedID descpb.ID, - tableSelector func(table *descpb.TableDescriptor) bool, -) (done bool, idToUpgrade descpb.ID, err error) { - var rows isql.Rows - rows, err = ex.QueryIterator(ctx, "upgrade-seq-find-desc", kvTxn, - `SELECT id, descriptor, crdb_internal_mvcc_timestamp FROM system.descriptor WHERE id > $1 ORDER BY ID ASC`, lastUpgradedID) - if err != nil { - return false, 0, err - } - defer func() { _ = rows.Close() }() - - var ok bool - for ok, err = rows.Next(ctx); ok; ok, err = rows.Next(ctx) { - row := rows.Cur() - id := descpb.ID(tree.MustBeDInt(row[0])) - ts, err := hlc.DecimalToHLC(&row[2].(*tree.DDecimal).Decimal) - if err != nil { - return false, 0, errors.Wrapf(err, - "failed to convert MVCC timestamp decimal to HLC for ID %d", id) - } - b, err := descbuilder.FromBytesAndMVCCTimestamp([]byte(tree.MustBeDBytes(row[1])), ts) - if err != nil { - return false, 0, errors.Wrapf(err, - "failed to unmarshal descriptor with ID %d", id) - } - // Return this descriptor if it's a non-dropped table or view. - if b != nil && b.DescriptorType() == catalog.Table { - tableDesc := b.BuildImmutable().(catalog.TableDescriptor) - if tableDesc != nil && !tableDesc.Dropped() && tableSelector(tableDesc.TableDesc()) { - return false, id, nil - } - } - } - - // Break out of the above loop either because we exhausted rows or an error occurred. - if err != nil { - return false, 0, err - } - return true, 0, nil -} - -// maybeUpgradeSeqReferencesInTableOrView updates descriptor `idToUpgrade` if it references any sequences by name. -func maybeUpgradeSeqReferencesInTableOrView( - ctx context.Context, idToUpgrade descpb.ID, d upgrade.TenantDeps, -) error { - return d.DB.DescsTxn(ctx, func( - ctx context.Context, txn descs.Txn, - ) error { - // Set up: retrieve table desc for `idToUpgrade` and a schema resolver - kvTxn, descriptors := txn.KV(), txn.Descriptors() - tableDesc, sc, cleanup, err := upgradeSetUpForTableOrView(ctx, d, kvTxn, descriptors, idToUpgrade) - if err != nil { - return err - } - defer cleanup() - - // Act: upgrade the table's (or view's) sequence references accordingly. - if tableDesc.IsTable() { - if err = upgradeSequenceReferenceInTable(ctx, kvTxn, tableDesc, sc, descriptors); err != nil { - return err - } - } else if tableDesc.IsView() { - if err = upgradeSequenceReferenceInView(ctx, kvTxn, tableDesc, sc, descriptors); err != nil { - return err - } - } else { - return errors.Errorf("Expect table or view desc to upgrade sequence references; "+ - "Got %v", tableDesc.String()) - } - return nil - }) -} - -// upgradeSetUpForTableOrView upgrades the given table or view (with id `idToUpgrade`) to reference sequence from -// by name to by ID, if any. -func upgradeSetUpForTableOrView( - ctx context.Context, - d upgrade.TenantDeps, - txn *kv.Txn, - descriptors *descs.Collection, - idToUpgrade descpb.ID, -) (*tabledesc.Mutable, resolver.SchemaResolver, func(), error) { - // Get the table descriptor that we are going to upgrade. - tableDesc, err := descriptors.MutableByID(txn).Table(ctx, idToUpgrade) - if err != nil { - return nil, nil, nil, err - } - - // Get the database of the table to pass to the planner constructor. - dbDesc, err := descriptors.ByIDWithLeased(txn).WithoutNonPublic().Get().Database(ctx, tableDesc.GetParentID()) - if err != nil { - return nil, nil, nil, err - } - - // Construct an internal planner - sc, cleanup, err := d.SchemaResolverConstructor(txn, descriptors, dbDesc.GetName()) - if err != nil { - return nil, nil, nil, err - } - - return tableDesc, sc, cleanup, nil -} - -// upgradeSequenceReferenceInTable upgrade sequence reference from by name to by ID in table `tableDesc`. -func upgradeSequenceReferenceInTable( - ctx context.Context, - txn *kv.Txn, - tableDesc *tabledesc.Mutable, - sc resolver.SchemaResolver, - descriptors *descs.Collection, -) error { - // 'changedSeqDescs' stores all sequences that are referenced by name from 'tableDesc'. They will soon - // be upgraded to be referenced by ID, so we store them here so that we can later schedule schema changes to - // them, after their backreference's ByID are set to true. - var changedSeqDescs []*tabledesc.Mutable - - // a function that will modify `expr` in-place if it contains any sequence reference by name (to by ID). - maybeUpgradeSeqReferencesInExpr := func(expr *string) error { - parsedExpr, err := parser.ParseExpr(*expr) - if err != nil { - return err - } - seqIdentifiers, err := seqexpr.GetUsedSequences(parsedExpr) - if err != nil { - return err - } - if len(seqIdentifiers) == 0 { - return nil - } - - seqNameToID, err := maybeUpdateBackRefsAndBuildMap(ctx, sc, tableDesc, seqIdentifiers, &changedSeqDescs) - if err != nil { - return err - } - - // Perform the sequence replacement in the default expression. - newExpr, err := seqexpr.ReplaceSequenceNamesWithIDs(parsedExpr, seqNameToID) - if err != nil { - return err - } - - // Modify the input `expr` in-place. - *expr = tree.Serialize(newExpr) - return nil - } - - // Check each column's DEFAULT and ON UPDATE expression and update sequence references if any. - for _, column := range tableDesc.Columns { - if column.HasDefault() { - if err := maybeUpgradeSeqReferencesInExpr(column.DefaultExpr); err != nil { - return err - } - } - if column.HasOnUpdate() { - if err := maybeUpgradeSeqReferencesInExpr(column.OnUpdateExpr); err != nil { - return err - } - } - } - - // Write the schema change for all referenced sequence descriptors. - for _, changedSeqDesc := range changedSeqDescs { - if err := descriptors.WriteDesc(ctx, false, changedSeqDesc, txn); err != nil { - return err - } - } - - // Write the schema change for the table. - return descriptors.WriteDesc(ctx, false /* kvTrace */, tableDesc, txn) -} - -// upgradeSequenceReferenceInView similarly upgrade sequence reference from by name to by ID in view `viewDesc`. -func upgradeSequenceReferenceInView( - ctx context.Context, - txn *kv.Txn, - viewDesc *tabledesc.Mutable, - sc resolver.SchemaResolver, - descriptors *descs.Collection, -) error { - var changedSeqDescs []*tabledesc.Mutable - replaceSeqFunc := func(expr tree.Expr) (recurse bool, newExpr tree.Expr, err error) { - seqIdentifiers, err := seqexpr.GetUsedSequences(expr) - if err != nil { - return false, expr, err - } - seqNameToID, err := maybeUpdateBackRefsAndBuildMap(ctx, sc, viewDesc, seqIdentifiers, &changedSeqDescs) - if err != nil { - return false, expr, err - } - - newExpr, err = seqexpr.ReplaceSequenceNamesWithIDs(expr, seqNameToID) - if err != nil { - return false, expr, err - } - return false, newExpr, nil - } - - stmt, err := parser.ParseOne(viewDesc.ViewQuery) - if err != nil { - return err - } - newStmt, err := tree.SimpleStmtVisit(stmt.AST, replaceSeqFunc) - if err != nil { - return err - } - viewDesc.ViewQuery = newStmt.String() - - // Write the schema change for all referenced sequence descriptors. - for _, changedSeqDesc := range changedSeqDescs { - if err := descriptors.WriteDesc(ctx, false, changedSeqDesc, txn); err != nil { - return err - } - } - - // Write the schema change for updated view descriptor. - return descriptors.WriteDesc(ctx, false /* kvTrace */, viewDesc, txn) -} - -// maybeUpdateBackRefsAndBuildMap iterates over all the sequence identifiers -// and the table that contains them, checks if they're referenced by ID. -// If not, it will update the back reference to reflect that it will now be -// stored by ID. It also builds a mapping of sequence names mapped to their IDs, -// and accumulates a list of changed descriptors. -func maybeUpdateBackRefsAndBuildMap( - ctx context.Context, - sc resolver.SchemaResolver, - t *tabledesc.Mutable, - seqIdentifiers []seqexpr.SeqIdentifier, - changedSeqDescs *[]*tabledesc.Mutable, -) (map[string]descpb.ID, error) { - seqNameToID := make(map[string]descpb.ID) - for _, seqIdentifier := range seqIdentifiers { - seqDesc, err := sql.GetSequenceDescFromIdentifier(ctx, sc, seqIdentifier) - if err != nil { - return nil, err - } - - // Get all the indexes of all references to the sequence by this table. - var refIdxs []int - for i, reference := range seqDesc.DependedOnBy { - if reference.ID == t.ID { - refIdxs = append(refIdxs, i) - } - } - - // Check if we're already referencing the sequence by ID. If so, skip. - // If not, update the back reference to reflect that it's now by ID. - for _, refIdx := range refIdxs { - if seqDesc.DependedOnBy[refIdx].ByID { - continue - } else { - seqDesc.DependedOnBy[refIdx].ByID = true - *changedSeqDescs = append(*changedSeqDescs, seqDesc) - } - } - seqNameToID[seqDesc.GetName()] = seqDesc.ID - } - - return seqNameToID, nil -} diff --git a/pkg/upgrade/upgrades/upgrade_sequence_to_be_referenced_by_ID_external_test.go b/pkg/upgrade/upgrades/upgrade_sequence_to_be_referenced_by_ID_external_test.go deleted file mode 100644 index 4610fe2af2db..000000000000 --- a/pkg/upgrade/upgrades/upgrade_sequence_to_be_referenced_by_ID_external_test.go +++ /dev/null @@ -1,153 +0,0 @@ -// 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 upgrades_test - -import ( - "context" - "encoding/hex" - "strings" - "testing" - - "github.com/cockroachdb/cockroach/pkg/base" - "github.com/cockroachdb/cockroach/pkg/clusterversion" - "github.com/cockroachdb/cockroach/pkg/server" - "github.com/cockroachdb/cockroach/pkg/settings/cluster" - "github.com/cockroachdb/cockroach/pkg/sql/catalog" - "github.com/cockroachdb/cockroach/pkg/sql/catalog/descbuilder" - "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" - "github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc" - "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" - "github.com/cockroachdb/cockroach/pkg/testutils/testcluster" - "github.com/cockroachdb/cockroach/pkg/util/hlc" - "github.com/cockroachdb/cockroach/pkg/util/leaktest" - "github.com/cockroachdb/cockroach/pkg/util/log" - "github.com/stretchr/testify/require" -) - -// TestUpgradeSeqToBeReferencedByID tests that sequence references by name will be upgraded -// to be by ID in tables or views. -func TestUpgradeSeqToBeReferencedByID(t *testing.T) { - defer leaktest.AfterTest(t)() - defer log.Scope(t).Close(t) - - var ( - v0 = clusterversion.ByKey(clusterversion.TODODelete_V22_2UpgradeSequenceToBeReferencedByID - 1) - v1 = clusterversion.ByKey(clusterversion.TODODelete_V22_2UpgradeSequenceToBeReferencedByID) - ) - - ctx := context.Background() - settings := cluster.MakeTestingClusterSettingsWithVersions(v1, v0, false /* initializeVersion */) - require.NoError(t, clusterversion.Initialize(ctx, v0, &settings.SV)) - - tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{ - ServerArgs: base.TestServerArgs{ - Settings: settings, - Knobs: base.TestingKnobs{ - Server: &server.TestingKnobs{ - DisableAutomaticVersionUpgrade: make(chan struct{}), - BinaryVersionOverride: v0, - }, - }, - }, - }) - defer tc.Stopper().Stop(ctx) - - sqlDB := tc.ServerConn(0) - tdb := sqlutils.MakeSQLRunner(sqlDB) - - /* - The hex for the descriptor to inject was created by running the following - commands in a 20.2 binary, in which sequences are referenced by name in - tables and views. - - CREATE SEQUENCE s; - CREATE TABLE tbl (i INT PRIMARY KEY, j INT NOT NULL DEFAULT nextval('s')); - CREATE VIEW v AS (SELECT nextval('s')); - - SELECT encode(descriptor, 'hex') - FROM system.descriptor - WHERE id = ( - SELECT id - FROM system.namespace - WHERE name = 's' - ); - - SELECT encode(descriptor, 'hex') - FROM system.descriptor - WHERE id = ( - SELECT id - FROM system.namespace - WHERE name = 'tbl' - ); - - SELECT encode(descriptor, 'hex') - FROM system.descriptor - WHERE id = ( - SELECT id - FROM system.namespace - WHERE name = 'v' - ); - */ - - var parentID, parentSchemaID descpb.ID - tdb.Exec(t, "CREATE TABLE temp_tbl()") - tdb.QueryRow(t, `SELECT "parentID", "parentSchemaID" FROM system.namespace WHERE name = 'temp_tbl'`). - Scan(&parentID, &parentSchemaID) - var table, createTable string - const sequenceDescriptorToInject = "0aa0020a01731834203228033a0042210a0576616c756510011a0c080110401800300050146000200030006800700078004800524e0a077072696d61727910011800220576616c7565300140004a10080010001a00200028003000380040005a007a020800800100880100900100980100a20106080012001800a80100b20100ba010060006a1d0a090a0561646d696e10020a080a04726f6f7410021204726f6f741801800100880103980100b201160a077072696d61727910001a0576616c756520012801b80100c20100d20106083510001802d2010408361000e201180801100118ffffffffffffffff7f20012800320408001000e80100f2010408001200f801008002009202009a020a08c0f0f4deb8b4f4f816b20200b80200c0021dc80200" - const tableDescriptorToInject = "0a9e020a0374626c1835203228013a00421d0a016910011a0c0801104018003000501460002000300068007000780042360a016a10021a0c08011040180030005014600020002a156e65787476616c282773273a3a3a535452494e4729300050346800700078004803524a0a077072696d61727910011801220169300140004a10080010001a00200028003000380040005a007a020800800100880100900101980100a20106080012001800a80100b20100ba010060026a1d0a090a0561646d696e10020a080a04726f6f7410021204726f6f741801800101880103980100b201170a077072696d61727910001a01691a016a200120022802b80101c20100e80100f2010408001200f801008002009202009a0200b20200b80200c0021dc80200" - const viewDescriptorToInject = "0ae3010a01761836203228013a0042230a076e65787476616c10011a0c080110401800300050146000200130006800700078004802523c0a00100018004a10080010001a00200028003000380040005a007a020800800100880100900100980100a20106080012001800a80100b20100ba010060006a1d0a090a0561646d696e10020a080a04726f6f7410021204726f6f741801800101880103980100b80100c2011e2853454c454354206e65787476616c282773273a3a3a535452494e472929c80134e80100f2010408001200f801008002009202009a0200b20200b80200c0021dc80200" - - // A function that decode a table descriptor from a hex-encoded string and insert it into the test cluster. - decodeTableDescriptorAndInsert := func(hexEncodedDescriptor string) { - decodedDescriptor, err := hex.DecodeString(hexEncodedDescriptor) - require.NoError(t, err) - b, err := descbuilder.FromBytesAndMVCCTimestamp(decodedDescriptor, hlc.Timestamp{WallTime: 1}) - require.NoError(t, err) - require.NotNil(t, b) - require.Equal(t, catalog.Table, b.DescriptorType()) - // Run post deserialization changes. - require.NoError(t, b.RunPostDeserializationChanges()) - // Modify this descriptor's parentID and parentSchemaID - tableDesc := b.(tabledesc.TableDescriptorBuilder).BuildCreatedMutableTable() - tableDesc.ParentID = parentID - tableDesc.UnexposedParentSchemaID = parentSchemaID - // Insert the descriptor into test cluster. - require.NoError(t, sqlutils.InjectDescriptors( - ctx, sqlDB, []*descpb.Descriptor{tableDesc.DescriptorProto()}, true, /* force */ - )) - } - - // Decode and insert the sequence descriptor. - decodeTableDescriptorAndInsert(sequenceDescriptorToInject) - - // Decode and insert the table descriptor, and assert that the sequence is referenced by name. - decodeTableDescriptorAndInsert(tableDescriptorToInject) - tdb.QueryRow(t, `SHOW CREATE tbl`).Scan(&table, &createTable) - require.True(t, strings.Contains(createTable, "j INT8 NOT NULL DEFAULT nextval('s':::STRING)")) - - // Decode and insert the view descriptor, and assert that the view is referenced by name. - decodeTableDescriptorAndInsert(viewDescriptorToInject) - tdb.QueryRow(t, `SHOW CREATE v`).Scan(&table, &createTable) - require.True(t, strings.Contains(createTable, "SELECT nextval('s':::STRING)")) - - // Upgrade to the new cluster version. - tdb.Exec(t, `SET CLUSTER SETTING version = $1`, v1.String()) - tdb.CheckQueryResultsRetry(t, "SHOW CLUSTER SETTING version", - [][]string{{v1.String()}}) - - // Assert the upgrade logic correctly changed the sequence reference from by name to by ID in - // both the table and view descriptor. - tdb.QueryRow(t, `SHOW CREATE tbl`).Scan(&table, &createTable) - require.True(t, strings.Contains(createTable, "j INT8 NOT NULL DEFAULT nextval('public.s'::REGCLASS)")) - tdb.QueryRow(t, `SHOW CREATE v`).Scan(&table, &createTable) - require.True(t, strings.Contains(createTable, "SELECT nextval('public.s'::REGCLASS)")) -} diff --git a/pkg/upgrade/upgrades/upgrades.go b/pkg/upgrade/upgrades/upgrades.go index ffed3988abf4..e52760725278 100644 --- a/pkg/upgrade/upgrades/upgrades.go +++ b/pkg/upgrade/upgrades/upgrades.go @@ -99,12 +99,6 @@ var upgrades = []upgradebase.Upgrade{ preconditionBeforeStartingAnUpgrade, NoTenantUpgradeFunc, ), - upgrade.NewTenantUpgrade( - "upgrade sequences to be referenced by ID", - toCV(clusterversion.TODODelete_V22_2UpgradeSequenceToBeReferencedByID), - upgrade.NoPrecondition, - upgradeSequenceToBeReferencedByID, - ), upgrade.NewTenantUpgrade( "update system.statement_diagnostics_requests to support sampling probabilities", toCV(clusterversion.TODODelete_V22_2SampledStmtDiagReqs), @@ -134,11 +128,6 @@ var upgrades = []upgradebase.Upgrade{ upgrade.NoPrecondition, waitForAllSchemaChanges, ), - upgrade.NewTenantUpgrade("update invalid column IDs in sequence back references", - toCV(clusterversion.TODODelete_V22_2UpdateInvalidColumnIDsInSequenceBackReferences), - upgrade.NoPrecondition, - updateInvalidColumnIDsInSequenceBackReferences, - ), upgrade.NewTenantUpgrade("fix corrupt user-file related table descriptors", toCV(clusterversion.TODODelete_V22_2FixUserfileRelatedDescriptorCorruption), upgrade.NoPrecondition, From ba6d1738abda387ffb5bb76212f6ae501879cec4 Mon Sep 17 00:00:00 2001 From: Nathan VanBenschoten Date: Mon, 13 Mar 2023 13:09:42 -0400 Subject: [PATCH 3/3] kv: deflake TestAbortCountConflictingWrites Fixes #96839. The test was made flaky by 5129578. See the comment in https://github.com/cockroachdb/cockroach/issues/96839#issuecomment-1466557458 for an explanation. This commit resolves that flakiness. Release note: None --- pkg/testutils/storageutils/mocking.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/testutils/storageutils/mocking.go b/pkg/testutils/storageutils/mocking.go index 52ab1012b147..4de75fb0b3e4 100644 --- a/pkg/testutils/storageutils/mocking.go +++ b/pkg/testutils/storageutils/mocking.go @@ -96,5 +96,8 @@ func (c *ReplayProtectionFilterWrapper) run(args kvserverbase.FilterArgs) *kvpb. c.Unlock() res := future.WaitForResult(args.Ctx) + if res.Err != nil { + return kvpb.NewError(res.Err) + } return shallowCloneErrorWithTxn(res.Val.(*kvpb.Error)) }