From ea432032f531a8fc13aa04b1acb68563a5bb6cc7 Mon Sep 17 00:00:00 2001 From: Andrew Werner Date: Sat, 7 Sep 2019 18:41:53 -0400 Subject: [PATCH 1/3] sql,sqlbase,client: bootstrap TableDescriptor timestamps from MVCC Using the MVCC timestamp of the value for table descriptors has long been theorized as the right mechanism to eliminate the need for transactions which update a table descriptor version to observe their commit timestamp (see https://github.com/cockroachdb/cockroach/issues/17698#issuecomment-452035873). The challenge was presumed to be the need to expose MVCC timestamps in our client library. It turns out we seem to do that already (how did nobody know that?!). This commit avoids using the CommitTimestamp by using the MVCC timestamp on the read path. In order to make this setting of the timestamp less of a footgun we add a `(*Descriptor).Table(hlc.Timestamp)` method which forces anybody who extracts a `TableDescriptor` from a `Descriptor` to pass in a timestamp which may be used to set `ModificationTime` and `CreateAsOfTime`. A linter in the following commit enforces this proper usage. The below SQL would always fail before this change and now passes: ``` CREATE TABLE foo ( k INT PRIMARY KEY ); BEGIN; DROP TABLE foo; COMMIT; ``` Similarly the TestImportData seems to pass under stressrace with a 5s `kv.closed_timestamp.target_duration`. Release justification: fixes a release blocker and known customer issue. References #37083. Release note: None --- docs/generated/settings/settings.html | 2 +- pkg/ccl/backupccl/backup.go | 26 ++-- pkg/ccl/backupccl/restore.go | 8 +- pkg/ccl/backupccl/show.go | 3 +- pkg/ccl/backupccl/targets.go | 5 +- pkg/ccl/backupccl/targets_test.go | 5 + pkg/ccl/changefeedccl/changefeed_stmt.go | 2 +- pkg/ccl/changefeedccl/table_history.go | 5 +- pkg/ccl/cliccl/load.go | 3 +- pkg/ccl/storageccl/bench_test.go | 3 +- pkg/ccl/storageccl/key_rewriter.go | 3 +- pkg/ccl/storageccl/key_rewriter_test.go | 8 +- pkg/ccl/utilccl/sampledataccl/bankdata.go | 2 +- pkg/internal/client/db.go | 30 ++++- pkg/internal/client/txn.go | 29 ++-- pkg/server/updates.go | 2 +- pkg/settings/cluster/cockroach_versions.go | 13 ++ pkg/settings/cluster/versionkey_string.go | 5 +- pkg/sql/conn_executor_exec.go | 62 +++++---- pkg/sql/create_sequence.go | 2 +- pkg/sql/create_table.go | 3 +- pkg/sql/create_view.go | 2 +- pkg/sql/descriptor.go | 8 +- pkg/sql/drop_test.go | 15 ++- pkg/sql/lease.go | 12 +- pkg/sql/lease_test.go | 7 +- .../testdata/logic_test/crdb_internal | 9 +- .../testdata/logic_test/schema_change_retry | 6 + .../opt/exec/execbuilder/testdata/show_trace | 12 +- pkg/sql/plan.go | 20 +++ pkg/sql/rename_test.go | 12 +- pkg/sql/resolver.go | 3 +- pkg/sql/schema_changer_test.go | 9 +- pkg/sql/sqlbase/structured.go | 124 ++++++++++++++---- pkg/sql/sqlbase/structured.pb.go | 89 +++++++------ pkg/sql/sqlbase/structured.proto | 14 ++ pkg/sql/sqlbase/structured_test.go | 111 +++++++++++----- pkg/sql/sqlbase/system.go | 2 +- pkg/sql/sqlbase/table.go | 1 + pkg/sql/sqlbase/testutils.go | 5 +- pkg/sql/table.go | 33 ++++- pkg/sql/zone_config.go | 4 +- pkg/storage/reports/reporter.go | 2 +- pkg/storage/reports/reporter_test.go | 30 +++-- 44 files changed, 521 insertions(+), 230 deletions(-) diff --git a/docs/generated/settings/settings.html b/docs/generated/settings/settings.html index 147fcc57443e..51f2a5ddfb59 100644 --- a/docs/generated/settings/settings.html +++ b/docs/generated/settings/settings.html @@ -132,6 +132,6 @@ trace.debug.enablebooleanfalseif set, traces for recent requests can be seen in the /debug page trace.lightstep.tokenstringif set, traces go to Lightstep using this token trace.zipkin.collectorstringif set, traces go to the given Zipkin instance (example: '127.0.0.1:9411'); ignored if trace.lightstep.token is set -versioncustom validation19.1-9set the active cluster version in the format '.' +versioncustom validation19.1-10set the active cluster version in the format '.' diff --git a/pkg/ccl/backupccl/backup.go b/pkg/ccl/backupccl/backup.go index 519cb53a73dc..d2fb4dfdfb23 100644 --- a/pkg/ccl/backupccl/backup.go +++ b/pkg/ccl/backupccl/backup.go @@ -178,7 +178,7 @@ func getRelevantDescChanges( // obviously interesting to our backup. for _, i := range descs { interestingIDs[i.GetID()] = struct{}{} - if t := i.GetTable(); t != nil { + if t := i.Table(hlc.Timestamp{}); t != nil { for j := t.ReplacementOf.ID; j != sqlbase.InvalidID; j = priorIDs[j] { interestingIDs[j] = struct{}{} } @@ -200,7 +200,7 @@ func getRelevantDescChanges( return nil, err } for _, i := range starting { - if table := i.GetTable(); table != nil { + if table := i.Table(hlc.Timestamp{}); table != nil { // We need to add to interestingIDs so that if we later see a delete for // this ID we still know it is interesting to us, even though we will not // have a parentID at that point (since the delete is a nil desc). @@ -231,7 +231,7 @@ func getRelevantDescChanges( if _, ok := interestingIDs[change.ID]; ok { interestingChanges = append(interestingChanges, change) } else if change.Desc != nil { - if table := change.Desc.GetTable(); table != nil { + if table := change.Desc.Table(hlc.Timestamp{}); table != nil { if _, ok := interestingParents[table.ParentID]; ok { interestingIDs[table.ID] = struct{}{} interestingChanges = append(interestingChanges, change) @@ -279,7 +279,8 @@ func getAllDescChanges( return nil, err } r.Desc = &desc - if t := desc.GetTable(); t != nil && t.ReplacementOf.ID != sqlbase.InvalidID { + t := desc.Table(rev.Timestamp) + if t != nil && t.ReplacementOf.ID != sqlbase.InvalidID { priorIDs[t.ID] = t.ReplacementOf.ID } } @@ -303,6 +304,9 @@ func allSQLDescriptors(ctx context.Context, txn *client.Txn) ([]sqlbase.Descript return nil, errors.NewAssertionErrorWithWrappedErrf(err, "%s: unable to unmarshal SQL descriptor", row.Key) } + if row.Value != nil { + sqlDescs[i].Table(row.Value.Timestamp) + } } return sqlDescs, nil } @@ -379,7 +383,7 @@ func spansForAllTableIndexes( // in them that we didn't already get above e.g. indexes or tables that are // not in latest because they were dropped during the time window in question. for _, rev := range revs { - if tbl := rev.Desc.GetTable(); tbl != nil { + if tbl := rev.Desc.Table(hlc.Timestamp{}); tbl != nil { for _, idx := range tbl.AllNonDropIndexes() { key := tableAndIndex{tableID: tbl.ID, indexID: idx.ID} if !added[key] { @@ -1016,7 +1020,7 @@ func backupPlanHook( return err } } - if tableDesc := desc.GetTable(); tableDesc != nil { + if tableDesc := desc.Table(hlc.Timestamp{}); tableDesc != nil { if err := p.CheckPrivilege(ctx, tableDesc, privilege.SELECT); err != nil { return err } @@ -1083,7 +1087,7 @@ func backupPlanHook( tablesInPrev := make(map[sqlbase.ID]struct{}) dbsInPrev := make(map[sqlbase.ID]struct{}) for _, d := range prevBackups[len(prevBackups)-1].Descriptors { - if t := d.GetTable(); t != nil { + if t := d.Table(hlc.Timestamp{}); t != nil { tablesInPrev[t.ID] = struct{}{} } } @@ -1092,7 +1096,7 @@ func backupPlanHook( } for _, d := range targetDescs { - if t := d.GetTable(); t != nil { + if t := d.Table(hlc.Timestamp{}); t != nil { // If we're trying to use a previous backup for this table, ideally it // actually contains this table. if _, ok := tablesInPrev[t.ID]; ok { @@ -1504,7 +1508,7 @@ func maybeDowngradeTableDescsInBackupDescriptor( // Copy Descriptors so we can return a shallow copy without mutating the slice. copy(backupDescCopy.Descriptors, backupDesc.Descriptors) for i := range backupDesc.Descriptors { - if tableDesc := backupDesc.Descriptors[i].GetTable(); tableDesc != nil { + if tableDesc := backupDesc.Descriptors[i].Table(hlc.Timestamp{}); tableDesc != nil { downgraded, newDesc, err := tableDesc.MaybeDowngradeForeignKeyRepresentation(ctx, settings) if err != nil { return nil, err @@ -1534,7 +1538,7 @@ func maybeUpgradeTableDescsInBackupDescriptors( // descriptors so that they can be looked up. for _, backupDesc := range backupDescs { for _, desc := range backupDesc.Descriptors { - if table := desc.GetTable(); table != nil { + if table := desc.Table(hlc.Timestamp{}); table != nil { protoGetter.Protos[string(sqlbase.MakeDescMetadataKey(table.ID))] = sqlbase.WrapDescriptor(protoutil.Clone(table).(*sqlbase.TableDescriptor)) } @@ -1544,7 +1548,7 @@ func maybeUpgradeTableDescsInBackupDescriptors( for i := range backupDescs { backupDesc := &backupDescs[i] for j := range backupDesc.Descriptors { - if table := backupDesc.Descriptors[j].GetTable(); table != nil { + if table := backupDesc.Descriptors[j].Table(hlc.Timestamp{}); table != nil { if _, err := table.MaybeUpgradeForeignKeyRepresentation(ctx, protoGetter, skipFKsWithNoMatchingTable); err != nil { return err } diff --git a/pkg/ccl/backupccl/restore.go b/pkg/ccl/backupccl/restore.go index bdfdf83c4ab7..d8eb402af50a 100644 --- a/pkg/ccl/backupccl/restore.go +++ b/pkg/ccl/backupccl/restore.go @@ -184,7 +184,7 @@ func loadSQLDescsFromBackupsAtTime( allDescs := make([]sqlbase.Descriptor, 0, len(byID)) for _, desc := range byID { - if t := desc.GetTable(); t != nil { + if t := desc.Table(hlc.Timestamp{}); t != nil { // A table revisions may have been captured before it was in a DB that is // backed up -- if the DB is missing, filter the table. if byID[t.ParentID] == nil { @@ -212,7 +212,7 @@ func selectTargets( seenTable := false for _, desc := range matched.descs { - if desc.GetTable() != nil { + if desc.Table(hlc.Timestamp{}) != nil { seenTable = true break } @@ -1534,7 +1534,7 @@ func doRestorePlan( for _, desc := range sqlDescs { if dbDesc := desc.GetDatabase(); dbDesc != nil { databasesByID[dbDesc.ID] = dbDesc - } else if tableDesc := desc.GetTable(); tableDesc != nil { + } else if tableDesc := desc.Table(hlc.Timestamp{}); tableDesc != nil { tablesByID[tableDesc.ID] = tableDesc } } @@ -1686,7 +1686,7 @@ func createImportingTables( var tables []*sqlbase.TableDescriptor var oldTableIDs []sqlbase.ID for _, desc := range sqlDescs { - if tableDesc := desc.GetTable(); tableDesc != nil { + if tableDesc := desc.Table(hlc.Timestamp{}); tableDesc != nil { tables = append(tables, tableDesc) oldTableIDs = append(oldTableIDs, tableDesc.ID) } diff --git a/pkg/ccl/backupccl/show.go b/pkg/ccl/backupccl/show.go index 771f680df6e0..a91cffd256d4 100644 --- a/pkg/ccl/backupccl/show.go +++ b/pkg/ccl/backupccl/show.go @@ -19,6 +19,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/sqlbase" "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/cockroach/pkg/util/encoding" + "github.com/cockroachdb/cockroach/pkg/util/hlc" "github.com/cockroachdb/cockroach/pkg/util/timeutil" "github.com/cockroachdb/cockroach/pkg/util/tracing" ) @@ -145,7 +146,7 @@ func backupShowerDefault(ctx context.Context, showSchemas bool) backupShower { var rows []tree.Datums var row tree.Datums for _, descriptor := range desc.Descriptors { - if table := descriptor.GetTable(); table != nil { + if table := descriptor.Table(hlc.Timestamp{}); table != nil { dbName := descs[table.ParentID] row = tree.Datums{ tree.NewDString(dbName), diff --git a/pkg/ccl/backupccl/targets.go b/pkg/ccl/backupccl/targets.go index 4ad651a12289..dfae8d2321ce 100644 --- a/pkg/ccl/backupccl/targets.go +++ b/pkg/ccl/backupccl/targets.go @@ -14,6 +14,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sessiondata" "github.com/cockroachdb/cockroach/pkg/sql/sqlbase" + "github.com/cockroachdb/cockroach/pkg/util/hlc" "github.com/pkg/errors" ) @@ -121,7 +122,7 @@ func newDescriptorResolver(descs []sqlbase.Descriptor) (*descriptorResolver, err } // Now on to the tables. for _, desc := range descs { - if tbDesc := desc.GetTable(); tbDesc != nil { + if tbDesc := desc.Table(hlc.Timestamp{}); tbDesc != nil { if tbDesc.Dropped() { continue } @@ -214,7 +215,7 @@ func descriptorsMatchingTargets( desc := descI.(sqlbase.Descriptor) // If the parent database is not requested already, request it now - parentID := desc.GetTable().GetParentID() + parentID := desc.Table(hlc.Timestamp{}).GetParentID() if _, ok := alreadyRequestedDBs[parentID]; !ok { parentDesc := resolver.descByID[parentID] ret.descs = append(ret.descs, parentDesc) diff --git a/pkg/ccl/backupccl/targets_test.go b/pkg/ccl/backupccl/targets_test.go index 7a69d832f0d5..c86676699fcf 100644 --- a/pkg/ccl/backupccl/targets_test.go +++ b/pkg/ccl/backupccl/targets_test.go @@ -20,6 +20,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/sessiondata" "github.com/cockroachdb/cockroach/pkg/sql/sqlbase" "github.com/cockroachdb/cockroach/pkg/testutils" + "github.com/cockroachdb/cockroach/pkg/util/hlc" "github.com/cockroachdb/cockroach/pkg/util/leaktest" ) @@ -34,6 +35,10 @@ func TestDescriptorsMatchingTargets(t *testing.T) { *sqlbase.WrapDescriptor(&sqlbase.DatabaseDescriptor{ID: 3, Name: "data"}), *sqlbase.WrapDescriptor(&sqlbase.DatabaseDescriptor{ID: 5, Name: "empty"}), } + // Set the timestamp on the table descriptors. + for _, d := range descriptors { + d.Table(hlc.Timestamp{WallTime: 1}) + } tests := []struct { sessionDatabase string diff --git a/pkg/ccl/changefeedccl/changefeed_stmt.go b/pkg/ccl/changefeedccl/changefeed_stmt.go index 7f2e8b0debef..af6736fde112 100644 --- a/pkg/ccl/changefeedccl/changefeed_stmt.go +++ b/pkg/ccl/changefeedccl/changefeed_stmt.go @@ -200,7 +200,7 @@ func changefeedPlanHook( } targets := make(jobspb.ChangefeedTargets, len(targetDescs)) for _, desc := range targetDescs { - if tableDesc := desc.GetTable(); tableDesc != nil { + if tableDesc := desc.Table(hlc.Timestamp{}); tableDesc != nil { targets[tableDesc.ID] = jobspb.ChangefeedTarget{ StatementTimeName: tableDesc.Name, } diff --git a/pkg/ccl/changefeedccl/table_history.go b/pkg/ccl/changefeedccl/table_history.go index 13f048a9f188..342dd140fd5b 100644 --- a/pkg/ccl/changefeedccl/table_history.go +++ b/pkg/ccl/changefeedccl/table_history.go @@ -260,7 +260,8 @@ func fetchTableDescriptorVersions( } else if !ok { return nil } - remaining, _, _, err := sqlbase.DecodeTableIDIndexID(it.UnsafeKey().Key) + k := it.UnsafeKey() + remaining, _, _, err := sqlbase.DecodeTableIDIndexID(k.Key) if err != nil { return err } @@ -282,7 +283,7 @@ func fetchTableDescriptorVersions( if err := value.GetProto(&desc); err != nil { return err } - if tableDesc := desc.GetTable(); tableDesc != nil { + if tableDesc := desc.Table(k.Timestamp); tableDesc != nil { tableDescs = append(tableDescs, tableDesc) } } diff --git a/pkg/ccl/cliccl/load.go b/pkg/ccl/cliccl/load.go index d7a44afc5bb0..bf1d98fd15ee 100644 --- a/pkg/ccl/cliccl/load.go +++ b/pkg/ccl/cliccl/load.go @@ -18,6 +18,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/ccl/storageccl" "github.com/cockroachdb/cockroach/pkg/cli" "github.com/cockroachdb/cockroach/pkg/settings/cluster" + "github.com/cockroachdb/cockroach/pkg/util/hlc" "github.com/cockroachdb/cockroach/pkg/util/humanizeutil" "github.com/cockroachdb/cockroach/pkg/util/timeutil" "github.com/pkg/errors" @@ -96,7 +97,7 @@ func runLoadShow(cmd *cobra.Command, args []string) error { // in case more fields need to be added to the output. fmt.Printf("Descriptors:\n") for _, d := range desc.Descriptors { - if desc := d.GetTable(); desc != nil { + if desc := d.Table(hlc.Timestamp{}); desc != nil { fmt.Printf(" %d: %s (table)\n", d.GetID(), d.GetName()) } if desc := d.GetDatabase(); desc != nil { diff --git a/pkg/ccl/storageccl/bench_test.go b/pkg/ccl/storageccl/bench_test.go index 0d0bdf28455e..85e465c50db6 100644 --- a/pkg/ccl/storageccl/bench_test.go +++ b/pkg/ccl/storageccl/bench_test.go @@ -25,6 +25,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/storage/engine" "github.com/cockroachdb/cockroach/pkg/testutils" "github.com/cockroachdb/cockroach/pkg/testutils/testcluster" + "github.com/cockroachdb/cockroach/pkg/util/hlc" "github.com/cockroachdb/cockroach/pkg/util/protoutil" "github.com/cockroachdb/cockroach/pkg/workload/bank" ) @@ -173,7 +174,7 @@ func BenchmarkImport(b *testing.B) { { // TODO(dan): The following should probably make it into // dataccl.Backup somehow. - tableDesc := backup.Desc.Descriptors[len(backup.Desc.Descriptors)-1].GetTable() + tableDesc := backup.Desc.Descriptors[len(backup.Desc.Descriptors)-1].Table(hlc.Timestamp{}) if tableDesc == nil || tableDesc.ParentID == keys.SystemDatabaseID { b.Fatalf("bad table descriptor: %+v", tableDesc) } diff --git a/pkg/ccl/storageccl/key_rewriter.go b/pkg/ccl/storageccl/key_rewriter.go index 9f7bd947d658..8df90829ab74 100644 --- a/pkg/ccl/storageccl/key_rewriter.go +++ b/pkg/ccl/storageccl/key_rewriter.go @@ -14,6 +14,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/sql/sqlbase" "github.com/cockroachdb/cockroach/pkg/util/encoding" + "github.com/cockroachdb/cockroach/pkg/util/hlc" "github.com/cockroachdb/cockroach/pkg/util/protoutil" "github.com/pkg/errors" ) @@ -63,7 +64,7 @@ func MakeKeyRewriterFromRekeys(rekeys []roachpb.ImportRequest_TableRekey) (*KeyR if err := protoutil.Unmarshal(rekey.NewDesc, &desc); err != nil { return nil, errors.Wrapf(err, "unmarshalling rekey descriptor for old table id %d", rekey.OldID) } - table := desc.GetTable() + table := desc.Table(hlc.Timestamp{}) if table == nil { return nil, errors.New("expected a table descriptor") } diff --git a/pkg/ccl/storageccl/key_rewriter_test.go b/pkg/ccl/storageccl/key_rewriter_test.go index 9c45346dc219..b7385dc62f36 100644 --- a/pkg/ccl/storageccl/key_rewriter_test.go +++ b/pkg/ccl/storageccl/key_rewriter_test.go @@ -15,6 +15,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/sql/sqlbase" "github.com/cockroachdb/cockroach/pkg/util/encoding" + "github.com/cockroachdb/cockroach/pkg/util/hlc" "github.com/cockroachdb/cockroach/pkg/util/leaktest" "github.com/cockroachdb/cockroach/pkg/util/protoutil" ) @@ -140,8 +141,11 @@ func TestKeyRewriter(t *testing.T) { }) } -func mustMarshalDesc(t *testing.T, desc *sqlbase.TableDescriptor) []byte { - bytes, err := protoutil.Marshal(sqlbase.WrapDescriptor(desc)) +func mustMarshalDesc(t *testing.T, tableDesc *sqlbase.TableDescriptor) []byte { + desc := sqlbase.WrapDescriptor(tableDesc) + // Set the timestamp to a non-zero value. + desc.Table(hlc.Timestamp{WallTime: 1}) + bytes, err := protoutil.Marshal(desc) if err != nil { t.Fatal(err) } diff --git a/pkg/ccl/utilccl/sampledataccl/bankdata.go b/pkg/ccl/utilccl/sampledataccl/bankdata.go index c8b20247bf93..acd5e8af621d 100644 --- a/pkg/ccl/utilccl/sampledataccl/bankdata.go +++ b/pkg/ccl/utilccl/sampledataccl/bankdata.go @@ -96,7 +96,7 @@ func (b *Backup) NextKeyValues( ) ([]engine.MVCCKeyValue, roachpb.Span, error) { var userTables []*sqlbase.TableDescriptor for _, d := range b.Desc.Descriptors { - if t := d.GetTable(); t != nil && t.ParentID != keys.SystemDatabaseID { + if t := d.Table(hlc.Timestamp{}); t != nil && t.ParentID != keys.SystemDatabaseID { userTables = append(userTables, t) } } diff --git a/pkg/internal/client/db.go b/pkg/internal/client/db.go index 304058859a4c..94e6aaa00432 100644 --- a/pkg/internal/client/db.go +++ b/pkg/internal/client/db.go @@ -28,10 +28,15 @@ import ( ) // KeyValue represents a single key/value pair. This is similar to -// roachpb.KeyValue except that the value may be nil. +// roachpb.KeyValue except that the value may be nil. The timestamp +// in the value will be populated with the MVCC timestamp at which this +// value was read if this struct was produced by a GetRequest or +// ScanRequest which uses the KEY_VALUES ScanFormat. Values created from +// a ScanRequest which uses the BATCH_RESPONSE ScanFormat will contain a +// zero Timestamp. type KeyValue struct { Key roachpb.Key - Value *roachpb.Value // Timestamp will always be zero + Value *roachpb.Value } func (kv *KeyValue) String() string { @@ -319,11 +324,28 @@ func (db *DB) Get(ctx context.Context, key interface{}) (KeyValue, error) { // // key can be either a byte slice or a string. func (db *DB) GetProto(ctx context.Context, key interface{}, msg protoutil.Message) error { + _, err := db.GetProtoTs(ctx, key, msg) + return err +} + +// GetProtoTs retrieves the value for a key and decodes the result as a proto +// message. It additionally returns the timestamp at which the key was read. +// If the key doesn't exist, the proto will simply be reset and a zero timestamp +// will be returned. A zero timestamp will also be returned if unmarshaling +// fails. +// +// key can be either a byte slice or a string. +func (db *DB) GetProtoTs( + ctx context.Context, key interface{}, msg protoutil.Message, +) (hlc.Timestamp, error) { r, err := db.Get(ctx, key) if err != nil { - return err + return hlc.Timestamp{}, err + } + if err := r.ValueProto(msg); err != nil || r.Value == nil { + return hlc.Timestamp{}, err } - return r.ValueProto(msg) + return r.Value.Timestamp, nil } // Put sets the value for a key. diff --git a/pkg/internal/client/txn.go b/pkg/internal/client/txn.go index 7c35b3cf91b4..6892fa67556a 100644 --- a/pkg/internal/client/txn.go +++ b/pkg/internal/client/txn.go @@ -263,14 +263,6 @@ func (txn *Txn) CommitTimestamp() hlc.Timestamp { return txn.mu.sender.CommitTimestamp() } -// CommitTimestampFixed returns true if the commit timestamp has -// been fixed to the start timestamp and cannot be pushed forward. -func (txn *Txn) CommitTimestampFixed() bool { - txn.mu.Lock() - defer txn.mu.Unlock() - return txn.mu.sender.CommitTimestampFixed() -} - // SetSystemConfigTrigger sets the system db trigger to true on this transaction. // This will impact the EndTransactionRequest. func (txn *Txn) SetSystemConfigTrigger() error { @@ -319,11 +311,28 @@ func (txn *Txn) Get(ctx context.Context, key interface{}) (KeyValue, error) { // // key can be either a byte slice or a string. func (txn *Txn) GetProto(ctx context.Context, key interface{}, msg protoutil.Message) error { + _, err := txn.GetProtoTs(ctx, key, msg) + return err +} + +// GetProtoTs retrieves the value for a key and decodes the result as a proto +// message. It additionally returns the timestamp at which the key was read. +// If the key doesn't exist, the proto will simply be reset and a zero timestamp +// will be returned. A zero timestamp will also be returned if unmarshaling +// fails. +// +// key can be either a byte slice or a string. +func (txn *Txn) GetProtoTs( + ctx context.Context, key interface{}, msg protoutil.Message, +) (hlc.Timestamp, error) { r, err := txn.Get(ctx, key) if err != nil { - return err + return hlc.Timestamp{}, err + } + if err := r.ValueProto(msg); err != nil || r.Value == nil { + return hlc.Timestamp{}, err } - return r.ValueProto(msg) + return r.Value.Timestamp, nil } // Put sets the value for a key diff --git a/pkg/server/updates.go b/pkg/server/updates.go index 8f45f8034bc4..df3f12262d43 100644 --- a/pkg/server/updates.go +++ b/pkg/server/updates.go @@ -497,7 +497,7 @@ func (s *Server) collectSchemaInfo(ctx context.Context) ([]sqlbase.TableDescript if err := kv.ValueProto(&desc); err != nil { return nil, errors.Wrapf(err, "%s: unable to unmarshal SQL descriptor", kv.Key) } - if t := desc.GetTable(); t != nil && t.ID > keys.MaxReservedDescID { + if t := desc.Table(kv.Value.Timestamp); t != nil && t.ID > keys.MaxReservedDescID { if err := reflectwalk.Walk(t, redactor); err != nil { panic(err) // stringRedactor never returns a non-nil err } diff --git a/pkg/settings/cluster/cockroach_versions.go b/pkg/settings/cluster/cockroach_versions.go index 7284c3fa1685..ba578fec8fa0 100644 --- a/pkg/settings/cluster/cockroach_versions.go +++ b/pkg/settings/cluster/cockroach_versions.go @@ -44,6 +44,7 @@ const ( VersionTopLevelForeignKeys VersionAtomicChangeReplicasTrigger VersionAtomicChangeReplicas + VersionTableDescModificationTimeFromMVCC // Add new versions here (step one of two). @@ -532,6 +533,18 @@ var versionsSingleton = keyedVersions([]keyedVersion{ Key: VersionAtomicChangeReplicas, Version: roachpb.Version{Major: 19, Minor: 1, Unstable: 9}, }, + { + // VersionTableDescModificationTimeFromMVCC is https://github.com/cockroachdb/cockroach/pull/40581 + // + // It represents an upgrade to the table descriptor format in which + // CreateAsOfTime and ModifiedTime are set to zero when new versions of + // table descriptors are written. This removes the need to fix the commit + // timestamp for transactions which update table descriptors. The value + // is then populated by the reading client with the MVCC timestamp of the + // row which contained the serialized table descriptor. + Key: VersionTableDescModificationTimeFromMVCC, + Version: roachpb.Version{Major: 19, Minor: 1, Unstable: 10}, + }, // Add new versions here (step two of two). diff --git a/pkg/settings/cluster/versionkey_string.go b/pkg/settings/cluster/versionkey_string.go index 03adc824bd13..110801e12816 100644 --- a/pkg/settings/cluster/versionkey_string.go +++ b/pkg/settings/cluster/versionkey_string.go @@ -21,11 +21,12 @@ func _() { _ = x[VersionTopLevelForeignKeys-10] _ = x[VersionAtomicChangeReplicasTrigger-11] _ = x[VersionAtomicChangeReplicas-12] + _ = x[VersionTableDescModificationTimeFromMVCC-13] } -const _VersionKey_name = "Version2_1VersionUnreplicatedRaftTruncatedStateVersionSideloadedStorageNoReplicaIDVersion19_1VersionStart19_2VersionQueryTxnTimestampVersionStickyBitVersionParallelCommitsVersionGenerationComparableVersionLearnerReplicasVersionTopLevelForeignKeysVersionAtomicChangeReplicasTriggerVersionAtomicChangeReplicas" +const _VersionKey_name = "Version2_1VersionUnreplicatedRaftTruncatedStateVersionSideloadedStorageNoReplicaIDVersion19_1VersionStart19_2VersionQueryTxnTimestampVersionStickyBitVersionParallelCommitsVersionGenerationComparableVersionLearnerReplicasVersionTopLevelForeignKeysVersionAtomicChangeReplicasTriggerVersionAtomicChangeReplicasVersionTableDescModificationTimeFromMVCC" -var _VersionKey_index = [...]uint16{0, 10, 47, 82, 93, 109, 133, 149, 171, 198, 220, 246, 280, 307} +var _VersionKey_index = [...]uint16{0, 10, 47, 82, 93, 109, 133, 149, 171, 198, 220, 246, 280, 307, 347} func (i VersionKey) String() string { if i < 0 || i >= VersionKey(len(_VersionKey_index)-1) { diff --git a/pkg/sql/conn_executor_exec.go b/pkg/sql/conn_executor_exec.go index dbc301b65925..b8327ff0c1d2 100644 --- a/pkg/sql/conn_executor_exec.go +++ b/pkg/sql/conn_executor_exec.go @@ -468,6 +468,10 @@ func (ex *connExecutor) execStmtInOpenState( // leases will only go down over time: no new conflicting leases can be // created as of the time of this call because v-2 can't be leased once // v-1 exists. +// +// If this method succeeds it is the caller's responsibility to release the +// executor's table leases after the txn commits so that schema changes can +// proceed. func (ex *connExecutor) checkTableTwoVersionInvariant(ctx context.Context) error { tables := ex.extraTxnState.tables.getTablesWithNewVersion() if tables == nil { @@ -477,40 +481,36 @@ func (ex *connExecutor) checkTableTwoVersionInvariant(ctx context.Context) error if txn.IsCommitted() { panic("transaction has already committed") } - if !txn.CommitTimestampFixed() { - panic("commit timestamp was not fixed") - } - // Release leases here for two reasons: - // 1. If there are existing leases at version V-2 for a descriptor - // being modified to version V being held the wait loop below that - // waits on a cluster wide release of old version leases will hang - // until these leases expire. - // 2. Once this transaction commits, the schema changers run and - // increment the version of the modified descriptors. If one of the - // descriptors being modified has a lease being held the schema - // changers will stall until the leases expire. - // - // The above two cases can be satified by releasing leases for both - // cases explicitly, but we prefer to call it here and kill two birds - // with one stone. + // We potentially hold leases for tables which we've modified which + // we need to drop. Say we're updating tables at version V. All leases + // for version V-2 need to be dropped immediately, otherwise the check + // below that nobody holds leases for version V-2 will fail. Worse yet, + // the code below loops waiting for nobody to hold leases on V-2. We also + // may hold leases for version V-1 of modified tables that are good to drop + // but not as vital for correctness. It's good to drop them because as soon + // as this transaction commits jobs may start and will need to wait until + // the lease expires. It is safe because V-1 must remain valid until this + // transaction commits; if we commit then nobody else could have written + // a new V beneath us because we've already laid down an intent. // - // It is safe to release leases even though the transaction hasn't yet - // committed only because the transaction timestamp has been fixed using - // CommitTimestamp(). - // - // releaseLeases can fail to release a lease if the server is shutting - // down. This is okay because it will result in the two cases mentioned - // above simply hanging until the expiration time for the leases. - ex.extraTxnState.tables.releaseLeases(ctx) - - count, err := CountLeases(ctx, ex.server.cfg.InternalExecutor, tables, txn.OrigTimestamp()) + // All this being said, we must retain our leases on tables which we have + // not modified to ensure that our writes to those other tables in this + // transaction remain valid. + ex.extraTxnState.tables.releaseTableLeases(ctx, tables) + + // We know that so long as there are no leases on the updated tables as of + // the current provisional commit timestamp for this transaction then if this + // transaction ends up committing then there won't have been any created + // in the meantime. + count, err := CountLeases(ctx, ex.server.cfg.InternalExecutor, tables, txn.Serialize().Timestamp) if err != nil { return err } if count == 0 { return nil } + // Restart the transaction so that it is able to replay itself at a newer timestamp // with the hope that the next time around there will be leases only at the current // version. @@ -533,6 +533,9 @@ func (ex *connExecutor) checkTableTwoVersionInvariant(ctx context.Context) error // We cleanup the transaction and create a new transaction wait time // might be extensive and so we'd better get rid of all the intents. txn.CleanupOnError(ctx, retryErr) + // Release the rest of our leases on unmodified tables so we don't hold up + // schema changes there and potentially create a deadlock. + ex.extraTxnState.tables.releaseLeases(ctx) // Wait until all older version leases have been released or expired. for r := retry.StartWithCtx(ctx, base.DefaultRetryOptions()); r.Next(); { @@ -580,6 +583,13 @@ func (ex *connExecutor) commitSQLTransaction( return ex.makeErrEvent(err, stmt) } + // Now that we've committed, if we modified any table we need to make sure + // to release the leases for them so that the schema change can proceed and + // we don't block the client. + if tables := ex.extraTxnState.tables.getTablesWithNewVersion(); tables != nil { + ex.extraTxnState.tables.releaseLeases(ctx) + } + if !isRelease { return eventTxnFinish{}, eventTxnFinishPayload{commit: true} } diff --git a/pkg/sql/create_sequence.go b/pkg/sql/create_sequence.go index a6e11884eccb..833528d944b6 100644 --- a/pkg/sql/create_sequence.go +++ b/pkg/sql/create_sequence.go @@ -77,7 +77,7 @@ func doCreateSequence( privs := dbDesc.GetPrivileges() desc, err := MakeSequenceTableDesc(name.Table(), opts, - dbDesc.ID, id, params.p.txn.CommitTimestamp(), privs, params.EvalContext().Settings) + dbDesc.ID, id, params.creationTimeForNewTableDescriptor(), privs, params.EvalContext().Settings) if err != nil { return err } diff --git a/pkg/sql/create_table.go b/pkg/sql/create_table.go index 8a396c5f6508..f3b79f4a0795 100644 --- a/pkg/sql/create_table.go +++ b/pkg/sql/create_table.go @@ -164,7 +164,7 @@ func (n *createTableNode) startExec(params runParams) error { var asCols sqlbase.ResultColumns var desc sqlbase.MutableTableDescriptor var affected map[sqlbase.ID]*sqlbase.MutableTableDescriptor - creationTime := params.p.txn.CommitTimestamp() + creationTime := params.creationTimeForNewTableDescriptor() if n.n.As() { asCols = planColumns(n.sourcePlan) if !n.run.fromHeuristicPlanner && !n.n.AsHasUserSpecifiedPrimaryKey() { @@ -1078,7 +1078,6 @@ func MakeTableDesc( evalCtx *tree.EvalContext, ) (sqlbase.MutableTableDescriptor, error) { desc := InitTableDescriptor(id, parentID, n.Table.Table(), creationTime, privileges) - for _, def := range n.Defs { if d, ok := def.(*tree.ColumnTableDef); ok { if !desc.IsVirtualTable() { diff --git a/pkg/sql/create_view.go b/pkg/sql/create_view.go index 5a7ce35d8f6e..48451b29f44e 100644 --- a/pkg/sql/create_view.go +++ b/pkg/sql/create_view.go @@ -135,7 +135,7 @@ func (n *createViewNode) startExec(params runParams) error { n.dbDesc.ID, id, n.columns, - params.p.txn.CommitTimestamp(), + params.creationTimeForNewTableDescriptor(), privs, ¶ms.p.semaCtx, ) diff --git a/pkg/sql/descriptor.go b/pkg/sql/descriptor.go index 6c887614acba..dde054269400 100644 --- a/pkg/sql/descriptor.go +++ b/pkg/sql/descriptor.go @@ -165,13 +165,13 @@ func getDescriptorByID( log.Eventf(ctx, "fetching descriptor with ID %d", id) descKey := sqlbase.MakeDescMetadataKey(id) desc := &sqlbase.Descriptor{} - if err := txn.GetProto(ctx, descKey, desc); err != nil { + ts, err := txn.GetProtoTs(ctx, descKey, desc) + if err != nil { return err } - switch t := descriptor.(type) { case *sqlbase.TableDescriptor: - table := desc.GetTable() + table := desc.Table(ts) if table == nil { return pgerror.Newf(pgcode.WrongObjectType, "%q is not a table", desc.String()) @@ -216,7 +216,7 @@ func GetAllDescriptors(ctx context.Context, txn *client.Txn) ([]sqlbase.Descript } switch t := desc.Union.(type) { case *sqlbase.Descriptor_Table: - table := desc.GetTable() + table := desc.Table(kv.Value.Timestamp) if err := table.MaybeFillInDescriptor(ctx, txn); err != nil { return nil, err } diff --git a/pkg/sql/drop_test.go b/pkg/sql/drop_test.go index 3c8a0a9c2718..24e6d36d2671 100644 --- a/pkg/sql/drop_test.go +++ b/pkg/sql/drop_test.go @@ -157,10 +157,11 @@ INSERT INTO t.kv VALUES ('c', 'e'), ('a', 'c'), ('b', 'd'); t.Fatalf(`table "kv" does not exist`) } tbDescKey := sqlbase.MakeDescMetadataKey(sqlbase.ID(gr.ValueInt())) - if err := kvDB.GetProto(ctx, tbDescKey, desc); err != nil { + ts, err := kvDB.GetProtoTs(ctx, tbDescKey, desc) + if err != nil { t.Fatal(err) } - tbDesc := desc.GetTable() + tbDesc := desc.Table(ts) // Add a zone config for both the table and database. cfg := config.DefaultZoneConfig() @@ -330,10 +331,11 @@ INSERT INTO t.kv2 VALUES ('c', 'd'), ('a', 'b'), ('e', 'a'); t.Fatalf(`table "kv" does not exist`) } tbDescKey := sqlbase.MakeDescMetadataKey(sqlbase.ID(gr.ValueInt())) - if err := kvDB.GetProto(ctx, tbDescKey, desc); err != nil { + ts, err := kvDB.GetProtoTs(ctx, tbDescKey, desc) + if err != nil { t.Fatal(err) } - tbDesc := desc.GetTable() + tbDesc := desc.Table(ts) tb2NameKey := sqlbase.MakeNameMetadataKey(dbDesc.ID, "kv2") gr2, err := kvDB.Get(ctx, tb2NameKey) @@ -344,10 +346,11 @@ INSERT INTO t.kv2 VALUES ('c', 'd'), ('a', 'b'), ('e', 'a'); t.Fatalf(`table "kv2" does not exist`) } tb2DescKey := sqlbase.MakeDescMetadataKey(sqlbase.ID(gr2.ValueInt())) - if err := kvDB.GetProto(ctx, tb2DescKey, desc); err != nil { + ts, err = kvDB.GetProtoTs(ctx, tb2DescKey, desc) + if err != nil { t.Fatal(err) } - tb2Desc := desc.GetTable() + tb2Desc := desc.Table(ts) tableSpan := tbDesc.TableSpan() table2Span := tb2Desc.TableSpan() diff --git a/pkg/sql/lease.go b/pkg/sql/lease.go index 4ff3c47e52d9..8e0f8aa8cfa5 100644 --- a/pkg/sql/lease.go +++ b/pkg/sql/lease.go @@ -306,10 +306,11 @@ func (s LeaseStore) WaitForOneVersion( // Get the current version of the table descriptor non-transactionally. // // TODO(pmattis): Do an inconsistent read here? - if err := s.db.GetProto(ctx, descKey, desc); err != nil { + ts, err := s.db.GetProtoTs(ctx, descKey, desc) + if err != nil { return 0, err } - tableDesc = desc.GetTable() + tableDesc = desc.Table(ts) if tableDesc == nil { return 0, errors.Errorf("ID %d is not a table", tableID) } @@ -404,7 +405,7 @@ func (s LeaseStore) PublishMultiple( descsToUpdate[id].Version, versions[id]) } - if err := descsToUpdate[id].MaybeIncrementVersion(ctx, txn); err != nil { + if err := descsToUpdate[id].MaybeIncrementVersion(ctx, txn, s.settings); err != nil { return err } if err := descsToUpdate[id].ValidateTable(); err != nil { @@ -553,10 +554,11 @@ func (s LeaseStore) getForExpiration( prevTimestamp := expiration.Prev() txn.SetFixedTimestamp(ctx, prevTimestamp) var desc sqlbase.Descriptor - if err := txn.GetProto(ctx, descKey, &desc); err != nil { + ts, err := txn.GetProtoTs(ctx, descKey, &desc) + if err != nil { return err } - tableDesc := desc.GetTable() + tableDesc := desc.Table(ts) if tableDesc == nil { return sqlbase.ErrDescriptorNotFound } diff --git a/pkg/sql/lease_test.go b/pkg/sql/lease_test.go index 84dbf6dd8711..31e9fb4fd89a 100644 --- a/pkg/sql/lease_test.go +++ b/pkg/sql/lease_test.go @@ -575,7 +575,7 @@ func isDeleted(tableID sqlbase.ID, cfg *config.SystemConfig) bool { if err := val.GetProto(&descriptor); err != nil { panic("unable to unmarshal table descriptor") } - table := descriptor.GetTable() + table := descriptor.Table(val.Timestamp) return table.Dropped() } @@ -1646,11 +1646,12 @@ CREATE TABLE t.test0 (k CHAR PRIMARY KEY, v CHAR); // Look up the descriptor. descKey := sqlbase.MakeDescMetadataKey(descID) dbDesc := &sqlbase.Descriptor{} - if err := txn.GetProto(ctx, descKey, dbDesc); err != nil { + ts, err := txn.GetProtoTs(ctx, descKey, dbDesc) + if err != nil { t.Fatalf("error while reading proto: %v", err) } // Look at the descriptor that comes back from the database. - dbTable := dbDesc.GetTable() + dbTable := dbDesc.Table(ts) if dbTable.Version != table.Version || dbTable.ModificationTime != table.ModificationTime { t.Fatalf("db has version %d at ts %s, expected version %d at ts %s", diff --git a/pkg/sql/logictest/testdata/logic_test/crdb_internal b/pkg/sql/logictest/testdata/logic_test/crdb_internal index a2d80b452460..74a429fa9d25 100644 --- a/pkg/sql/logictest/testdata/logic_test/crdb_internal +++ b/pkg/sql/logictest/testdata/logic_test/crdb_internal @@ -78,11 +78,12 @@ SELECT * FROM crdb_internal.schema_changes ---- table_id parent_id name type target_id target_name state direction -query IITTITRTTTTTTT colnames -SELECT * FROM crdb_internal.tables WHERE NAME = 'namespace' +# We don't select the modification time as it does not remain contant. +query IITTITTTTTTT colnames +SELECT table_id, parent_id, name, database_name, version, format_version, state, sc_lease_node_id, sc_lease_expiration_time, drop_time, audit_mode, schema_name FROM crdb_internal.tables WHERE NAME = 'namespace' ---- -table_id parent_id name database_name version mod_time mod_time_logical format_version state sc_lease_node_id sc_lease_expiration_time drop_time audit_mode schema_name -2 1 namespace system 1 1970-01-01 00:00:00 +0000 +0000 0E-10 InterleavedFormatVersion PUBLIC NULL NULL NULL DISABLED public +table_id parent_id name database_name version format_version state sc_lease_node_id sc_lease_expiration_time drop_time audit_mode schema_name +2 1 namespace system 1 InterleavedFormatVersion PUBLIC NULL NULL NULL DISABLED public # Verify that table names are not double escaped. diff --git a/pkg/sql/logictest/testdata/logic_test/schema_change_retry b/pkg/sql/logictest/testdata/logic_test/schema_change_retry index 4d0c12e8a3be..04acf7ab130f 100644 --- a/pkg/sql/logictest/testdata/logic_test/schema_change_retry +++ b/pkg/sql/logictest/testdata/logic_test/schema_change_retry @@ -3,6 +3,12 @@ # Schema changes that experienced retriable errors in RELEASE # SAVEPOINT would previously deadlock. +# Prevent transaction refreshes so we encounter the necessary +# TransactionRetryError below. + +statement ok +SET CLUSTER SETTING kv.transaction.max_refresh_spans_bytes = 0; + statement ok BEGIN diff --git a/pkg/sql/opt/exec/execbuilder/testdata/show_trace b/pkg/sql/opt/exec/execbuilder/testdata/show_trace index 868790669b6e..e4d960ad8f2f 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/show_trace +++ b/pkg/sql/opt/exec/execbuilder/testdata/show_trace @@ -38,7 +38,7 @@ WHERE message NOT LIKE '%Z/%' AND operation != 'dist sender send' ---- flow CPut /Table/2/1/53/"kv"/3/1 -> 54 -flow CPut /Table/3/1/54/2/1 -> table: columns: > nullable:false hidden:false > columns: > nullable:true hidden:false > next_column_id:3 families: next_family_id:1 primary_index: interleave:<> partitioning: type:FORWARD > next_index_id:2 privileges: users: > next_mutation_id:1 format_version:3 state:PUBLIC offline_reason:"" view_query:"" drop_time:0 replacement_of: > audit_mode:DISABLED drop_job_id:0 create_query:"" create_as_of_time: > +flow CPut /Table/3/1/54/2/1 -> table: columns: > nullable:false hidden:false > columns: > nullable:true hidden:false > next_column_id:3 families: next_family_id:1 primary_index: interleave:<> partitioning: type:FORWARD > next_index_id:2 privileges: users: > next_mutation_id:1 format_version:3 state:PUBLIC offline_reason:"" view_query:"" drop_time:0 replacement_of: > audit_mode:DISABLED drop_job_id:0 create_query:"" create_as_of_time:<> > exec stmt rows affected: 0 # We avoid using the full trace output, because that would make the @@ -64,7 +64,7 @@ WHERE message NOT LIKE '%Z/%' AND message NOT LIKE 'querying next range at%' AND tag NOT LIKE '%IndexBackfiller%' AND operation != 'dist sender send' ---- -flow Put /Table/3/1/54/2/1 -> table: columns: > nullable:false hidden:false > columns: > nullable:true hidden:false > next_column_id:3 families: next_family_id:1 primary_index: interleave:<> partitioning: type:FORWARD > next_index_id:3 privileges: users: > mutations: interleave:<> partitioning: type:FORWARD > state:DELETE_ONLY direction:ADD mutation_id:1 rollback:false > next_mutation_id:2 format_version:3 state:PUBLIC offline_reason:"" view_query:"" mutationJobs:<...> drop_time:0 replacement_of: > audit_mode:DISABLED drop_job_id:0 create_query:"" create_as_of_time: > +flow Put /Table/3/1/54/2/1 -> table: columns: > nullable:false hidden:false > columns: > nullable:true hidden:false > next_column_id:3 families: next_family_id:1 primary_index: interleave:<> partitioning: type:FORWARD > next_index_id:3 privileges: users: > mutations: interleave:<> partitioning: type:FORWARD > state:DELETE_ONLY direction:ADD mutation_id:1 rollback:false > next_mutation_id:2 format_version:3 state:PUBLIC offline_reason:"" view_query:"" mutationJobs:<...> drop_time:0 replacement_of: > audit_mode:DISABLED drop_job_id:0 create_query:"" create_as_of_time: > exec stmt rows affected: 0 statement ok @@ -123,7 +123,7 @@ WHERE message NOT LIKE '%Z/%' ---- table reader Scan /Table/54/{1-2} flow CPut /Table/2/1/53/"kv2"/3/1 -> 55 -flow CPut /Table/3/1/55/2/1 -> table: columns: > nullable:true hidden:false > columns: > nullable:true hidden:false > columns: > nullable:false default_expr:"unique_rowid()" hidden:true > next_column_id:4 families: next_family_id:1 primary_index: interleave:<> partitioning: type:FORWARD > next_index_id:2 privileges: users: > next_mutation_id:1 format_version:3 state:ADD offline_reason:"" view_query:"" drop_time:0 replacement_of: > audit_mode:DISABLED drop_job_id:0 create_query:"TABLE t.public.kv" create_as_of_time: > +flow CPut /Table/3/1/55/2/1 -> table: columns: > nullable:true hidden:false > columns: > nullable:true hidden:false > columns: > nullable:false default_expr:"unique_rowid()" hidden:true > next_column_id:4 families: next_family_id:1 primary_index: interleave:<> partitioning: type:FORWARD > next_index_id:2 privileges: users: > next_mutation_id:1 format_version:3 state:ADD offline_reason:"" view_query:"" drop_time:0 replacement_of: > audit_mode:DISABLED drop_job_id:0 create_query:"TABLE t.public.kv" create_as_of_time:<> > exec stmt rows affected: 0 statement ok @@ -172,7 +172,7 @@ WHERE message NOT LIKE '%Z/%' AND message NOT LIKE 'querying next range at%' AND tag NOT LIKE '%IndexBackfiller%' AND operation != 'dist sender send' ---- -flow Put /Table/3/1/55/2/1 -> table: columns: > nullable:true hidden:false > columns: > nullable:true hidden:false > columns: > nullable:false default_expr:"unique_rowid()" hidden:true > next_column_id:4 families: next_family_id:1 primary_index: interleave:<> partitioning: type:FORWARD > next_index_id:2 privileges: users: > next_mutation_id:1 format_version:3 state:DROP offline_reason:"" draining_names: view_query:"" drop_time:... replacement_of: > audit_mode:DISABLED drop_job_id:... create_query:"TABLE t.public.kv" create_as_of_time: > +flow Put /Table/3/1/55/2/1 -> table: columns: > nullable:true hidden:false > columns: > nullable:true hidden:false > columns: > nullable:false default_expr:"unique_rowid()" hidden:true > next_column_id:4 families: next_family_id:1 primary_index: interleave:<> partitioning: type:FORWARD > next_index_id:2 privileges: users: > next_mutation_id:1 format_version:3 state:DROP offline_reason:"" draining_names: view_query:"" drop_time:... replacement_of: > audit_mode:DISABLED drop_job_id:... create_query:"TABLE t.public.kv" create_as_of_time: > exec stmt rows affected: 0 statement ok @@ -207,7 +207,7 @@ WHERE message NOT LIKE '%Z/%' AND message NOT LIKE 'querying next range at%' AND tag NOT LIKE '%IndexBackfiller%' AND operation != 'dist sender send' ---- -flow Put /Table/3/1/54/2/1 -> table: columns: > nullable:false hidden:false > columns: > nullable:true hidden:false > next_column_id:3 families: next_family_id:1 primary_index: interleave:<> partitioning: type:FORWARD > next_index_id:3 privileges: users: > mutations: interleave:<> partitioning: type:FORWARD > state:DELETE_AND_WRITE_ONLY direction:DROP mutation_id:2 rollback:false > next_mutation_id:3 format_version:3 state:PUBLIC offline_reason:"" view_query:"" mutationJobs:<...> drop_time:0 replacement_of: > audit_mode:DISABLED drop_job_id:0 create_query:"" create_as_of_time: > +flow Put /Table/3/1/54/2/1 -> table: columns: > nullable:false hidden:false > columns: > nullable:true hidden:false > next_column_id:3 families: next_family_id:1 primary_index: interleave:<> partitioning: type:FORWARD > next_index_id:3 privileges: users: > mutations: interleave:<> partitioning: type:FORWARD > state:DELETE_AND_WRITE_ONLY direction:DROP mutation_id:2 rollback:false > next_mutation_id:3 format_version:3 state:PUBLIC offline_reason:"" view_query:"" mutationJobs:<...> drop_time:0 replacement_of: > audit_mode:DISABLED drop_job_id:0 create_query:"" create_as_of_time: > exec stmt rows affected: 0 statement ok @@ -224,7 +224,7 @@ WHERE message NOT LIKE '%Z/%' AND message NOT LIKE 'querying next range at%' AND tag NOT LIKE '%IndexBackfiller%' AND operation != 'dist sender send' ---- -flow Put /Table/3/1/54/2/1 -> table: columns: > nullable:false hidden:false > columns: > nullable:true hidden:false > next_column_id:3 families: next_family_id:1 primary_index: interleave:<> partitioning: type:FORWARD > next_index_id:3 privileges: users: > next_mutation_id:3 format_version:3 state:DROP offline_reason:"" draining_names: view_query:"" drop_time:... replacement_of: > audit_mode:DISABLED drop_job_id:... gc_mutations: create_query:"" create_as_of_time: > +flow Put /Table/3/1/54/2/1 -> table: columns: > nullable:false hidden:false > columns: > nullable:true hidden:false > next_column_id:3 families: next_family_id:1 primary_index: interleave:<> partitioning: type:FORWARD > next_index_id:3 privileges: users: > next_mutation_id:3 format_version:3 state:DROP offline_reason:"" draining_names: view_query:"" drop_time:... replacement_of: > audit_mode:DISABLED drop_job_id:... gc_mutations: create_query:"" create_as_of_time: > exec stmt rows affected: 0 # Check that session tracing does not inhibit the fast path for inserts & diff --git a/pkg/sql/plan.go b/pkg/sql/plan.go index 2dc1659ae0ee..8b12b4426280 100644 --- a/pkg/sql/plan.go +++ b/pkg/sql/plan.go @@ -14,6 +14,7 @@ import ( "context" "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/sql/delegate" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" @@ -22,6 +23,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/sqlbase" "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented" + "github.com/cockroachdb/cockroach/pkg/util/hlc" "github.com/cockroachdb/cockroach/pkg/util/tracing" "github.com/cockroachdb/errors" ) @@ -61,6 +63,24 @@ func (r *runParams) Ann() *tree.Annotations { return r.extendedEvalCtx.EvalContext.Annotations } +// createTimeForNewTableDescriptor consults the cluster version to determine +// whether the CommitTimestamp() needs to be observed when creating a new +// TableDescriptor. See TableDescriptor.ModificationTime. +// +// TODO(ajwerner): remove in 20.1. +func (r *runParams) creationTimeForNewTableDescriptor() hlc.Timestamp { + // Before 19.2 we needed to observe the transaction CommitTimestamp to ensure + // that CreateAsOfTime and ModificationTime reflected the timestamp at which the + // creating transaction committed. Starting in 19.2 we use a zero-valued + // CreateAsOfTime and ModificationTime when creating a table descriptor and then + // upon reading use the MVCC timestamp to populate the values. + var ts hlc.Timestamp + if !r.ExecCfg().Settings.Version.IsActive(cluster.VersionTableDescModificationTimeFromMVCC) { + ts = r.p.txn.CommitTimestamp() + } + return ts +} + // planNode defines the interface for executing a query or portion of a query. // // The following methods apply to planNodes and contain special cases diff --git a/pkg/sql/rename_test.go b/pkg/sql/rename_test.go index 56ab6d8dae38..d71e57064e2e 100644 --- a/pkg/sql/rename_test.go +++ b/pkg/sql/rename_test.go @@ -49,10 +49,11 @@ func TestRenameTable(t *testing.T) { // Check the table descriptor. desc := &sqlbase.Descriptor{} tableDescKey := sqlbase.MakeDescMetadataKey(sqlbase.ID(counter)) - if err := kvDB.GetProto(context.TODO(), tableDescKey, desc); err != nil { + ts, err := kvDB.GetProtoTs(context.TODO(), tableDescKey, desc) + if err != nil { t.Fatal(err) } - tableDesc := desc.GetTable() + tableDesc := desc.Table(ts) if tableDesc.Name != oldName { t.Fatalf("Wrong table name, expected %s, got: %+v", oldName, tableDesc) } @@ -74,10 +75,11 @@ func TestRenameTable(t *testing.T) { } // Check the table descriptor again. - if err := kvDB.GetProto(context.TODO(), tableDescKey, desc); err != nil { + ts, err = kvDB.GetProtoTs(context.TODO(), tableDescKey, desc) + if err != nil { t.Fatal(err) } - tableDesc = desc.GetTable() + tableDesc = desc.Table(ts) if tableDesc.Name != newName { t.Fatalf("Wrong table name, expected %s, got: %+v", newName, tableDesc) } @@ -103,7 +105,7 @@ func isRenamed( if err := val.GetProto(&descriptor); err != nil { panic("unable to unmarshal table descriptor") } - table := descriptor.GetTable() + table := descriptor.Table(val.Timestamp) return table.Name == expectedName && table.Version == expectedVersion } diff --git a/pkg/sql/resolver.go b/pkg/sql/resolver.go index 927b61632a23..f2d623b63dfe 100644 --- a/pkg/sql/resolver.go +++ b/pkg/sql/resolver.go @@ -23,6 +23,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sessiondata" "github.com/cockroachdb/cockroach/pkg/sql/sqlbase" + "github.com/cockroachdb/cockroach/pkg/util/hlc" "github.com/cockroachdb/errors" ) @@ -611,7 +612,7 @@ func newInternalLookupCtxFromDescriptors( if prefix == nil || prefix.ID == database.ID { dbIDs = append(dbIDs, database.ID) } - } else if table := desc.GetTable(); table != nil { + } else if table := desc.Table(hlc.Timestamp{}); table != nil { tbDescs[table.ID] = table if prefix == nil || prefix.ID == table.ParentID { // Only make the table visible for iteration if the prefix was included. diff --git a/pkg/sql/schema_changer_test.go b/pkg/sql/schema_changer_test.go index ba3817b4639a..a2b19d5b9bbc 100644 --- a/pkg/sql/schema_changer_test.go +++ b/pkg/sql/schema_changer_test.go @@ -3833,12 +3833,9 @@ func TestSchemaChangeRetryError(t *testing.T) { t.Fatal(err) } - // TODO(vivek): fix #17698. The transaction should get retried - // without returning this error to the user. - if err := tx.Commit(); !testutils.IsError(err, - `restart transaction: TransactionRetryWithProtoRefreshError: TransactionRetryError: retry txn \(RETRY_SERIALIZABLE\)`, - ) { - t.Fatalf("err = %+v", err) + // The transaction should get pushed and commit without an error. + if err := tx.Commit(); err != nil { + t.Fatal(err) } } diff --git a/pkg/sql/sqlbase/structured.go b/pkg/sql/sqlbase/structured.go index 138276618a8f..1c2cf155e54d 100644 --- a/pkg/sql/sqlbase/structured.go +++ b/pkg/sql/sqlbase/structured.go @@ -29,6 +29,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/cockroach/pkg/util/encoding" "github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented" + "github.com/cockroachdb/cockroach/pkg/util/hlc" "github.com/cockroachdb/cockroach/pkg/util/interval" "github.com/cockroachdb/cockroach/pkg/util/log" "github.com/cockroachdb/cockroach/pkg/util/protoutil" @@ -280,10 +281,10 @@ func NewImmutableTableDescriptor(tbl TableDescriptor) *ImmutableTableDescriptor // protoGetter is a sub-interface of client.Txn that can fetch protobufs in a // transaction. type protoGetter interface { - // GetProto retrieves a protoutil.Message that's stored at key, storing it + // GetProtoTs retrieves a protoutil.Message that's stored at key, storing it // into the input msg parameter. If the key doesn't exist, the input proto // will be reset. - GetProto(ctx context.Context, key interface{}, msg protoutil.Message) error + GetProtoTs(ctx context.Context, key interface{}, msg protoutil.Message) (hlc.Timestamp, error) } // GetDatabaseDescFromID retrieves the database descriptor for the database @@ -294,8 +295,8 @@ func GetDatabaseDescFromID( ) (*DatabaseDescriptor, error) { desc := &Descriptor{} descKey := MakeDescMetadataKey(id) - - if err := protoGetter.GetProto(ctx, descKey, desc); err != nil { + _, err := protoGetter.GetProtoTs(ctx, descKey, desc) + if err != nil { return nil, err } db := desc.GetDatabase() @@ -334,15 +335,14 @@ func getTableDescFromIDRaw( ) (*TableDescriptor, error) { desc := &Descriptor{} descKey := MakeDescMetadataKey(id) - - if err := protoGetter.GetProto(ctx, descKey, desc); err != nil { + ts, err := protoGetter.GetProtoTs(ctx, descKey, desc) + if err != nil { return nil, err } - table := desc.GetTable() + table := desc.Table(ts) if table == nil { return nil, ErrDescriptorNotFound } - return table, nil } @@ -773,8 +773,8 @@ type MapProtoGetter struct { Protos map[interface{}]protoutil.Message } -// GetProto implements the protoGetter interface. -func (m MapProtoGetter) GetProto( +// getProto implements the protoGetter interface. +func (m MapProtoGetter) getProto( ctx context.Context, key interface{}, msg protoutil.Message, ) error { msg.Reset() @@ -790,6 +790,13 @@ func (m MapProtoGetter) GetProto( return nil } +// GetProtoTs implements the protoGetter interface. +func (m MapProtoGetter) GetProtoTs( + ctx context.Context, key interface{}, msg protoutil.Message, +) (hlc.Timestamp, error) { + return hlc.Timestamp{}, m.getProto(ctx, key, msg) +} + // MaybeUpgradeForeignKeyRepresentation destructively modifies the input table // descriptor by replacing all old-style foreign key references (the ForeignKey // and ReferencedBy fields on IndexDescriptor) with new-style foreign key @@ -1486,27 +1493,25 @@ func (desc *MutableTableDescriptor) allocateColumnFamilyIDs(columnNames map[stri // MaybeIncrementVersion increments the version of a descriptor if necessary. func (desc *MutableTableDescriptor) MaybeIncrementVersion( - ctx context.Context, txn *client.Txn, + ctx context.Context, txn *client.Txn, settings *cluster.Settings, ) error { // Already incremented, no-op. if desc.Version == desc.ClusterVersion.Version+1 { return nil } desc.Version++ - // We need to set ModificationTime to the transaction's commit - // timestamp. Using CommitTimestamp() guarantees that the - // transaction will commit at the CommitTimestamp(). + + // Before 19.2 we needed to observe the transaction CommitTimestamp to ensure + // that ModificationTime reflected the timestamp at which the transaction + // committed. Starting in 19.2 we use a zero-valued ModificationTime when + // incrementing the version and then upon reading use the MVCC timestamp to + // populate the ModificationTime. // - // TODO(vivek): Stop needing to do this by deprecating the - // ModificationTime. A Descriptor modification time can be - // the mvcc timestamp of the descriptor. This requires moving the - // schema change lease out of the descriptor making the - // descriptor truly immutable at a version. - // Also recognize that the leases are released before the transaction - // is committed through a call to TableCollection.releaseLeases(), - // so updating this policy will also need to consider not doing - // that. - modTime := txn.CommitTimestamp() + // TODO(ajwerner): remove this check in 20.1. + var modTime hlc.Timestamp + if !settings.Version.IsActive(cluster.VersionTableDescModificationTimeFromMVCC) { + modTime = txn.CommitTimestamp() + } desc.ModificationTime = modTime log.Infof(ctx, "publish: descID=%d (%s) version=%d mtime=%s", desc.ID, desc.Name, desc.Version, modTime.GoTime()) @@ -3266,6 +3271,77 @@ func (desc *Descriptor) GetName() string { } } +// Table is a replacement for GetTable() which seeks to ensure that clients +// which unmarshal Descriptor structs properly set the ModificationTime on +// tables based on the MVCC timestamp at which the descriptor was read. +// +// A linter should ensure that GetTable() is not called. +func (desc *Descriptor) Table(ts hlc.Timestamp) *TableDescriptor { + t := desc.GetTable() + if t != nil { + t.maybeSetTimeFromMVCCTimestamp(ts) + } + return t +} + +// maybeSetTimeFromMVCCTimestamp will update ModificationTime and possible +// CreateAsOfTime with the provided timestamp. If desc.ModificationTime is +// non-zero it must be the case that it is not after the provided timestamp. +// +// When table descriptor versions are incremented they are written with a +// zero-valued ModificationTime. This is done to avoid the need to observe +// the commit timestamp for the writing transaction which would prevent +// pushes. This method is used in the read path to set the modification time +// based on the MVCC timestamp of row which contained this descriptor. If +// the ModificationTime is non-zero then we know that either this table +// descriptor was written by older version of cockroach which included the +// exact commit timestamp or it was re-written in which case it will include +// a timestamp which was set by this method. +// +// It is vital that users which read table descriptor values from the KV store +// call this method. +func (desc *TableDescriptor) maybeSetTimeFromMVCCTimestamp(ts hlc.Timestamp) { + // CreateAsOfTime is used for CREATE TABLE ... AS ... and was introduced in + // v19.1. In general it is not critical to set except for tables in the ADD + // ADD state which were created from CTAS so we should not assert on its not + // being set. It's not always sensical to set it from the passed MVCC + // timestamp. However, starting in 19.2 the CreateAsOfTime and + // ModificationTime fields are both unset for the first Version of a + // TableDescriptor and the code relies on the value being set based on the + // MVCC timestamp. + if !ts.IsEmpty() && + desc.ModificationTime.IsEmpty() && + desc.CreateAsOfTime.IsEmpty() && + desc.Version == 1 { + desc.CreateAsOfTime = ts + } + // Ensure that if the table is in the process of being added and relies on + // CreateAsOfTime that it is now set. + if desc.Adding() && desc.IsAs() && desc.CreateAsOfTime.IsEmpty() { + log.Fatalf(context.TODO(), "table descriptor for %q (%d.%d) is in the "+ + "ADD state and was created with CREATE TABLE ... AS but does not have a "+ + "CreateAsOfTime set", desc.Name, desc.ParentID, desc.ID) + } + + // Set the ModificationTime based on the passed ts if we should. + // Table descriptors can be updated in place after their version has been + // incremented (e.g. to include a schema change lease). + // When this happens we permit the ModificationTime to be written explicitly + // with the value that lives on the in-memory copy. That value should contain + // a timestamp set by this method. Thus if the ModificationTime is set it + // must not be after the MVCC timestamp we just read it at. + if desc.ModificationTime.IsEmpty() && ts.IsEmpty() { + log.Fatalf(context.TODO(), "read table descriptor for %q (%d.%d) without ModificationTime "+ + "with zero MVCC timestamp", desc.Name, desc.ParentID, desc.ID) + } else if desc.ModificationTime.IsEmpty() { + desc.ModificationTime = ts + } else if !ts.IsEmpty() && ts.Less(desc.ModificationTime) { + log.Fatalf(context.TODO(), "read table descriptor %q (%d.%d) which has a ModificationTime "+ + "after its MVCC timestamp: has %v, expected %v", + desc.Name, desc.ParentID, desc.ID, desc.ModificationTime, ts) + } +} + // IsSet returns whether or not the foreign key actually references a table. func (f ForeignKeyReference) IsSet() bool { return f.Table != 0 diff --git a/pkg/sql/sqlbase/structured.pb.go b/pkg/sql/sqlbase/structured.pb.go index 8aa8722f77a0..84888b9b82c4 100644 --- a/pkg/sql/sqlbase/structured.pb.go +++ b/pkg/sql/sqlbase/structured.pb.go @@ -73,7 +73,7 @@ func (x *ConstraintValidity) UnmarshalJSON(data []byte) error { return nil } func (ConstraintValidity) EnumDescriptor() ([]byte, []int) { - return fileDescriptor_structured_188f44329a8fdff9, []int{0} + return fileDescriptor_structured_4ce39be31035aa5b, []int{0} } type ForeignKeyReference_Action int32 @@ -118,7 +118,7 @@ func (x *ForeignKeyReference_Action) UnmarshalJSON(data []byte) error { return nil } func (ForeignKeyReference_Action) EnumDescriptor() ([]byte, []int) { - return fileDescriptor_structured_188f44329a8fdff9, []int{0, 0} + return fileDescriptor_structured_4ce39be31035aa5b, []int{0, 0} } // Match is the algorithm used to compare composite keys. @@ -158,7 +158,7 @@ func (x *ForeignKeyReference_Match) UnmarshalJSON(data []byte) error { return nil } func (ForeignKeyReference_Match) EnumDescriptor() ([]byte, []int) { - return fileDescriptor_structured_188f44329a8fdff9, []int{0, 1} + return fileDescriptor_structured_4ce39be31035aa5b, []int{0, 1} } // The direction of a column in the index. @@ -195,7 +195,7 @@ func (x *IndexDescriptor_Direction) UnmarshalJSON(data []byte) error { return nil } func (IndexDescriptor_Direction) EnumDescriptor() ([]byte, []int) { - return fileDescriptor_structured_188f44329a8fdff9, []int{6, 0} + return fileDescriptor_structured_4ce39be31035aa5b, []int{6, 0} } // The type of the index. @@ -232,7 +232,7 @@ func (x *IndexDescriptor_Type) UnmarshalJSON(data []byte) error { return nil } func (IndexDescriptor_Type) EnumDescriptor() ([]byte, []int) { - return fileDescriptor_structured_188f44329a8fdff9, []int{6, 1} + return fileDescriptor_structured_4ce39be31035aa5b, []int{6, 1} } type ConstraintToUpdate_ConstraintType int32 @@ -275,7 +275,7 @@ func (x *ConstraintToUpdate_ConstraintType) UnmarshalJSON(data []byte) error { return nil } func (ConstraintToUpdate_ConstraintType) EnumDescriptor() ([]byte, []int) { - return fileDescriptor_structured_188f44329a8fdff9, []int{7, 0} + return fileDescriptor_structured_4ce39be31035aa5b, []int{7, 0} } // A descriptor within a mutation is unavailable for reads, writes @@ -340,7 +340,7 @@ func (x *DescriptorMutation_State) UnmarshalJSON(data []byte) error { return nil } func (DescriptorMutation_State) EnumDescriptor() ([]byte, []int) { - return fileDescriptor_structured_188f44329a8fdff9, []int{8, 0} + return fileDescriptor_structured_4ce39be31035aa5b, []int{8, 0} } // Direction of mutation. @@ -383,7 +383,7 @@ func (x *DescriptorMutation_Direction) UnmarshalJSON(data []byte) error { return nil } func (DescriptorMutation_Direction) EnumDescriptor() ([]byte, []int) { - return fileDescriptor_structured_188f44329a8fdff9, []int{8, 1} + return fileDescriptor_structured_4ce39be31035aa5b, []int{8, 1} } // State is set if this TableDescriptor is in the process of being added or deleted. @@ -434,7 +434,7 @@ func (x *TableDescriptor_State) UnmarshalJSON(data []byte) error { return nil } func (TableDescriptor_State) EnumDescriptor() ([]byte, []int) { - return fileDescriptor_structured_188f44329a8fdff9, []int{9, 0} + return fileDescriptor_structured_4ce39be31035aa5b, []int{9, 0} } // AuditMode indicates which auditing actions to take when this table is used. @@ -471,7 +471,7 @@ func (x *TableDescriptor_AuditMode) UnmarshalJSON(data []byte) error { return nil } func (TableDescriptor_AuditMode) EnumDescriptor() ([]byte, []int) { - return fileDescriptor_structured_188f44329a8fdff9, []int{9, 1} + return fileDescriptor_structured_4ce39be31035aa5b, []int{9, 1} } type ForeignKeyReference struct { @@ -493,7 +493,7 @@ func (m *ForeignKeyReference) Reset() { *m = ForeignKeyReference{} } func (m *ForeignKeyReference) String() string { return proto.CompactTextString(m) } func (*ForeignKeyReference) ProtoMessage() {} func (*ForeignKeyReference) Descriptor() ([]byte, []int) { - return fileDescriptor_structured_188f44329a8fdff9, []int{0} + return fileDescriptor_structured_4ce39be31035aa5b, []int{0} } func (m *ForeignKeyReference) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -561,7 +561,7 @@ func (m *ForeignKeyConstraint) Reset() { *m = ForeignKeyConstraint{} } func (m *ForeignKeyConstraint) String() string { return proto.CompactTextString(m) } func (*ForeignKeyConstraint) ProtoMessage() {} func (*ForeignKeyConstraint) Descriptor() ([]byte, []int) { - return fileDescriptor_structured_188f44329a8fdff9, []int{1} + return fileDescriptor_structured_4ce39be31035aa5b, []int{1} } func (m *ForeignKeyConstraint) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -606,7 +606,7 @@ func (m *ColumnDescriptor) Reset() { *m = ColumnDescriptor{} } func (m *ColumnDescriptor) String() string { return proto.CompactTextString(m) } func (*ColumnDescriptor) ProtoMessage() {} func (*ColumnDescriptor) Descriptor() ([]byte, []int) { - return fileDescriptor_structured_188f44329a8fdff9, []int{2} + return fileDescriptor_structured_4ce39be31035aa5b, []int{2} } func (m *ColumnDescriptor) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -657,7 +657,7 @@ func (m *ColumnFamilyDescriptor) Reset() { *m = ColumnFamilyDescriptor{} func (m *ColumnFamilyDescriptor) String() string { return proto.CompactTextString(m) } func (*ColumnFamilyDescriptor) ProtoMessage() {} func (*ColumnFamilyDescriptor) Descriptor() ([]byte, []int) { - return fileDescriptor_structured_188f44329a8fdff9, []int{3} + return fileDescriptor_structured_4ce39be31035aa5b, []int{3} } func (m *ColumnFamilyDescriptor) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -703,7 +703,7 @@ func (m *InterleaveDescriptor) Reset() { *m = InterleaveDescriptor{} } func (m *InterleaveDescriptor) String() string { return proto.CompactTextString(m) } func (*InterleaveDescriptor) ProtoMessage() {} func (*InterleaveDescriptor) Descriptor() ([]byte, []int) { - return fileDescriptor_structured_188f44329a8fdff9, []int{4} + return fileDescriptor_structured_4ce39be31035aa5b, []int{4} } func (m *InterleaveDescriptor) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -747,7 +747,7 @@ func (m *InterleaveDescriptor_Ancestor) Reset() { *m = InterleaveDescrip func (m *InterleaveDescriptor_Ancestor) String() string { return proto.CompactTextString(m) } func (*InterleaveDescriptor_Ancestor) ProtoMessage() {} func (*InterleaveDescriptor_Ancestor) Descriptor() ([]byte, []int) { - return fileDescriptor_structured_188f44329a8fdff9, []int{4, 0} + return fileDescriptor_structured_4ce39be31035aa5b, []int{4, 0} } func (m *InterleaveDescriptor_Ancestor) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -792,7 +792,7 @@ func (m *PartitioningDescriptor) Reset() { *m = PartitioningDescriptor{} func (m *PartitioningDescriptor) String() string { return proto.CompactTextString(m) } func (*PartitioningDescriptor) ProtoMessage() {} func (*PartitioningDescriptor) Descriptor() ([]byte, []int) { - return fileDescriptor_structured_188f44329a8fdff9, []int{5} + return fileDescriptor_structured_4ce39be31035aa5b, []int{5} } func (m *PartitioningDescriptor) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -835,7 +835,7 @@ func (m *PartitioningDescriptor_List) Reset() { *m = PartitioningDescrip func (m *PartitioningDescriptor_List) String() string { return proto.CompactTextString(m) } func (*PartitioningDescriptor_List) ProtoMessage() {} func (*PartitioningDescriptor_List) Descriptor() ([]byte, []int) { - return fileDescriptor_structured_188f44329a8fdff9, []int{5, 0} + return fileDescriptor_structured_4ce39be31035aa5b, []int{5, 0} } func (m *PartitioningDescriptor_List) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -880,7 +880,7 @@ func (m *PartitioningDescriptor_Range) Reset() { *m = PartitioningDescri func (m *PartitioningDescriptor_Range) String() string { return proto.CompactTextString(m) } func (*PartitioningDescriptor_Range) ProtoMessage() {} func (*PartitioningDescriptor_Range) Descriptor() ([]byte, []int) { - return fileDescriptor_structured_188f44329a8fdff9, []int{5, 1} + return fileDescriptor_structured_4ce39be31035aa5b, []int{5, 1} } func (m *PartitioningDescriptor_Range) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -1013,7 +1013,7 @@ func (m *IndexDescriptor) Reset() { *m = IndexDescriptor{} } func (m *IndexDescriptor) String() string { return proto.CompactTextString(m) } func (*IndexDescriptor) ProtoMessage() {} func (*IndexDescriptor) Descriptor() ([]byte, []int) { - return fileDescriptor_structured_188f44329a8fdff9, []int{6} + return fileDescriptor_structured_4ce39be31035aa5b, []int{6} } func (m *IndexDescriptor) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -1064,7 +1064,7 @@ func (m *ConstraintToUpdate) Reset() { *m = ConstraintToUpdate{} } func (m *ConstraintToUpdate) String() string { return proto.CompactTextString(m) } func (*ConstraintToUpdate) ProtoMessage() {} func (*ConstraintToUpdate) Descriptor() ([]byte, []int) { - return fileDescriptor_structured_188f44329a8fdff9, []int{7} + return fileDescriptor_structured_4ce39be31035aa5b, []int{7} } func (m *ConstraintToUpdate) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -1118,7 +1118,7 @@ func (m *DescriptorMutation) Reset() { *m = DescriptorMutation{} } func (m *DescriptorMutation) String() string { return proto.CompactTextString(m) } func (*DescriptorMutation) ProtoMessage() {} func (*DescriptorMutation) Descriptor() ([]byte, []int) { - return fileDescriptor_structured_188f44329a8fdff9, []int{8} + return fileDescriptor_structured_4ce39be31035aa5b, []int{8} } func (m *DescriptorMutation) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -1313,6 +1313,14 @@ type TableDescriptor struct { // particular version increment. Version DescriptorVersion `protobuf:"varint,5,opt,name=version,casttype=DescriptorVersion" json:"version"` // Last modification time of the table descriptor. + // Starting in 19.2 this field's value may sometime be zero-valued in which + // case the MVCC timestamp of the row containing the value should be used to + // populate it. This dance allows us to avoid observing the commit timestamp + // for transactions which increment the descriptor version. + // Encoded TableDescriptor structs should not be stored directly but rather + // should live inside of a Descriptor. The Descriptor.Table() method takes an + // hlc timestamp to ensure that this field is set properly when extracted from + // a Descriptor. ModificationTime hlc.Timestamp `protobuf:"bytes,7,opt,name=modification_time,json=modificationTime" json:"modification_time"` Columns []ColumnDescriptor `protobuf:"bytes,8,rep,name=columns" json:"columns"` // next_column_id is used to ensure that deleted column ids are not reused. @@ -1395,9 +1403,14 @@ type TableDescriptor struct { // // TODO(vivekmenezes): This is currently only used by the non-interleaved drop // index case. Also use for dropped interleaved indexes and columns. - GCMutations []TableDescriptor_GCDescriptorMutation `protobuf:"bytes,33,rep,name=gc_mutations,json=gcMutations" json:"gc_mutations"` - CreateQuery string `protobuf:"bytes,34,opt,name=create_query,json=createQuery" json:"create_query"` - CreateAsOfTime hlc.Timestamp `protobuf:"bytes,35,opt,name=create_as_of_time,json=createAsOfTime" json:"create_as_of_time"` + GCMutations []TableDescriptor_GCDescriptorMutation `protobuf:"bytes,33,rep,name=gc_mutations,json=gcMutations" json:"gc_mutations"` + CreateQuery string `protobuf:"bytes,34,opt,name=create_query,json=createQuery" json:"create_query"` + // Starting in 19.2 CreateAsOfTime is initialized to zero for the first + // version of a table and is populated from the MVCC timestamp of the read + // like ModificationTime. See Descriptor.Table(). + // CreateAsOfSystemTime is used for CREATE TABLE ... AS ... and was + // added in 19.1. + CreateAsOfTime hlc.Timestamp `protobuf:"bytes,35,opt,name=create_as_of_time,json=createAsOfTime" json:"create_as_of_time"` // outbound_fks contains all foreign key constraints that have this table as // the origin table. OutboundFKs []ForeignKeyConstraint `protobuf:"bytes,36,rep,name=outbound_fks,json=outboundFks" json:"outbound_fks"` @@ -1410,7 +1423,7 @@ func (m *TableDescriptor) Reset() { *m = TableDescriptor{} } func (m *TableDescriptor) String() string { return proto.CompactTextString(m) } func (*TableDescriptor) ProtoMessage() {} func (*TableDescriptor) Descriptor() ([]byte, []int) { - return fileDescriptor_structured_188f44329a8fdff9, []int{9} + return fileDescriptor_structured_4ce39be31035aa5b, []int{9} } func (m *TableDescriptor) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -1695,7 +1708,7 @@ func (m *TableDescriptor_SchemaChangeLease) Reset() { *m = TableDescript func (m *TableDescriptor_SchemaChangeLease) String() string { return proto.CompactTextString(m) } func (*TableDescriptor_SchemaChangeLease) ProtoMessage() {} func (*TableDescriptor_SchemaChangeLease) Descriptor() ([]byte, []int) { - return fileDescriptor_structured_188f44329a8fdff9, []int{9, 0} + return fileDescriptor_structured_4ce39be31035aa5b, []int{9, 0} } func (m *TableDescriptor_SchemaChangeLease) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -1733,7 +1746,7 @@ func (m *TableDescriptor_CheckConstraint) Reset() { *m = TableDescriptor func (m *TableDescriptor_CheckConstraint) String() string { return proto.CompactTextString(m) } func (*TableDescriptor_CheckConstraint) ProtoMessage() {} func (*TableDescriptor_CheckConstraint) Descriptor() ([]byte, []int) { - return fileDescriptor_structured_188f44329a8fdff9, []int{9, 1} + return fileDescriptor_structured_4ce39be31035aa5b, []int{9, 1} } func (m *TableDescriptor_CheckConstraint) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -1836,7 +1849,7 @@ func (m *TableDescriptor_NameInfo) Reset() { *m = TableDescriptor_NameIn func (m *TableDescriptor_NameInfo) String() string { return proto.CompactTextString(m) } func (*TableDescriptor_NameInfo) ProtoMessage() {} func (*TableDescriptor_NameInfo) Descriptor() ([]byte, []int) { - return fileDescriptor_structured_188f44329a8fdff9, []int{9, 2} + return fileDescriptor_structured_4ce39be31035aa5b, []int{9, 2} } func (m *TableDescriptor_NameInfo) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -1876,7 +1889,7 @@ func (m *TableDescriptor_Reference) Reset() { *m = TableDescriptor_Refer func (m *TableDescriptor_Reference) String() string { return proto.CompactTextString(m) } func (*TableDescriptor_Reference) ProtoMessage() {} func (*TableDescriptor_Reference) Descriptor() ([]byte, []int) { - return fileDescriptor_structured_188f44329a8fdff9, []int{9, 3} + return fileDescriptor_structured_4ce39be31035aa5b, []int{9, 3} } func (m *TableDescriptor_Reference) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -1913,7 +1926,7 @@ func (m *TableDescriptor_MutationJob) Reset() { *m = TableDescriptor_Mut func (m *TableDescriptor_MutationJob) String() string { return proto.CompactTextString(m) } func (*TableDescriptor_MutationJob) ProtoMessage() {} func (*TableDescriptor_MutationJob) Descriptor() ([]byte, []int) { - return fileDescriptor_structured_188f44329a8fdff9, []int{9, 4} + return fileDescriptor_structured_4ce39be31035aa5b, []int{9, 4} } func (m *TableDescriptor_MutationJob) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -1955,7 +1968,7 @@ func (m *TableDescriptor_SequenceOpts) Reset() { *m = TableDescriptor_Se func (m *TableDescriptor_SequenceOpts) String() string { return proto.CompactTextString(m) } func (*TableDescriptor_SequenceOpts) ProtoMessage() {} func (*TableDescriptor_SequenceOpts) Descriptor() ([]byte, []int) { - return fileDescriptor_structured_188f44329a8fdff9, []int{9, 5} + return fileDescriptor_structured_4ce39be31035aa5b, []int{9, 5} } func (m *TableDescriptor_SequenceOpts) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -1989,7 +2002,7 @@ func (m *TableDescriptor_Replacement) Reset() { *m = TableDescriptor_Rep func (m *TableDescriptor_Replacement) String() string { return proto.CompactTextString(m) } func (*TableDescriptor_Replacement) ProtoMessage() {} func (*TableDescriptor_Replacement) Descriptor() ([]byte, []int) { - return fileDescriptor_structured_188f44329a8fdff9, []int{9, 6} + return fileDescriptor_structured_4ce39be31035aa5b, []int{9, 6} } func (m *TableDescriptor_Replacement) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -2026,7 +2039,7 @@ func (m *TableDescriptor_GCDescriptorMutation) Reset() { *m = TableDescr func (m *TableDescriptor_GCDescriptorMutation) String() string { return proto.CompactTextString(m) } func (*TableDescriptor_GCDescriptorMutation) ProtoMessage() {} func (*TableDescriptor_GCDescriptorMutation) Descriptor() ([]byte, []int) { - return fileDescriptor_structured_188f44329a8fdff9, []int{9, 7} + return fileDescriptor_structured_4ce39be31035aa5b, []int{9, 7} } func (m *TableDescriptor_GCDescriptorMutation) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -2065,7 +2078,7 @@ func (m *DatabaseDescriptor) Reset() { *m = DatabaseDescriptor{} } func (m *DatabaseDescriptor) String() string { return proto.CompactTextString(m) } func (*DatabaseDescriptor) ProtoMessage() {} func (*DatabaseDescriptor) Descriptor() ([]byte, []int) { - return fileDescriptor_structured_188f44329a8fdff9, []int{10} + return fileDescriptor_structured_4ce39be31035aa5b, []int{10} } func (m *DatabaseDescriptor) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -2123,7 +2136,7 @@ func (m *Descriptor) Reset() { *m = Descriptor{} } func (m *Descriptor) String() string { return proto.CompactTextString(m) } func (*Descriptor) ProtoMessage() {} func (*Descriptor) Descriptor() ([]byte, []int) { - return fileDescriptor_structured_188f44329a8fdff9, []int{11} + return fileDescriptor_structured_4ce39be31035aa5b, []int{11} } func (m *Descriptor) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -10723,10 +10736,10 @@ var ( ) func init() { - proto.RegisterFile("sql/sqlbase/structured.proto", fileDescriptor_structured_188f44329a8fdff9) + proto.RegisterFile("sql/sqlbase/structured.proto", fileDescriptor_structured_4ce39be31035aa5b) } -var fileDescriptor_structured_188f44329a8fdff9 = []byte{ +var fileDescriptor_structured_4ce39be31035aa5b = []byte{ // 3345 bytes of a gzipped FileDescriptorProto 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0xb4, 0x5a, 0xcd, 0x6f, 0x23, 0xc7, 0x95, 0x57, 0xf3, 0x9b, 0x8f, 0x5f, 0xad, 0xd2, 0xcc, 0x98, 0x23, 0x8f, 0x45, 0x0e, 0xc7, 0x63, diff --git a/pkg/sql/sqlbase/structured.proto b/pkg/sql/sqlbase/structured.proto index 27696824b8d0..51b773d249aa 100644 --- a/pkg/sql/sqlbase/structured.proto +++ b/pkg/sql/sqlbase/structured.proto @@ -521,6 +521,14 @@ message TableDescriptor { reserved 6; // Last modification time of the table descriptor. + // Starting in 19.2 this field's value may sometime be zero-valued in which + // case the MVCC timestamp of the row containing the value should be used to + // populate it. This dance allows us to avoid observing the commit timestamp + // for transactions which increment the descriptor version. + // Encoded TableDescriptor structs should not be stored directly but rather + // should live inside of a Descriptor. The Descriptor.Table() method takes an + // hlc timestamp to ensure that this field is set properly when extracted from + // a Descriptor. optional util.hlc.Timestamp modification_time = 7 [(gogoproto.nullable) = false]; repeated ColumnDescriptor columns = 8 [(gogoproto.nullable) = false]; // next_column_id is used to ensure that deleted column ids are not reused. @@ -811,6 +819,12 @@ message TableDescriptor { (gogoproto.customname) = "GCMutations"]; optional string create_query = 34 [(gogoproto.nullable) = false]; + + // Starting in 19.2 CreateAsOfTime is initialized to zero for the first + // version of a table and is populated from the MVCC timestamp of the read + // like ModificationTime. See Descriptor.Table(). + // CreateAsOfSystemTime is used for CREATE TABLE ... AS ... and was + // added in 19.1. optional util.hlc.Timestamp create_as_of_time = 35 [(gogoproto.nullable) = false]; // outbound_fks contains all foreign key constraints that have this table as diff --git a/pkg/sql/sqlbase/structured_test.go b/pkg/sql/sqlbase/structured_test.go index 17adb07754d5..6ab8a0f35666 100644 --- a/pkg/sql/sqlbase/structured_test.go +++ b/pkg/sql/sqlbase/structured_test.go @@ -27,6 +27,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/testutils" "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" + "github.com/cockroachdb/cockroach/pkg/util/hlc" "github.com/cockroachdb/cockroach/pkg/util/leaktest" "github.com/cockroachdb/cockroach/pkg/util/protoutil" "github.com/gogo/protobuf/proto" @@ -1356,7 +1357,8 @@ func TestUpgradeDowngradeFKRepr(t *testing.T) { cluster.BinaryMinimumSupportedVersion, cluster.VersionByKey(cluster.VersionTopLevelForeignKeys), ) - + // Use a non-zero ts for CreateAsOfTime and ModificationTime + ts := hlc.Timestamp{WallTime: 1} testCases := []struct { name string origin oldFormatUpgradedPair @@ -1366,8 +1368,10 @@ func TestUpgradeDowngradeFKRepr(t *testing.T) { name: "simple", origin: oldFormatUpgradedPair{ oldFormat: TableDescriptor{ - ID: 1, - Columns: []ColumnDescriptor{{ID: 1}, {ID: 2}}, + ID: 1, + CreateAsOfTime: ts, + ModificationTime: ts, + Columns: []ColumnDescriptor{{ID: 1}, {ID: 2}}, Indexes: []IndexDescriptor{ { ID: 1, @@ -1386,8 +1390,10 @@ func TestUpgradeDowngradeFKRepr(t *testing.T) { }, }, expectedUpgraded: TableDescriptor{ - ID: 1, - Columns: []ColumnDescriptor{{ID: 1}, {ID: 2}}, + ID: 1, + CreateAsOfTime: ts, + ModificationTime: ts, + Columns: []ColumnDescriptor{{ID: 1}, {ID: 2}}, Indexes: []IndexDescriptor{ { ID: 1, @@ -1411,8 +1417,10 @@ func TestUpgradeDowngradeFKRepr(t *testing.T) { }, referenced: oldFormatUpgradedPair{ oldFormat: TableDescriptor{ - ID: 2, - Columns: []ColumnDescriptor{{ID: 2}}, + ID: 2, + CreateAsOfTime: ts, + ModificationTime: ts, + Columns: []ColumnDescriptor{{ID: 2}}, Indexes: []IndexDescriptor{ { ColumnIDs: ColumnIDs{2}, @@ -1427,8 +1435,10 @@ func TestUpgradeDowngradeFKRepr(t *testing.T) { }, }, expectedUpgraded: TableDescriptor{ - ID: 2, - Columns: []ColumnDescriptor{{ID: 2}}, + ID: 2, + CreateAsOfTime: ts, + ModificationTime: ts, + Columns: []ColumnDescriptor{{ID: 2}}, Indexes: []IndexDescriptor{ { ColumnIDs: ColumnIDs{2}, @@ -1455,8 +1465,10 @@ func TestUpgradeDowngradeFKRepr(t *testing.T) { name: "primaryKey", origin: oldFormatUpgradedPair{ oldFormat: TableDescriptor{ - ID: 1, - Columns: []ColumnDescriptor{{ID: 1}, {ID: 2}}, + ID: 1, + CreateAsOfTime: ts, + ModificationTime: ts, + Columns: []ColumnDescriptor{{ID: 1}, {ID: 2}}, PrimaryIndex: IndexDescriptor{ ID: 1, ColumnIDs: ColumnIDs{1}, @@ -1473,8 +1485,10 @@ func TestUpgradeDowngradeFKRepr(t *testing.T) { }, }, expectedUpgraded: TableDescriptor{ - ID: 1, - Columns: []ColumnDescriptor{{ID: 1}, {ID: 2}}, + ID: 1, + CreateAsOfTime: ts, + ModificationTime: ts, + Columns: []ColumnDescriptor{{ID: 1}, {ID: 2}}, PrimaryIndex: IndexDescriptor{ ID: 1, ColumnIDs: ColumnIDs{1}, @@ -1496,8 +1510,10 @@ func TestUpgradeDowngradeFKRepr(t *testing.T) { }, referenced: oldFormatUpgradedPair{ oldFormat: TableDescriptor{ - ID: 2, - Columns: []ColumnDescriptor{{ID: 2}}, + ID: 2, + CreateAsOfTime: ts, + ModificationTime: ts, + Columns: []ColumnDescriptor{{ID: 2}}, PrimaryIndex: IndexDescriptor{ ColumnIDs: ColumnIDs{2}, ID: 2, @@ -1510,8 +1526,10 @@ func TestUpgradeDowngradeFKRepr(t *testing.T) { }, }, expectedUpgraded: TableDescriptor{ - ID: 2, - Columns: []ColumnDescriptor{{ID: 2}}, + ID: 2, + CreateAsOfTime: ts, + ModificationTime: ts, + Columns: []ColumnDescriptor{{ID: 2}}, PrimaryIndex: IndexDescriptor{ ColumnIDs: ColumnIDs{2}, ID: 2, @@ -1536,8 +1554,10 @@ func TestUpgradeDowngradeFKRepr(t *testing.T) { name: "self-reference-cycle", origin: oldFormatUpgradedPair{ oldFormat: TableDescriptor{ - ID: 1, - Columns: []ColumnDescriptor{{ID: 1}, {ID: 2}}, + ID: 1, + CreateAsOfTime: ts, + ModificationTime: ts, + Columns: []ColumnDescriptor{{ID: 1}, {ID: 2}}, Indexes: []IndexDescriptor{ { ID: 1, @@ -1582,8 +1602,10 @@ func TestUpgradeDowngradeFKRepr(t *testing.T) { }, }, expectedUpgraded: TableDescriptor{ - ID: 1, - Columns: []ColumnDescriptor{{ID: 1}, {ID: 2}}, + ID: 1, + CreateAsOfTime: ts, + ModificationTime: ts, + Columns: []ColumnDescriptor{{ID: 1}, {ID: 2}}, Indexes: []IndexDescriptor{ { ID: 1, @@ -1656,8 +1678,10 @@ func TestUpgradeDowngradeFKRepr(t *testing.T) { origin: oldFormatUpgradedPair{ oldFormatWasAlreadyUpgraded: true, oldFormat: TableDescriptor{ - ID: 1, - Columns: []ColumnDescriptor{{ID: 1}, {ID: 2}}, + ID: 1, + CreateAsOfTime: ts, + ModificationTime: ts, + Columns: []ColumnDescriptor{{ID: 1}, {ID: 2}}, Indexes: []IndexDescriptor{ { ID: 1, @@ -1687,8 +1711,10 @@ func TestUpgradeDowngradeFKRepr(t *testing.T) { // Our referenced table is *not* upgraded. referenced: oldFormatUpgradedPair{ oldFormat: TableDescriptor{ - ID: 2, - Columns: []ColumnDescriptor{{ID: 2}}, + ID: 2, + CreateAsOfTime: ts, + ModificationTime: ts, + Columns: []ColumnDescriptor{{ID: 2}}, PrimaryIndex: IndexDescriptor{ ColumnIDs: ColumnIDs{2}, ID: 2, @@ -1701,8 +1727,10 @@ func TestUpgradeDowngradeFKRepr(t *testing.T) { }, }, expectedUpgraded: TableDescriptor{ - ID: 2, - Columns: []ColumnDescriptor{{ID: 2}}, + ID: 2, + CreateAsOfTime: ts, + ModificationTime: ts, + Columns: []ColumnDescriptor{{ID: 2}}, PrimaryIndex: IndexDescriptor{ ColumnIDs: ColumnIDs{2}, ID: 2, @@ -1729,8 +1757,10 @@ func TestUpgradeDowngradeFKRepr(t *testing.T) { // Origin table has *not* been upgraded. origin: oldFormatUpgradedPair{ oldFormat: TableDescriptor{ - ID: 1, - Columns: []ColumnDescriptor{{ID: 1}, {ID: 2}}, + ID: 1, + CreateAsOfTime: ts, + ModificationTime: ts, + Columns: []ColumnDescriptor{{ID: 1}, {ID: 2}}, Indexes: []IndexDescriptor{ { ID: 1, @@ -1749,8 +1779,10 @@ func TestUpgradeDowngradeFKRepr(t *testing.T) { }, }, expectedUpgraded: TableDescriptor{ - ID: 1, - Columns: []ColumnDescriptor{{ID: 1}, {ID: 2}}, + ID: 1, + CreateAsOfTime: ts, + ModificationTime: ts, + Columns: []ColumnDescriptor{{ID: 1}, {ID: 2}}, Indexes: []IndexDescriptor{ { ID: 1, @@ -1776,8 +1808,10 @@ func TestUpgradeDowngradeFKRepr(t *testing.T) { referenced: oldFormatUpgradedPair{ oldFormatWasAlreadyUpgraded: true, oldFormat: TableDescriptor{ - ID: 2, - Columns: []ColumnDescriptor{{ID: 2}}, + ID: 2, + CreateAsOfTime: ts, + ModificationTime: ts, + Columns: []ColumnDescriptor{{ID: 2}}, PrimaryIndex: IndexDescriptor{ ColumnIDs: ColumnIDs{2}, ID: 2, @@ -1815,9 +1849,16 @@ func TestUpgradeDowngradeFKRepr(t *testing.T) { tc.origin.oldFormat.Privileges = NewDefaultPrivilegeDescriptor() tc.referenced.expectedUpgraded.Privileges = NewDefaultPrivilegeDescriptor() tc.referenced.oldFormat.Privileges = NewDefaultPrivilegeDescriptor() + // Make sure that the table descriptors have initialized timestamps. + // They always will when being used in a schema change. + toDesc := func(tableDesc *TableDescriptor) *Descriptor { + desc := WrapDescriptor(tableDesc) + desc.Table(hlc.Timestamp{WallTime: 1}) + return desc + } txn := MapProtoGetter{Protos: map[interface{}]protoutil.Message{ - string(MakeDescMetadataKey(tc.origin.oldFormat.ID)): WrapDescriptor(&tc.origin.oldFormat), - string(MakeDescMetadataKey(tc.referenced.oldFormat.ID)): WrapDescriptor(&tc.referenced.oldFormat), + string(MakeDescMetadataKey(tc.origin.oldFormat.ID)): toDesc(&tc.origin.oldFormat), + string(MakeDescMetadataKey(tc.referenced.oldFormat.ID)): toDesc(&tc.referenced.oldFormat), }} tables := []oldFormatUpgradedPair{tc.origin, tc.referenced} diff --git a/pkg/sql/sqlbase/system.go b/pkg/sql/sqlbase/system.go index c765b2d58a9d..b0e4b3c70877 100644 --- a/pkg/sql/sqlbase/system.go +++ b/pkg/sql/sqlbase/system.go @@ -44,7 +44,7 @@ func SplitAtIDHook(id uint32, cfg *config.SystemConfig) bool { if dbDesc := desc.GetDatabase(); dbDesc != nil { return false } - if tableDesc := desc.GetTable(); tableDesc != nil { + if tableDesc := desc.Table(descVal.Timestamp); tableDesc != nil { if viewStr := tableDesc.GetViewQuery(); viewStr != "" { return false } diff --git a/pkg/sql/sqlbase/table.go b/pkg/sql/sqlbase/table.go index df8d376c28a2..2400464bb97a 100644 --- a/pkg/sql/sqlbase/table.go +++ b/pkg/sql/sqlbase/table.go @@ -481,6 +481,7 @@ func ConditionalGetTableDescFromTxn( return nil, errors.Wrapf(err, "decoding current table descriptor value for id: %d", expectation.ID) } + existing.Table(existingKV.Value.Timestamp) } wrapped := WrapDescriptor(expectation) if !existing.Equal(wrapped) { diff --git a/pkg/sql/sqlbase/testutils.go b/pkg/sql/sqlbase/testutils.go index 7af7e2f4db94..be68b37914ae 100644 --- a/pkg/sql/sqlbase/testutils.go +++ b/pkg/sql/sqlbase/testutils.go @@ -66,10 +66,11 @@ func GetTableDescriptor(kvDB *client.DB, database string, table string) *TableDe descKey := MakeDescMetadataKey(ID(gr.ValueInt())) desc := &Descriptor{} - if err := kvDB.GetProto(ctx, descKey, desc); err != nil || (*desc == Descriptor{}) { + ts, err := kvDB.GetProtoTs(ctx, descKey, desc) + if err != nil || (*desc == Descriptor{}) { log.Fatalf(ctx, "proto with id %d missing. err: %v", gr.ValueInt(), err) } - tableDesc := desc.GetTable() + tableDesc := desc.Table(ts) if tableDesc == nil { return nil } diff --git a/pkg/sql/table.go b/pkg/sql/table.go index 2363ee269aef..745f370ff546 100644 --- a/pkg/sql/table.go +++ b/pkg/sql/table.go @@ -13,6 +13,7 @@ package sql import ( "context" "fmt" + "sort" "strings" "github.com/cockroachdb/cockroach/pkg/internal/client" @@ -385,6 +386,36 @@ func (tc *TableCollection) getMutableTableVersionByID( return sqlbase.GetMutableTableDescFromID(ctx, txn, tableID) } +// releaseTableLeases releases the leases for the tables with ids in +// the passed slice. Errors are logged but ignored. +func (tc *TableCollection) releaseTableLeases(ctx context.Context, tables []IDVersion) { + // Sort the tables and leases to make it easy to find the leases to release. + leasedTables := tc.leasedTables + sort.Slice(tables, func(i, j int) bool { + return tables[i].id < tables[j].id + }) + sort.Slice(leasedTables, func(i, j int) bool { + return leasedTables[i].ID < leasedTables[j].ID + }) + + filteredLeases := leasedTables[:0] // will store the remaining leases + tablesToConsider := tables + shouldRelease := func(id sqlbase.ID) (found bool) { + for len(tablesToConsider) > 0 && tablesToConsider[0].id < id { + tablesToConsider = tablesToConsider[1:] + } + return len(tablesToConsider) > 0 && tablesToConsider[0].id == id + } + for _, l := range leasedTables { + if !shouldRelease(l.ID) { + filteredLeases = append(filteredLeases, l) + } else if err := tc.leaseMgr.Release(l); err != nil { + log.Warning(ctx, err) + } + } + tc.leasedTables = filteredLeases +} + func (tc *TableCollection) releaseLeases(ctx context.Context) { if len(tc.leasedTables) > 0 { log.VEventf(ctx, 2, "releasing %d tables", len(tc.leasedTables)) @@ -844,7 +875,7 @@ func (p *planner) writeTableDescToBatch( } } else { // Only increment the table descriptor version once in this transaction. - if err := tableDesc.MaybeIncrementVersion(ctx, p.txn); err != nil { + if err := tableDesc.MaybeIncrementVersion(ctx, p.txn, p.execCfg.Settings); err != nil { return err } diff --git a/pkg/sql/zone_config.go b/pkg/sql/zone_config.go index 439c18745a2b..4d9972af2bb3 100644 --- a/pkg/sql/zone_config.go +++ b/pkg/sql/zone_config.go @@ -78,7 +78,7 @@ func getZoneConfig( if err := descVal.GetProto(&desc); err != nil { return 0, nil, 0, nil, err } - if tableDesc := desc.GetTable(); tableDesc != nil { + if tableDesc := desc.Table(descVal.Timestamp); tableDesc != nil { // This is a table descriptor. Look up its parent database zone config. dbID, zone, _, _, err := getZoneConfig(uint32(tableDesc.ParentID), getKey, false /* getInheritedDefault */) if err != nil { @@ -122,7 +122,7 @@ func completeZoneConfig( if err := descVal.GetProto(&desc); err != nil { return err } - if tableDesc := desc.GetTable(); tableDesc != nil { + if tableDesc := desc.Table(descVal.Timestamp); tableDesc != nil { _, dbzone, _, _, err := getZoneConfig(uint32(tableDesc.ParentID), getKey, false /* getInheritedDefault */) if err != nil { return err diff --git a/pkg/storage/reports/reporter.go b/pkg/storage/reports/reporter.go index c1bb36347915..36630b3cfbf3 100644 --- a/pkg/storage/reports/reporter.go +++ b/pkg/storage/reports/reporter.go @@ -346,7 +346,7 @@ func visitAncestors( if err := descVal.GetProto(&desc); err != nil { return false, err } - tableDesc := desc.GetTable() + tableDesc := desc.Table(descVal.Timestamp) // If it's a database, the parent is the default zone. if tableDesc == nil { return visitDefaultZone(ctx, cfg, visitor), nil diff --git a/pkg/storage/reports/reporter_test.go b/pkg/storage/reports/reporter_test.go index 4825f4977889..0d0ff8a5b8a2 100644 --- a/pkg/storage/reports/reporter_test.go +++ b/pkg/storage/reports/reporter_test.go @@ -31,6 +31,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/testutils/keysutils" "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" "github.com/cockroachdb/cockroach/pkg/util/encoding" + "github.com/cockroachdb/cockroach/pkg/util/hlc" "github.com/cockroachdb/cockroach/pkg/util/leaktest" "github.com/cockroachdb/cockroach/pkg/util/uuid" "github.com/cockroachdb/errors" @@ -508,7 +509,10 @@ func processTestCase( // And we're going to use keys in user space, otherwise there's special cases // in the zone config lookup that we bump into. objectCounter := keys.MinUserDescID - var sysCfgBuilder systemConfigBuilder + sysCfgBuilder := systemConfigBuilder{ + // Use a non-zero timestamp for descriptor creation/modification time. + ts: hlc.Timestamp{WallTime: 1}, + } sysCfgBuilder.setDefaultZoneConfig(tc.defaultZone.toZoneConfig()) objects["default"] = MakeZoneKey(keys.RootNamespaceID, NoSubzone) // Assign ids to databases, table, indexes; create descriptors and populate @@ -726,6 +730,9 @@ func addIndexSubzones( type systemConfigBuilder struct { kv []roachpb.KeyValue defaultZoneConfig *config.ZoneConfig + + // ts is used for the creation time of synthesized descriptors + ts hlc.Timestamp } func (b *systemConfigBuilder) setDefaultZoneConfig(cfg config.ZoneConfig) { @@ -758,35 +765,36 @@ func (b *systemConfigBuilder) build() *config.SystemConfig { } // addTableDesc adds a table descriptor to the SystemConfig. -func (b *systemConfigBuilder) addTableDesc(id int, desc sqlbase.TableDescriptor) { - if desc.ParentID == 0 { - panic(fmt.Sprintf("parent not set for table %q", desc.Name)) +func (b *systemConfigBuilder) addTableDesc(id int, tableDesc sqlbase.TableDescriptor) { + if tableDesc.ParentID == 0 { + panic(fmt.Sprintf("parent not set for table %q", tableDesc.Name)) } // Write the table to the SystemConfig, in the descriptors table. k := sqlbase.MakeDescMetadataKey(sqlbase.ID(id)) - dbDesc := &sqlbase.Descriptor{ + desc := &sqlbase.Descriptor{ Union: &sqlbase.Descriptor_Table{ - Table: &desc, + Table: &tableDesc, }, } + desc.Table(b.ts) var v roachpb.Value - if err := v.SetProto(dbDesc); err != nil { + if err := v.SetProto(desc); err != nil { panic(err) } b.kv = append(b.kv, roachpb.KeyValue{Key: k, Value: v}) } // addTableDesc adds a database descriptor to the SystemConfig. -func (b *systemConfigBuilder) addDBDesc(id int, desc sqlbase.DatabaseDescriptor) { +func (b *systemConfigBuilder) addDBDesc(id int, dbDesc sqlbase.DatabaseDescriptor) { // Write the table to the SystemConfig, in the descriptors table. k := sqlbase.MakeDescMetadataKey(sqlbase.ID(id)) - dbDesc := &sqlbase.Descriptor{ + desc := &sqlbase.Descriptor{ Union: &sqlbase.Descriptor_Database{ - Database: &desc, + Database: &dbDesc, }, } var v roachpb.Value - if err := v.SetProto(dbDesc); err != nil { + if err := v.SetProto(desc); err != nil { panic(err) } b.kv = append(b.kv, roachpb.KeyValue{Key: k, Value: v}) From 28aabd62c1e6550af9f5648214d434e5ee0a9c6a Mon Sep 17 00:00:00 2001 From: Andrew Werner Date: Wed, 11 Sep 2019 10:11:57 -0400 Subject: [PATCH 2/3] roachlint: add linter to prevent calls to sqlbase.Descriptor.GetTable() Callers should use sqlbase.Descriptor.Table() instead. Release justification: This is a linter for the first commit which is an approved change. Release note: None --- pkg/cmd/roachlint/main.go | 2 + .../descriptormarshal/descriptor_marshal.go | 134 ++++++++++++++++++ .../descriptormarshal_test.go | 27 ++++ .../descriptormarshal/testdata/src/a/a.go | 18 +++ .../pkg/sql/sqlbase/test_descriptor_lint.go | 19 +++ 5 files changed, 200 insertions(+) create mode 100644 pkg/testutils/lint/passes/descriptormarshal/descriptor_marshal.go create mode 100644 pkg/testutils/lint/passes/descriptormarshal/descriptormarshal_test.go create mode 100644 pkg/testutils/lint/passes/descriptormarshal/testdata/src/a/a.go create mode 100644 pkg/testutils/lint/passes/descriptormarshal/testdata/src/github.com/cockroachdb/cockroach/pkg/sql/sqlbase/test_descriptor_lint.go diff --git a/pkg/cmd/roachlint/main.go b/pkg/cmd/roachlint/main.go index 4281ca3f0d91..224c00e78c58 100644 --- a/pkg/cmd/roachlint/main.go +++ b/pkg/cmd/roachlint/main.go @@ -11,6 +11,7 @@ package main import ( + "github.com/cockroachdb/cockroach/pkg/testutils/lint/passes/descriptormarshal" "github.com/cockroachdb/cockroach/pkg/testutils/lint/passes/hash" "github.com/cockroachdb/cockroach/pkg/testutils/lint/passes/timer" "github.com/cockroachdb/cockroach/pkg/testutils/lint/passes/unconvert" @@ -22,5 +23,6 @@ func main() { hash.Analyzer, timer.Analyzer, unconvert.Analyzer, + descriptormarshal.Analyzer, ) } diff --git a/pkg/testutils/lint/passes/descriptormarshal/descriptor_marshal.go b/pkg/testutils/lint/passes/descriptormarshal/descriptor_marshal.go new file mode 100644 index 000000000000..08b649abbb3c --- /dev/null +++ b/pkg/testutils/lint/passes/descriptormarshal/descriptor_marshal.go @@ -0,0 +1,134 @@ +// Copyright 2019 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 descriptormarshal defines an suite of Analyzers that +// detects correct setting of timestamps when unmarshaling table +// descriptors. +package descriptormarshal + +import ( + "fmt" + "go/ast" + "go/types" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/astutil" + "golang.org/x/tools/go/ast/inspector" +) + +// Doc documents this pass. +const Doc = `check for correct unmarshaling of sqlbase descriptors` + +// TODO(ajwerner): write an Analyzer which determines whether a function passes +// a pointer to a struct which contains a sqlbase.Descriptor to a function +// which will pass that pointer to protoutil.Unmarshal and verify that said +// function also calls Descriptor.Table(). + +const sqlbasePkg = "github.com/cockroachdb/cockroach/pkg/sql/sqlbase" + +// Analyzer is a linter that ensures there are no calls to +// sqlbase.Descriptor.GetTable() except where appropriate. +var Analyzer = &analysis.Analyzer{ + Name: "descriptormarshal", + Doc: Doc, + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: func(pass *analysis.Pass) (interface{}, error) { + inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + inspect.Preorder([]ast.Node{ + (*ast.CallExpr)(nil), + }, func(n ast.Node) { + call := n.(*ast.CallExpr) + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + return + } + obj, ok := pass.TypesInfo.Uses[sel.Sel] + if !ok { + return + } + f, ok := obj.(*types.Func) + if !ok { + return + } + if f.Pkg() == nil || f.Pkg().Path() != sqlbasePkg || f.Name() != "GetTable" { + return + } + if !isMethodForNamedType(f, "Descriptor") { + return + } + containing := findContainingFunc(pass, n) + if isAllowed(containing) { + return + } + pass.Report(analysis.Diagnostic{ + Pos: n.Pos(), + Message: fmt.Sprintf("Illegal call to Descriptor.GetTable() in %s, see Descriptor.Table()", + containing.Name()), + }) + }) + return nil, nil + }, +} + +var allowedFunctions = []string{ + "(*github.com/cockroachdb/cockroach/pkg/sql/sqlbase.Descriptor).Table", + "github.com/cockroachdb/cockroach/pkg/sql/sqlbase.TestDefaultExprNil", + "github.com/cockroachdb/cockroach/pkg/sql/sqlbase.TestKeysPerRow", +} + +func isAllowed(obj *types.Func) bool { + str := obj.FullName() + for _, allowed := range allowedFunctions { + if allowed == str { + return true + } + } + return false +} + +func findContainingFile(pass *analysis.Pass, n ast.Node) *ast.File { + fPos := pass.Fset.File(n.Pos()) + for _, f := range pass.Files { + if pass.Fset.File(f.Pos()) == fPos { + return f + } + } + panic(fmt.Errorf("cannot file file for %v", n)) +} + +func findContainingFunc(pass *analysis.Pass, n ast.Node) *types.Func { + stack, _ := astutil.PathEnclosingInterval(findContainingFile(pass, n), n.Pos(), n.End()) + for i := len(stack) - 1; i >= 0; i-- { + // If we stumble upon a func decl or func lit then we're in an interesting spot + funcDecl, ok := stack[i].(*ast.FuncDecl) + if !ok { + continue + } + return pass.TypesInfo.ObjectOf(funcDecl.Name).(*types.Func) + } + return nil +} + +func isMethodForNamedType(f *types.Func, name string) bool { + sig := f.Type().(*types.Signature) + recv := sig.Recv() + if recv == nil { // not a method + return false + } + switch recv := recv.Type().(type) { + case *types.Named: + return recv.Obj().Name() == name + case *types.Pointer: + named, ok := recv.Elem().(*types.Named) + return ok && named.Obj().Name() == name + } + return false +} diff --git a/pkg/testutils/lint/passes/descriptormarshal/descriptormarshal_test.go b/pkg/testutils/lint/passes/descriptormarshal/descriptormarshal_test.go new file mode 100644 index 000000000000..1bb4a44dfbb8 --- /dev/null +++ b/pkg/testutils/lint/passes/descriptormarshal/descriptormarshal_test.go @@ -0,0 +1,27 @@ +// Copyright 2019 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 descriptormarshal_test + +import ( + "testing" + + "github.com/cockroachdb/cockroach/pkg/testutils" + "github.com/cockroachdb/cockroach/pkg/testutils/lint/passes/descriptormarshal" + "golang.org/x/tools/go/analysis/analysistest" +) + +func Test(t *testing.T) { + if testutils.NightlyStress() { + t.Skip("Go cache files don't work under stress") + } + testdata := analysistest.TestData() + analysistest.Run(t, testdata, descriptormarshal.Analyzer, "a") +} diff --git a/pkg/testutils/lint/passes/descriptormarshal/testdata/src/a/a.go b/pkg/testutils/lint/passes/descriptormarshal/testdata/src/a/a.go new file mode 100644 index 000000000000..c62a115dbba3 --- /dev/null +++ b/pkg/testutils/lint/passes/descriptormarshal/testdata/src/a/a.go @@ -0,0 +1,18 @@ +// Copyright 2019 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 a + +import "github.com/cockroachdb/cockroach/pkg/sql/sqlbase" + +func F() { + var d sqlbase.Descriptor + d.GetTable() // want `Illegal call to Descriptor.GetTable\(\) in F, see Descriptor.Table\(\)` +} diff --git a/pkg/testutils/lint/passes/descriptormarshal/testdata/src/github.com/cockroachdb/cockroach/pkg/sql/sqlbase/test_descriptor_lint.go b/pkg/testutils/lint/passes/descriptormarshal/testdata/src/github.com/cockroachdb/cockroach/pkg/sql/sqlbase/test_descriptor_lint.go new file mode 100644 index 000000000000..beb266225519 --- /dev/null +++ b/pkg/testutils/lint/passes/descriptormarshal/testdata/src/github.com/cockroachdb/cockroach/pkg/sql/sqlbase/test_descriptor_lint.go @@ -0,0 +1,19 @@ +// Copyright 2019 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 sqlbase + +type Descriptor struct{} + +type TableDescriptor struct{} + +func (m *Descriptor) GetTable() *TableDescriptor { + return nil +} From 05a000a4002e9c93e8b9933c9d341eef09449207 Mon Sep 17 00:00:00 2001 From: Andrew Werner Date: Tue, 17 Sep 2019 09:53:10 -0400 Subject: [PATCH 3/3] sqlbase,backupccl: properly handle TableDescriptor timestamps for old versions This PR ensures that the now logic to set the TableDescriptor timestamps from MVCC timestamps works in the face of table descriptors created in previous versions. There are two main hazards which this commit addresses: 1) Tables created before 19.1 will not carry a CreateAsOfTimestamp. This field is not important to set on these tables as they certainly were not relying on this value for a CTAS. 2) Ensure that TableDescriptors from a backup contain a non-zero ModificationTime. Before 19.1 we created Version 1 of tables with a zero ModificationTime. When restoring such a table we set its ModificationTime to hlc.Timestamp{WallTime: 1}. This prevents any invariants from being violated. This commit also adds a test to backupccl which restores a database from all previous major versions to ensure that the restore works properly. Release Justification: Fixes a bug in a PR which was approved for release and adds testing. Release note: None --- pkg/ccl/backupccl/backup.go | 17 ++++ .../backupccl/restore_old_versions_test.go | 86 ++++++++++++++++++ .../testdata/restore_old_versions/create.sql | 35 +++++++ .../exports/v1.0.7/486711033700122625.sst | Bin 0 -> 945 bytes .../exports/v1.0.7/486711033700253697.sst | Bin 0 -> 945 bytes .../exports/v1.0.7/486711033700483073.sst | Bin 0 -> 2942 bytes .../exports/v1.0.7/486711033700843521.sst | Bin 0 -> 936 bytes .../exports/v1.0.7/486711033757466625.sst | Bin 0 -> 986 bytes .../exports/v1.0.7/BACKUP | Bin 0 -> 1547 bytes .../exports/v1.1.9/486711187401211905.sst | Bin 0 -> 945 bytes .../exports/v1.1.9/486711187490635777.sst | Bin 0 -> 945 bytes .../exports/v1.1.9/486711187725058049.sst | Bin 0 -> 936 bytes .../exports/v1.1.9/486711187746521089.sst | Bin 0 -> 986 bytes .../exports/v1.1.9/486711187748749313.sst | Bin 0 -> 3257 bytes .../exports/v1.1.9/BACKUP | Bin 0 -> 2116 bytes .../exports/v1.1.9/BACKUP-CHECKPOINT | Bin 0 -> 1666 bytes .../exports/v19.1.4/486711494153863169.sst | Bin 0 -> 846 bytes .../exports/v19.1.4/486711494249414657.sst | Bin 0 -> 846 bytes .../exports/v19.1.4/486711494320259073.sst | Bin 0 -> 837 bytes .../exports/v19.1.4/486711494785302529.sst | Bin 0 -> 889 bytes .../exports/v19.1.4/BACKUP | Bin 0 -> 1730 bytes .../exports/v2.0.7/486711288918671361.sst | Bin 0 -> 1045 bytes .../exports/v2.0.7/486711288998658049.sst | Bin 0 -> 1005 bytes .../exports/v2.0.7/486711289118064641.sst | Bin 0 -> 996 bytes .../exports/v2.0.7/486711289209061377.sst | Bin 0 -> 1005 bytes .../exports/v2.0.7/BACKUP | Bin 0 -> 1700 bytes .../exports/v2.1.8/486711390409097217.sst | Bin 0 -> 1124 bytes .../exports/v2.1.8/486711390490296321.sst | Bin 0 -> 1115 bytes .../exports/v2.1.8/486711390560256001.sst | Bin 0 -> 1124 bytes .../exports/v2.1.8/486711390640865281.sst | Bin 0 -> 1165 bytes .../exports/v2.1.8/BACKUP | Bin 0 -> 1702 bytes pkg/sql/sqlbase/structured.go | 1 + .../descriptormarshal/descriptor_marshal.go | 1 + 33 files changed, 140 insertions(+) create mode 100644 pkg/ccl/backupccl/restore_old_versions_test.go create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/create.sql create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/exports/v1.0.7/486711033700122625.sst create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/exports/v1.0.7/486711033700253697.sst create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/exports/v1.0.7/486711033700483073.sst create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/exports/v1.0.7/486711033700843521.sst create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/exports/v1.0.7/486711033757466625.sst create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/exports/v1.0.7/BACKUP create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/exports/v1.1.9/486711187401211905.sst create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/exports/v1.1.9/486711187490635777.sst create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/exports/v1.1.9/486711187725058049.sst create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/exports/v1.1.9/486711187746521089.sst create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/exports/v1.1.9/486711187748749313.sst create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/exports/v1.1.9/BACKUP create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/exports/v1.1.9/BACKUP-CHECKPOINT create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/exports/v19.1.4/486711494153863169.sst create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/exports/v19.1.4/486711494249414657.sst create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/exports/v19.1.4/486711494320259073.sst create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/exports/v19.1.4/486711494785302529.sst create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/exports/v19.1.4/BACKUP create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/exports/v2.0.7/486711288918671361.sst create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/exports/v2.0.7/486711288998658049.sst create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/exports/v2.0.7/486711289118064641.sst create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/exports/v2.0.7/486711289209061377.sst create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/exports/v2.0.7/BACKUP create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/exports/v2.1.8/486711390409097217.sst create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/exports/v2.1.8/486711390490296321.sst create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/exports/v2.1.8/486711390560256001.sst create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/exports/v2.1.8/486711390640865281.sst create mode 100644 pkg/ccl/backupccl/testdata/restore_old_versions/exports/v2.1.8/BACKUP diff --git a/pkg/ccl/backupccl/backup.go b/pkg/ccl/backupccl/backup.go index d2fb4dfdfb23..832d7caf8c95 100644 --- a/pkg/ccl/backupccl/backup.go +++ b/pkg/ccl/backupccl/backup.go @@ -117,6 +117,23 @@ func readBackupDescriptor( if err := protoutil.Unmarshal(descBytes, &backupDesc); err != nil { return BackupDescriptor{}, err } + for _, d := range backupDesc.Descriptors { + // Calls to GetTable are generally frowned upon. + // This specific call exists to provide backwards compatibility with + // backups created prior to version 19.1. Starting in v19.1 the + // ModificationTime is always written in backups for all versions + // of table descriptors. In earlier cockroach versions only later + // table descriptor versions contain a non-empty ModificationTime. + // Later versions of CockroachDB use the MVCC timestamp to fill in + // the ModificationTime for table descriptors. When performing a restore + // we no longer have access to that MVCC timestamp but we can set it + // to a value we know will be safe. + if t := d.GetTable(); t == nil { + continue + } else if t.Version == 1 && t.ModificationTime.IsEmpty() { + t.ModificationTime = hlc.Timestamp{WallTime: 1} + } + } return backupDesc, err } diff --git a/pkg/ccl/backupccl/restore_old_versions_test.go b/pkg/ccl/backupccl/restore_old_versions_test.go new file mode 100644 index 000000000000..8bffc0432443 --- /dev/null +++ b/pkg/ccl/backupccl/restore_old_versions_test.go @@ -0,0 +1,86 @@ +// Copyright 2019 The Cockroach Authors. +// +// Licensed as a CockroachDB Enterprise file under the Cockroach Community +// License (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// https://github.com/cockroachdb/cockroach/blob/master/licenses/CCL.txt + +package backupccl_test + +import ( + "io/ioutil" + "os" + "path/filepath" + "testing" + + "github.com/cockroachdb/cockroach/pkg/base" + "github.com/cockroachdb/cockroach/pkg/util/leaktest" + "github.com/stretchr/testify/require" +) + +// TestRestoreOldVersions ensures that we can successfully restore tables +// and databases exported by old version. +// +// The files being restored live in testdata and are all made from the same +// input SQL which lives in /create.sql. +// +// The SSTs were created via the following commands: +// +// VERSION=... +// roachprod wipe local +// roachprod stage local release ${VERSION} +// roachprod start local +// # If the version is v1.0.7 then you need to enable enterprise with the +// # enterprise.enabled cluster setting. +// roachprod sql local:1 -- -e "$(cat pkg/ccl/backupccl/testdata/restore_old_versions/create.sql)" +// # Create an S3 bucket to store the backup. +// roachprod sql local:1 -- -e "BACKUP DATABASE test TO 's3:///${VERSION}?AWS_ACCESS_KEY_ID=<...>&AWS_SECRET_ACCESS_KEY=<...>'" +// # Then download the backup from s3 and plop the files into the appropriate +// # testdata directory. +// +func TestRestoreOldVersions(t *testing.T) { + defer leaktest.AfterTest(t)() + const ( + testdataBase = "testdata/restore_old_versions" + exportDirs = testdataBase + "/exports" + ) + dirs, err := ioutil.ReadDir(exportDirs) + require.NoError(t, err) + for _, dir := range dirs { + require.True(t, dir.IsDir()) + exportDir, err := filepath.Abs(filepath.Join(exportDirs, dir.Name())) + require.NoError(t, err) + t.Run(dir.Name(), restoreOldVersionTest(exportDir)) + } +} + +func restoreOldVersionTest(exportDir string) func(t *testing.T) { + return func(t *testing.T) { + params := base.TestServerArgs{} + const numAccounts = 1000 + _, _, sqlDB, dir, cleanup := backupRestoreTestSetupWithParams(t, singleNode, numAccounts, + initNone, base.TestClusterArgs{ServerArgs: params}) + defer cleanup() + err := os.Symlink(exportDir, filepath.Join(dir, "foo")) + require.NoError(t, err) + sqlDB.Exec(t, `CREATE DATABASE test`) + var unused string + var importedRows int + sqlDB.QueryRow(t, `RESTORE test.* FROM $1`, localFoo).Scan( + &unused, &unused, &unused, &importedRows, &unused, &unused, &unused, + ) + const totalRows = 12 + if importedRows != totalRows { + t.Fatalf("expected %d rows, got %d", totalRows, importedRows) + } + results := [][]string{ + {"1", "1", "1"}, + {"2", "2", "2"}, + {"3", "3", "3"}, + } + sqlDB.CheckQueryResults(t, `SELECT * FROM test.t1 ORDER BY k`, results) + sqlDB.CheckQueryResults(t, `SELECT * FROM test.t2 ORDER BY k`, results) + sqlDB.CheckQueryResults(t, `SELECT * FROM test.t4 ORDER BY k`, results) + } +} diff --git a/pkg/ccl/backupccl/testdata/restore_old_versions/create.sql b/pkg/ccl/backupccl/testdata/restore_old_versions/create.sql new file mode 100644 index 000000000000..0ceca727bd5b --- /dev/null +++ b/pkg/ccl/backupccl/testdata/restore_old_versions/create.sql @@ -0,0 +1,35 @@ +-- The below SQL is used to create the data that is then exported with BACKUP +-- for use in the RestoreOldVersions test. Each of t1, t2, and t3 should contain +-- the same rows when ordered by k. + +CREATE DATABASE test; + +SET database = test; + +-- t1 gets some modifications. + +CREATE TABLE t1 (k INT8 PRIMARY KEY, v1 INT8); + +ALTER TABLE t1 ADD COLUMN v2 INT8; + +CREATE INDEX t1_v2 ON t1 (v2); + +-- t2 is an unmodified table. + +CREATE TABLE t2 (k INT8 PRIMARY KEY, v1 INT8, v2 INT8); + +-- t3 and t4 are interleaved. + +CREATE TABLE t3 (k INT8 PRIMARY KEY); + +CREATE TABLE t4 (k INT8 PRIMARY KEY, v1 INT8, v2 INT8, CONSTRAINT fk_t3 FOREIGN KEY (k) REFERENCES t3) + INTERLEAVE IN PARENT t3 (k); + +INSERT INTO t1 VALUES (1, 1, 1), (2, 2, 2), (3, 3, 3); + +INSERT INTO t2 VALUES (1, 1, 1), (2, 2, 2), (3, 3, 3); + +INSERT INTO t3 VALUES (1), (2), (3); + +INSERT INTO t4 VALUES (1, 1, 1), (2, 2, 2), (3, 3, 3); + diff --git a/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v1.0.7/486711033700122625.sst b/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v1.0.7/486711033700122625.sst new file mode 100644 index 0000000000000000000000000000000000000000..d138244ba5f1a8b699ac4bb23c360e06a5afc6b1 GIT binary patch literal 945 zcmaKrKWGzC9LImTrfCz9(j>+}5nED3ls@9%;3joYbg%J4 zE3ll_%H0YZbygSjh1Ge84Q6;X3i+wOMx{O1zD!#`<P(LoM2&LccIZ}bBTqymy8Gq2x%eQus* z@($#k`cymXg_>J(-Ew|(YZdAjqgVApEnCW>VT^_ie)#ZBp_VJ<0FPt&G5Pro*0M@^ zJG$K>EskGEi)z*;hPF5(zG$;PF@uoV99Vb$9~vsw@%yCXbHkJ`x^=j6LiB!5V)-35 zm?vC?>QYkPlROFp>CnLE1LC)T;aP@adF~HFs;H65!;zJwUa#rN#0I6@VxVGL`%>Hy zGz@u>2jF2Pb9r6Xq;tP8X*+vZV6gfg( zBA<{cQbftGBU{KWQpP;^{`g4)ikZJg#XUDaE?Gb3;Tct{P;n>2@gIMy*adLgKI6Ct ZpDT?Qxz+c%+RSyrm)crCi|;Wz#S9MJ#( literal 0 HcmV?d00001 diff --git a/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v1.0.7/486711033700483073.sst b/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v1.0.7/486711033700483073.sst new file mode 100644 index 0000000000000000000000000000000000000000..bf0e1070d4bce3620b041635b02f65e7022e7c87 GIT binary patch literal 2942 zcmaJ@4RBP|6+ZXMcJYgYVA={r<&bIM+1fr13B3Zp4C zttIsKHeAI-i3AF`Apos16G)vbaX4sp6exL&i$)~gQE?4UUM6(bFmoy$jjQQli73QN zbFUV>0=c3zKmuGdG(!g?nQ|8{RSsRSj=Qnw$ihv8Y$yCa0*8plxk3P!V>!6Q<;`y5 z=E`wUX0%t0hik&(?QOW3n=j|bi0~2t&T*XVTzLh#X8F*u*Le0SVJxC$deYI9o=huz z&`vDx0dc%18Fst;0VuNrEwhu)DBoG)e1d!?;c@};&}o2t=VAcyWgujNmNJP<2*I#( zKSei-o>tHdZjA8a8G4rNT6qSVw{BTo&$AB*6SEmDol%6~Uw|TIq|kGpc(&n0b1}2` ztC@Z+vP#WlhLY(>>jeOx^JnCOqrLM}pgGL4;$h)DzxaJ{I@=7FTP;&`A(Jgrt=pmT zUhta;Jwk3@X(G+L#_!wBv*&QLjmd>tcuR=GlIZ(Tt|+(L`THF$GTm$njsaywPw{ zaWHXEjb)qO;saim6H_z#>QqG4wdkKf44tZl3Neu!D)%9kPG%9mEnhR7(j;L|fl8^^ zS=Mm8rBHj~$$XBkVGBGtLLl6-2f|`Mgo{QkdE!QZ9SmHS-+;?M2H#!h^Lf7p%+7or@YOTYm3+2E2~Ttyp8GCpQsT;Jh0Ba9)LJbrss?Hoa)3 zUN%R~!=(1-l7`~VCDI|{W_D60)Pxt$t^3AF83z`yTv8&`hU-g-zlOLmHo_sIFh+br zUAQt(wn%Y{<>DAIX|41gVNc~`u$5o*%1=L>$Fto6qiHqKr+Fs977X!%lQ}5d=1wS%anZ&suPB9<0HRP(xel+36luhdWTDp_`z|j1=r4vZO@2L2W=bWXM z6hQe}3pKk;wg80+d;5GjoK6l!BO$Eid+i*Ur|evl@!w*n#mwfpADwKds%~t@cl55| zTh*=Ybj2#DUAqbd|0*sd&~lbX`p~Dwb`o4eTIAJLP~Tn!rWIAPsoXS1%%Li}jk{-M z6<3?{^WNupHboehjVkt_kCXEmKcEeVN_-D_E(8ymyf<0YkN@ZnmF-z-#pADZxX)^w zd*r)M-eCgSc{v7xEH1d8QWwkd%!lC7I|PY){Wjm0coeOx-v|Ev;29}?l+iQX-pcvb zEgwwOJ-y_zTbDjA^X$(Y6K+g)XOuZX4*_r9EjcHTwZGl*8292988b`A)2FjxnOw}w zJ!!4?gxM&MlsX(ssuArv-;GBf*!?-Rb}eCr_l+bj$N)4P$Lvu7Bi*JbMor)T@;mcd*?2@}xX+ z=S!1&r;biaxr_`*i(kMpwr^#^V0GC6-eG6X|!I%I=U@0wA0n$dwJzvbLrB}~+- z!-K)H^^_Y7`nnrk6!?lEo0Z!{_~IcO6g$<0$oYVa8>~hIeYN)W)`F!L`Yd_gcF}rk z-*-;e)hKTdJ#~a<2gyY#sMNZH6+QN;PvMqN0oM83?Z8#^SqG1S*U9K3!`c` zddD&4pC^;UJiDJ9lWUYZbfw@Zl@5M#&zXJQZxXtU>#R^M*B&5pC!_TeDK^i6?jn2l zD*$=*_mR!F7r({m8)V!zZoTU@VVh%I8LEBs6Q2EwjMG}B-Z5_7ck1vMOwvV}iR23k>3$bRA0hJqA7bDSlNLXeVx#XF$V9sR-7$Re zcjNIH@#|`LO!G$*5p9EC$0OUA0~O@!Ra2Ns(GC9IXiV4A*d_u{`G2&B`jfakR>YSR zszDKFmhVZ%vhjo;`*$=p?2kq+;$wz9`Ac~m?-?qd3O#6YI;r;bhkKGUV~}t9s$yEp z7$^VL2{n}(hFoq&hFwbaXQCUmy%H>-Go#uDv_GN7!kLU7MsGBKUo6>ed~53y$!Qvl zAI~gn|2yGQtKkg2{8Vhxs%0c7nqBcy zRME$@+Jkiajho-h!gFV-Or?!Y^%yAD|zDs z$eRt=C1cUkz0&)lp@0(s-afzI4Tb#99phhIkS`eJh8N^Z&!}X)On-hZoeNJnJP#L? T%?Q8#|GIx|+F6=i`tE-KXVDoX literal 0 HcmV?d00001 diff --git a/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v1.0.7/486711033700843521.sst b/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v1.0.7/486711033700843521.sst new file mode 100644 index 0000000000000000000000000000000000000000..2b6852d59e9cf14d71a578da5b66f3e62761b444 GIT binary patch literal 936 zcmaKr&ubGw6vy9YlhleJrfE} zv6Q9vl!yb#Iy~^jkooOjv}T}Eo!EnrYi^YGsAa9V*K2wzJA=9Mw4=r-@|COGkt_fQ)V}1R8AYZ^!yxMl)v|l+OO>i%AE$HC{tehbZR-`R-&|g8 zUzoHWz~a>LfYi1^2$WsZ16Y{Yln%II2l)Uhxn=)eZ0~1bVJ7Q6K;1>ALQIvBu@Irr zP|Kra9I$lg^r9nYsyk~!X|#`#3-vtX!KKoSntP-+lRUsI@yS9~IMlYN88W&zfz~m) zvj)|?u;!cTh_>NtQv=%b4K89#WjSsIJ5Q_aGTTjX2{?#hYx$?DS?Gy)a^nLsZYK5IqlKk;ASsSgZ_K`aF@9n-E Vmrw1M8}{h%b$jnY(ul8r{tbv$9vuJx literal 0 HcmV?d00001 diff --git a/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v1.0.7/486711033757466625.sst b/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v1.0.7/486711033757466625.sst new file mode 100644 index 0000000000000000000000000000000000000000..612dd79ed6ae1f6d64a102e219cc6670f19d6fa9 GIT binary patch literal 986 zcmaJ=O=}ZD7=Cw~Y)y!Q`D%k&l!P2w9O9vZmr(Jbc=6ytN-1HonIud1%h}mB(Thc_ zwn5N~_UKJ0;y-BcP`wnrcvBBOco)H2q4k|@b`vUf;Mv*P=ly)%ogL*C(3y! zv-|s1Zncn5Afc3Q+$k(1$MT7T{^_aI>#Kjx zpmmcG!)~w+5nY!_ADC87Hg!2`8XeL!Ou@Kew<>V?Kh`U42g5J1?$F^1Ovc0Tx?}p5 zO`6m)%q}q+hq%TeogMO)OF0z|*Rf>oP`zE#o$woEv{6IOJkN0KyEb**E}V_3YEVIm zXFOm&luv~Ytb^s-)U0`)s9`gVv`nWi&+NYK1S-hKrAV}Y9hMoF`BZ6_iLxCD6XXM! z9QoWqwWSayvsCeFpO5VeJ;L* zL6dLz?ts_=aSy63MyAGQmgW#!((QxzPI6w`xaI7R-{!I_$eTq^L)bt5L+N|glvnc zxrvz>+!hxZ@g0$0B{qJEGoKRtdGb!~QU}8|>c3$mgrS*Pco zZM77r+Mq6N`|-I7_X9`mi_wCaDOzGeB3vvbsl_D%Mp9f{99%3#`S~RROj-{ax$ZG? zF_jof7%LcQup2N~@o;oaJoIBaB*i%iaxrELFiNp;FbXh8Feoq>FgOWuF_jq#Fu}zb zp<+e?%*bLMEI~?K>;*-cxrs%U0*n%jN{rbCj1CN5T%d$41vE>8A&MbNnTxf=Fuu%4 zfJuTuiK)!UfY|~=Q37)olA9YCnL8MnXD~8sVw6Kzz#zq#Ed_KK5F05lDljQ9YcTC# zWIV*kptXgOYa_^yMiM4KKLVX=1ReT210Ug7631v;NKEjzx%7?^|@qd2(OKtd9XINgswl|cPs(649( zOqoJVTnth`>?QE_fA*GhYL0eo`TaILYurNv7^BL#*wgb3^(^#^h1kjr^$hgPrG$+P z3@!8xjPwnG`NzP@5QvPGG?I+W&C`s{QY=i2%#%$`jgwL>Q%#Lhl2Z)KEG;d~EfbAX vEfUi#%(OVulam$9^o;ckjJR_$^GYie5_40`Ostrloqe3xi&Aq^6N^&;w9Jl( literal 0 HcmV?d00001 diff --git a/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v1.1.9/486711187401211905.sst b/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v1.1.9/486711187401211905.sst new file mode 100644 index 0000000000000000000000000000000000000000..16ea38934022437eb0c4b5c108a3292d67d34129 GIT binary patch literal 945 zcmaKr&uO; zAVw2!UcC40trugA7muEd2mKHH1HjZbTXsQ>o#ey9`~AMNJh>0$+{s{Y1eZR3p1j%m zb((V^=hVj9SY0<*S)u1`Q)L+~r44Kjo|0Vixc?mbcy4S70rp zq}SEW25E5oLK;-FCNZ?Z8SzDv^@!<*%;vzl^Z(FLv6kN>EuR~ve9^AK#yQak9f{?; ztUpV*1(n65yeD}S2-2c~&-=u0p5j@8LTTm?LaL~d%EOVBq+X})$;1Z5?0leNTKiHw z6f_L`a50^#New01KV;i*?Lv~kdN{tIysovW<1&o2xop_qY){Bo1^aO|746@IeWq+b z*TxmoZfC;8{QxR+<{qkTh2TuIO$VSn^GOv@L#Fir3aRJpUhM2QpuCjK9x&BrM20L@ zMns&4M!|BLC8B`DOUDLRRG{ph3`T$atzs9zmVL%? aFD9MvU3T?rwmNm4^LR9VHd>4BeftfmnI0qn literal 0 HcmV?d00001 diff --git a/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v1.1.9/486711187490635777.sst b/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v1.1.9/486711187490635777.sst new file mode 100644 index 0000000000000000000000000000000000000000..1175190c2b16f931b6a9bf85381e5643f959fc1e GIT binary patch literal 945 zcmaKrKWGzC9LImTrfHK!nw&9Egi4Bt;v*d#+_VlBT?!6Tq=c8tFUi?=@2>A&(wviS zCK+^gadQZWi(>}|H%BLNP;e3j5hrOufA5;RM8!V%2GdZ%G~m5iF%sHM!<2bnoFNoLx*3uutPlF6v>Z>$D8Aw(uL~H||P5R>6FnNk#iN zp@+)!vrJqm?RF$g+z+5KWA2mMMhJnjtvdkasn1s)*KAS`pqP4&?#0Z04$2G3>>jE% zGC#mr8QB#g&;x?YEmB1A0|>TJ;xn7<-l>>@-?B5$`+s)%J*by=hTGfEVD{bn zpRexzI+%7~+G(;!AukqY%fsQF2AsO_oPTy2r6i%v!qXq(NfBr{mT%X0ThJ_N<*h|+ zm$`yon9Cz@L8SW4t$eiqfy)U>vXr z^W`kPr$ih`*5QFK2F!0C(3*m3ZEO!huDMa#qn5SeUa#e;>NC$a$SQ~Q#ORuq{Q4TG#Jlxz36E>)_6eVorl`!`?%wXIjMelvNs zLt)Z-0P_>aeNx*BAy9Tr4`6O=Q##;=9pwY4=9a^IvAth}xv8x80Cg9c3Ncki#zKTf zLp_g@alq1{(~FLriSDckrO`e{MQG$1k1myF)Z8PrndAXxiO&|Y!lAZB&4AIp3AB#U z$|}_I!kTZUBie>@u-T42co(iTF;G=!3mVtP+NJNsNfM#8+aU zsFLK@h-G4(s8Jle-g-fn;$N-OoJ)IUYo|DXlON(QP%rEZljM(|$=Yb$w2#!W+k10= VR5|yt(zHj%uZR1OlV*H-_c#3GAYcFh literal 0 HcmV?d00001 diff --git a/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v1.1.9/486711187746521089.sst b/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v1.1.9/486711187746521089.sst new file mode 100644 index 0000000000000000000000000000000000000000..2174e63b369a005c6f069e05c38c9d8c3328abbf GIT binary patch literal 986 zcmaJ=%WD%s82@&gY)yynT!B@1dsRkQJps})FHOuSN? z6(~P^@a1W;5X;1n=u4uy^KJo&>izfX*JMGk!d(ne>4<A@O~Vw78+NM<=l)~8)^;%b3hRy>&ck#(46i$; zZ`q_tEyL^*qj7+164KcbZ@H9H;cy*G<_^`{HQfonK}H)l{p+yGxXh0`UZ~iFl3JMeHHc zsQCrNB7!5b=m&3amr{_P_^VcI)ZdLu9{m9F74;s>s9U}M{$P0hOfkb&{QgO*XsB z&Lkw%_P|lZ@bFRPQ68rT6`?#-S_>%F19%RiSgWna(^6}A+4@32s2-r$yAh&2eat!E z-kp2@|K9)o-+9+N9Wvdk1_o9RfaCSIzTWu36BjH9Ea>5b(ib(XJ%gC^e%;WzZOBCk zCR`#GGi*M_aRseh*-#GWs~$Ua=ASINh=?Vsc58ZEZPO%1MqcED2^9wxJ1c0b|{BVZgKpj9*K(VWrS9$UqdcMu^VO>awt<3=nYv3?O* zh5)&TY)QGZ6hGvPfQv+Q9kWfj&KJ>j8{N)_ww?n>cUA)M#{i)roJVYEi1P=XJ26$$ zeL$jOF#8e9pT`%_>KW&u3caBcmVAN;-=k{@UE=(o0j5SnnVzTk3(DV6l}u6>s`^4L zG*i{}#j!-F;W9vhb3eN5Xl%L)IE7dm_j2di%1^=RtV!xzUBKY6L@Z#cSHgtNpb&(Q zqBS!Js@kq=E|$DTPg@d;YP!Vx37UHu$r3TO#iH%uj)rTn@dBFv514xq;VdFp&O3Uo zOO+wO`S?xi*l$D*}mfQtQ`4^WvH zjfJ%O8vi67x$?3I(fZ=ygleGip1QJ5kp?E;G_$-8S?`COv`)xzPl6n4XmF9><9zI3 z58|LJpn_T#%uk)bL z($fb@mKP$W0?E`p++HLRf(p2@U{PuQ zL`mko{C;H8=Hh3#wT+yAE<670-S<4flFb~!2{qcG1vs$!=ds*q;{1E{HH4|l_;E?) zt|+e|iN5JHKhbDq6Q(h-wIWl!SGmr~*+hsL3$;WOdNfJrWxf06jE5X!QDVn^d{m5KfiB~i0W>hkz z@Av)M!;%LX!t{itRR1_ebP%>n>#r>?-sZXL|BI>cMawelhjq0jTQq^zz`Xt+3(4Y# zTK4QSf#{q}J%K7S-o>;=qX}%;b42`xL>KnDs^Z z!n2tUTTVroe1DmIQCeWcVP%&#Mv=w%369)O@VE(gP*J{u#^MA$@=VnFmU zK6X56n7{(ta>|6Kky|{=1s|oFq(Ry%_J>kvI&{&r=;B;vt6cY$3Gd=J6|E6=R6V1X zt*Kfgd{w}bXLuqk)>?zQrl(3+(4}-)S#Djo7E!MV8Kx9*wnhas@;6ns9J-3B*cr!D z1{q}^qOoZyn!*K){PoSB``@tGAHag92aIP;j=UXriV3YZ)}^7qU`m=>qK&3Hm*)6% z(E02^Yg%@B5Ja^n6gKKlh5(m`%-mGVIv0Z{6PGdtLD2C+v)g%u^YH;k`DI)kfHDUk z5Ll{vg_rYjbphb+<67o0!Yhc(YHn7xQ=a0r?r!J)vgYum{AvhRc#u#^oso>XoCBIT9CclFTX}hYF5!*hV6T7q zL`;bWipnL1yN@k=$R;pL5u5#x4YHl;SQ?mJE~dAb!e3CGdjF`5YJv8hgGIcsO1nH* z#@lZ1TzeOrnn;xx!!MP}etTbFQ9l3#VQ7#HNy|nP{T|uLSUNg{qOiU=9lThnv_}TMlEYN3eIsijQv^-<$ z3CN>&q_!?S)T*>b=pUt(cAb!7sI3uA2}eU(pJLEXm*j&&^uI~t39UWcr?iJ7hL)i1 z)dxlYhkEhC7~P&$k?$FbLE>+2-xiDXbVn6>@d-!zm2l`=`k28ke!t#LuV^YQq-|8> zL`-d47;KB(oP%`J4+#^Ro@|)2qiQ_f4{6NJHQL-&MGr60-m}9Pd~;Uoqw+`9NKn^} zAk{`wIwG;w$jhV&65d}o>A0|BHTO!rZ9CoK`tq8-LU zdL#LMEtcq34dupuG&Dw5zH>}c_(I4q-;}*uqit1*#kFD6M0+BUxRHR&5w2)Yw=%5t zu%~tO1L=}Ww3{cwnw|#kksWV%mFk#K+o-vfdbJT|BH}Qs5OVWc=Gg^46sd+ht>Z(qy+$%%fOI@k@$7P<%7K$E<$0>GF z9HKZ$ag8FK8hrxAdi+hvczmIocUAii-XUd6x0l{5pF8}}l literal 0 HcmV?d00001 diff --git a/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v1.1.9/BACKUP b/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v1.1.9/BACKUP new file mode 100644 index 0000000000000000000000000000000000000000..28c7906e36aa4e9b42109c813746540d6165ffb3 GIT binary patch literal 2116 zcmchYe@q)?7{~9uun%77KHS32kZ z7)E0drkTb}21b-Wh>QgdF&Sx$$sZ5?IvBXse{mUw=}A4_`i=o_ZBe3qDB2f{j-lvSC~84b ztMYUZE#NwTLXsT18Q?ih_-2d6O*r4D0)UIx(-Y>qm8&i#POt zAOE1Zc=_~&PG@aX)+)ZS?&#F;t=5;!T@z~qzH@zhYtG9pdzZ8Mr+w1d>r0@6sE)IL_?h-7_z1k6d0CcSsP;x310Jfn~TN@pL#3zy)SZiwKN504!I6@r+<89 zX6n#^8+GTYX5%*nedCU}#JsF>@NEakdHNK6WHfzq*Ugs)+SXTbB$-JONDm9^1q-0k z6FDG!!J%~imps29C+i%x=*HN(^j^p&m+Aw#7ErL3zb==C)gMZZx%%{q}SnnY@k=e zcR#WEXy7^j)f)*t@>>4uIQn?W*w~()EtiT8m}lvktHTXvrIH5!K~2N5N1FLuf6Z?j ztM?aum^yGbU#OWXJyEcjIhL`8`PRMCo4Y3W$v%AZ&~E25ucph*sd#k$+=@-rUoB1- z{IaJa@7bGKtO5M>!Kw!vHNad>xAKnD!y3I?pl3Ze2I=+PBMtQ^~p*Qks%0niMH zHh}emq{v}RkB%ZgD7ykuZilCWtB6=d!Ybo5Zo~%wF#@pczX1FL!01L<^2)lXJ9zYv zRMK@0j})YeRZ-C}VTVKBA-PH&vb?L_>$(qFR>Yh_wshHj`Qo4^T<8Fy0!)qEf%d>{ zJ0Z+fFgb*IkCVH+sc=)Y$uP<%$v%ox^%l?m9l%)taUZMWY0emndKyo*EsP!gKD2`J zk|3y6wHCbysI{o!el`dmYB(B)aF(Npr7;L*3!=&l5fr5yp!VK6lxM(bA6wz)C|rjb za6iWo)_aG6Pz?j#1c=+ikQNcZ^q^Lc%%LAg0~)6ZrQtwn_y~Z*3Mr=(5UDb~@#JTh zKAQZ0N$;3w1hF*H4~M|Q5M2OvDiFNRj3DqJ#ECDbI(~Ray9)3!d!$15Z$TL(EvU`m z*UqW>_dvti$el{nmC+2FM9bo&Vp7mqmQ2oGnCp9($^ST0IT3Lc(4(XoK}5T2D||d! zAfTLtF-l4Y9!T-8%5FzXNsB|YX&_ceu`z>d3g=5s@7qqNCG=+(mV)xvVQf`BYqFYn zZH%8au_l{7iDOu64$I^)EY0$EPPCf^gAsj*Ej%aMcq=bBdACb)nJe8^XQfqixm<3B z5m-0J*+eN@Q&m|>TTHx(;o^2y*EPOIJ3KCnU?=kPi}T{V(oV@CORKBu8l4)lJClk> HD>?BWmFy4Z literal 0 HcmV?d00001 diff --git a/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v1.1.9/BACKUP-CHECKPOINT b/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v1.1.9/BACKUP-CHECKPOINT new file mode 100644 index 0000000000000000000000000000000000000000..a503311250f5b738d41eb85384c59a71512b7436 GIT binary patch literal 1666 zcmchXPiPc(6vuyWW|NG+1RXbN7>A<6Ds^4#W_~mKFWW=odI{Sin&TqG&CYC0wiuO>ot>8fqBKVIMQ87^Hp%mKpn_V+w6N9H77T&(! z{@(BVdGEWAMkFGWXqWWM>H7&G6F& z%q{r2D;Qmwj@2nC9lPUr#4p*NYAOS#zd7yH)(?*p3qyb{w9b zTzq=Y`tI9JU;Xj);klE4)gQ@I@oR(U@02^&7q*q)#u@1Q^jGcsA2;ne`}f{Ua|5G> zByE*Ng=i$9M0;twd&KqZnhYwX5{)>Gh9~QZyFl*%rIzhhoUwym!%;*H;-Hhf&s^xC z!kC43Bnf1q5FM-Or0`+*hAiNz)H=nRgW}L;+HuestCgK2GAIzk5;_c!BIRvVu|%Ad z$UfnVkdR4$DG=v?%tLo@nW%Ub^Wor0r5P2P2G&6Y@97C&>5DJ8Qc^4a6x&aWoYchCEKsxNpv1hh^{BSc$ueAqK_ zfj~*3?L4KxJ1O3)?Ri{EQGw#7p`<9Y_JvQ1uXf#=-KT9_JD(`-hSE{mG1_3sT+)oT zA7V+C>{q)Do#pzN-lsE-nRz3dPp4vW+~XP3$o88#Gc{sXE!#?0s=1L$E^ApPh@yJ#Hak zj`t)mp%NG{S7hKf&QH_Kz_u8$Z6O^Pq58}7O}f9Mv2_=UKjmw|^mwdgo+`G@Qy~i` zl3y6LC}^b_H{59xBbd=V-r0)vs8+8{7_e4owV`;H6=9tk;0|-7n za4~=Z(okG(S?jit6tKS3J)gr9r9EdK^R?<87{9fGSujaY>6cqb)Sv^@>*-5SeYZox zb5xs@J2o65!5lp`!Z!)aatp=}kVchyCQFSr-QzMxzbs2YB0*5GP9Pycqn3i{1JfCl zs#Pv!=1ed}Ri3g^OliBN^%@1`i~u`UpQ47VK3v6R;MH4#16n3vUCH#+jJewII?x8g zWAv0Mp283!nz`jcUBU1sB=}?da1&iz(YZ=Cd-9&q+3M(Mt(-5e-1LZW-=qJd1gh5Z-m1+)s=X;@E|eAd(|Q0jnU<(4mk7geV9^qChBG-;M1fd%MT(UgD#H zj+hj@1TO%hPDMe1Xh1Yb6o{8VDPZqKy3@_<%zQt4k8YAya5x(6ljge*3oqZl zS_~)&=#|$$)*$Ru8kNdo@M3>siL9jGpFa&d)kYO72e+2Uwe#n$j>67NV}@W*_v{b8 zd8Ac293A|%&Yff2b}!dQg3tL!GV4b=?lNmw%7-W56+e-age+IJigpZkbp?K~E;kS} z%ew+tB4ZdbTV&uiw&!W4p_dF6cqbSf@kO>*x!ReLEn* zDXMhJEgKIJze$gca7~F}xdH74NWDrOlMD40-Q^-jzYL2(AVH9kiXkR`-Ah6DfvF5i z)iM_%v)Z4dGEZ44rnKGAnnyueoq(CD%~8!|A1>iCaOw@g0VQHEwq$y2M$;Q#2il-{ zgq||VQy4>pr`H{*Eg0T}0C#L2Y@v&*DwpwASKQGmyFNMEDCdidYYq|aI`n_EJoNEW z_+H7wsb8&yWA!dO;|wqVzc-)0tdoVQ$*w^+X2(_j6Zemv K9W3WJzWxSO3IlQg literal 0 HcmV?d00001 diff --git a/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v19.1.4/486711494320259073.sst b/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v19.1.4/486711494320259073.sst new file mode 100644 index 0000000000000000000000000000000000000000..dd32ffd34cd560db687bb6b6dcc8ad960f668f9b GIT binary patch literal 837 zcmah{zi$&U6t?q&yQD`_3TdUHAchEm1Xh`l7#Ip|B*Z|W5(A*fclVN5_1TB*YZHV} z#ndJO1}4M=D}O^bHdvYX13*lSNFBhwBt;B}C*8@<{@#1v_u2FoS}Grm_V@Qt4H%sI3%_dsec>nTs5!t}*TP$pRM@xl+ z(Rln<69=%d>V6K0uI^qVXwHvR91^2RLI+193qBLG1hq;^MqApVLT(o%q#9ykXvl#u z5yOC(VG6F}@;puzblHG(88@Jq=&dZD#5;Xyp<6Kgvwhj0FGrGRi6C8?Fg_$K{$<0f z`lphVDr%&P>@X#1)bB)cwikHE6ll$)xGQLy4l4mxK#k3bVF@6=f@5kZ(d;p_{WCrc zEDb?+!6#4$rxhf*SrK&4~R@5S1mHm zQ=|Mk7Fj}aF}dv;2Ob7t6a!rh>R3_HgG;s=nA{3q@m?v7ho G`^#?t$ppUu literal 0 HcmV?d00001 diff --git a/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v19.1.4/486711494785302529.sst b/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v19.1.4/486711494785302529.sst new file mode 100644 index 0000000000000000000000000000000000000000..3af0543cf92ca87c764e6f5aa0b19d9b9d1d3467 GIT binary patch literal 889 zcmah{&1(}u6rYJ{ve|Ca+B5}wh?Eu&+Kv@O&_WPDTIoTgRy>H5+1;0P>FmrpGiee} zdTR}0y{iX{M{nK)TcH=hgNPULFYxX`P+Dg;X+4P#mYp~A-ur#b?BZ)^v~PQ3V;z-u zKMrp`Ki4T>RKSzxpDfI}M@9>KI|CK(>9dEM<;vMF@3zL>iA*Vz8R>hpK37FUvmbu= zu3tqnpX={mxf9t^)*N*fs>r+0|Ke7yiY|USef+CCkt^kp32eXO>o*ZPmf7Cu?Ctbm z?R*2<3u*sp_V(x0JnDA>8Tv#k64RC?nO0|C$fJ3S&QPF*9Iu!?6 zh6bl>St-42NCBxc;oBVUabY>z7|UnZK-#GplR5pkOFx~0+#+rny_P-$Ue_zAFo1=O zsV2Q9(oi6611e^*Wpb)Giq|M>7{3$=ff<6p0}(=qoMJu(SqCB#NLBL& zV~Li|Am)vjq+(Lr754HNxRw#9oHvLi<#m`g!@#On$TSERg3>9|t{LTg?>dYPk_N_8 z!f6aW2>0k!3#!}1+yn!AtnMrs7cYqh50`v)LyF|`!O>jW-=4Z+5n<7y|D)ut<7Fgm zraY|r$Jldw@@;z3I-Wkj>xW+rJ)k JCL8nL{s5&m5~2VA literal 0 HcmV?d00001 diff --git a/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v19.1.4/BACKUP b/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v19.1.4/BACKUP new file mode 100644 index 0000000000000000000000000000000000000000..9bd4a7820424eeb73710402aef898368b016b065 GIT binary patch literal 1730 zcmc(feQXnD9LJyMu3L`-1YaUFTyX&&H zA+z*st$F#nu3eA6efQzZK4ScP)9Lg3N3YL>4#=5vLvP->u7%eS_foh6xIxjcaGnxB zf#-OuowEt^e(!e8o(MH>k4Dx;#~aZeddxQvBf;)_E)0Em{r*iKf3~l= zCp_HuPJ`UAHZZqp;MkSXUdQ_L`rbXNpPnx9o2M@Ewn*N>a8)Qa(DX3 zF<1XT`R21N@w3TRK_%pidG0#&b;O{%JvZ=7v-37M>#U|v^mx&-^c1siC$s6k!I2mL z{9xeB&y81@T95MN%oNz&+WBqW)0X!qm6dJN*IrQl9;Pbud79SfR+E}!pgl>6Ocl9= ztWf;XP>jMzg4879@g#+rdNJ~KjKq?Nwz8s8>mZ4t(riL%bJ)0jOU%1z=#9Q3=$CXCaHR zxDgwM@FWD6Ajm-t0d!yvf(!;sKSA;$RHNZ(NsR`o(F_h9o&4XT*@Be?g*OKZ%7A_Y z$*=OD0&P=4F%|USwTbESpc^pHQwxbM)>jWj$m#^$&}r|#v|{gX22i))o9`qGjpd-O zLh_e9lt|0P?kx^=i-Y~Z!a+&`s3L4CZbdNMk`&bGR&PdOx(Tu}zd*xVAYFWB%TPNs zI`v+;U_I4&>Uh`LH{ANZFJ>pxuX$&mfKC07BwFLhf{;bhX;a8=)sG45)rZbX4BmUr4D@A?0)aP`QGXHjim)RbTaeU ztS>7X>PxKKEmpUMLTS5=GR*xadVR&C@~hb1PdEmn15tU) z6HUiub?(@r&FtE5T4~4?_IbzWk}FSIBw2dgTB%rG^bPW<{y>RAV0-Qbm-~JjhGSKh zM6QTn%66!~p_r&~#aUoqMFrD?Q6+3qUoIDwAW#+R5?ModRsD`vTrado{~V3o{5N3@ zr50l9xO}`@&#`bnfYH9sEgFM%NTAzLD*(lP&XW#TY_A?bE@pZI)7odDn2xkNC>zK; zA4ALBrVzf8FcN3EO^1a|hbJH~`qXI(T_tS{Qhgbos9~T&c|G^WMH6B! zcQD%Xuo-FcrPia`jAkuR2L{WRp%C+uuZQJzFPlbR^O{)WqKTnpGuq5q!X?56;VI!A z;VWT~PWd?D9AS+>b>Io%1>rs62cbaYm?fMhID}2YBSMk#;oIkDO~@tw(o9~c2k|We xTw)6_W^Q%5yW7#tr8`PDcW;HKBYe9yf8y7@^yKsObi^I}y!i7*cdB{j!yloMIcNX? literal 0 HcmV?d00001 diff --git a/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v2.0.7/486711288998658049.sst b/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v2.0.7/486711288998658049.sst new file mode 100644 index 0000000000000000000000000000000000000000..8f226ffdd97aa5665e2947caf45fbb324c66c412 GIT binary patch literal 1005 zcmaKr&ubGw6vy9gV$$ShlEy+QR8mx09O6G9;z310O9e$l!tCxOSv$M4&djEXc<>@L z4L$0`vj;_Q-Xb1UJb4jM;vb>lNs8#3O?ML&JMdX{=lz)PdvEsk1*n(C!{Ii}zx$B0 zUta5!EGSvcJ!P$y8+O&UYo(ow%g{U?ziE~m`Dz|FM!0c?Z{K@gZWO8oz-1Ypi;eHF zl9$5WRGvc|jxWTaig`p+hcn`b9_tf5h?t3iRqOw*kz{SZPuf1$O!{G`2`3Nm-s%cW zzsUyEfK#w=C{6DQ9tR<5)4=Bg;(Nbv6`@j_+JlHns)ck>vJ}+qwp@|!LA5aJD49~e z2(N}TiUu&Bsd6Qw8VyAI0oHdoGk8=}qWrsT9gfYVnOGm|3Mp?XrCV4fBONXr^Udyr zB599#9L>7k3Co&KN4W zza(QT)*oj-5uCoV@c`=fcsTmwmo)LXE|~|O1MuR}tB>Cb%MXhU^L6mLzIT7L5}*I_ E8>%-gs{jB1 literal 0 HcmV?d00001 diff --git a/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v2.0.7/486711289118064641.sst b/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v2.0.7/486711289118064641.sst new file mode 100644 index 0000000000000000000000000000000000000000..daee92306b661704a2847a900bcbe5883595b130 GIT binary patch literal 996 zcmaKr&ubGw6vy9Yo1_InO!K240SiIU;86bo!GjS*yeNu@#L4C*Svotj&Q8+Ai#-V1 zto0x0*@GU%TaOA}yeWcs_u`@8O+4tEO?ML&JMdX{=lz)PdvBIsg<5`py0h~HDsSJr zxz{)QD-NtU4fZhPZmyD_POsGA*oCM3lT%-cWBS;5{9Qb9ftF(_eq7&%#*$XvX4LJl zj-VIT;SqM3;T?g@mt7n)GYQegz**=2uA#=BKW05&7}UP(Z@{@Dytf95(r@Bq9&jGk z@@aZciD4jFj|aY(Fu(hYRuM|&xjhKE=0<6cBx}XJLEBU59;_A?9W_RgujF;f!*BwX zOqHjR8!|BC2UOpQ%+TeAv&esdEjYcLX2LPmCAnxvk!e$v$ofKc>^I(*Drt{>tY_W) zH(-R?3dvc&)$Fuc$CC2^))uzMGzQxtfwFG~0M)ro>3|z{m=Bm}(DD z_mQa(la|S$5TVgf%cA5kU`f%*4Tzitb=HK^XdfeYAwr2RMkbLr>s~!O?C>H``I>t) zAtrf%>6s^Usisg{J!Zma)&i|#bmuOVGfkRrCFyM~mlkTb`y(!fm~`y8?PZg=O-zZG z#0TPlSf)!}C$14AVwZS9ydgdlKZr6-qe--gfY>FT6IF_bd(Za1LCO6~GPR<$SFm=9 u2e{!jTTsjGPviKHAJN)qU9^wH@!c!`;A`PjTx{5_qt~sU+i_!f`O9yZq%9@@ literal 0 HcmV?d00001 diff --git a/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v2.0.7/486711289209061377.sst b/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v2.0.7/486711289209061377.sst new file mode 100644 index 0000000000000000000000000000000000000000..493c02a5045f0f1b28fa517464e6b43bbb1a016f GIT binary patch literal 1005 zcmaKr&ubGw6vtmSF~2-WH)$-CLQ9H>#UcI!A_^)BTJ#_y5@vTN$+ojI>+B>=Jb4jJ zL-FLvgC`O7;2&V|qDOCf5y6WWJ?S-w=$lP<6BRq~S$60BnD2XUcH=zMO1s11J(z#< zHk&>8=}RdCrA*_I%|DmxR>iWarR@vL&^S|lF)r70l^kx2aN|_*>)fGoJzvQKu8iUR z*tTIcr=+_Xc@A+nz7U5-%p;mQoDpAmtWWeHWF`iVW&ZCPD%SG*q~&wXlrP#1IKGef zR!3s`O*WVYoPdP`NqSfEZXigD20kAU-}{BD0Csh14??P_mdZuRN>aDebY-#!mHe!u zVo~HvaaGVT9Kd|4%2kYNG!X3vSl?V~@TjIF@^7;w7GQ5 zH@hWd+#d5-NxS)PK#wUS#4>)B^t4IG;`0C&X14n{2Gb$VL|b0a0c~oiQ25pdh~$ctT{7|(T$r>O*JXH6{k0~oSv!O?Dr_| zvbbZ5rk88Tbz~cPhP*?*Aw|68OUNaphwLCvk=MuvWQr0l-MC>%q!NiPd*Y}imE%s$@ zO-RZhx^?JCKv=S9P#i)0AR=*#dr=7nBMBc2&hZjnrY8H~3}S+FM&oI>&`LG}pZsu> zC;yyt|IhFLJO=?IqAnbsIek6*=~R`8G7%#^W@02)sB*aK%~zvX8!c#KxEjluW9art z;|fug6`mJVRnt^iQaMqxC6g(0?TgdT{53kWhJCLq>>wl!BCn@S&aO;&tFPFke$Y}cs^?j1*Ze4D;b zrO*96z_!dyjD*T>#l9Wxy>d~QIHtLtnCpdm4`RQ1TA$C;y4v^%(xn2)6fKyl&@xo! z@wbIzG=V6jOT^sR@3N-m&b)&Aa1{t;|!fl>JCk+{Bh7k_d0#^kEXkY>?u;4$cpc!Nc z3S=Q@wxO~V?`{`pf&pftUEoOdZs%LcZHt3tA=wSdeh5y$RmBc~2}Vq~d;AG3WC@nE z5W^6jf?yhgn@~pqBbbH2`XfZwAWlVKM4Sr7!>M!)4h;TJsk9PHQo?(q1dCCgh3K0+ zrJa#5B~hU4-aYp7(v%yC#^;J@EMI?}f}*bj;6>g_cEWZa_4X zrx6+XHqGqu**})1xz9~@6#FP4ucRbQS{97BQ+B-0cvtW`Q%Ike8F(L}@w$PM81{ud(Hd_!hF^Un)1k_4S*%INBC+^uv6{lw Unoun5HBdv4Ue1sSDC8jc7ic@3od5s; literal 0 HcmV?d00001 diff --git a/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v2.1.8/486711390409097217.sst b/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v2.1.8/486711390409097217.sst new file mode 100644 index 0000000000000000000000000000000000000000..b122e051931b2d133642ac6fa17447bb7ed6645b GIT binary patch literal 1124 zcmaKs&1zFY6vt1}k0dQfP1;xqVryCvF+&9(pj7aCQLxa3NSWMwnqE71<~lQJldjwd zjdh{*1;mAqAb!w=#g({JH$H&Nf^GyC^~`PWZK~LT-z7Qc%(?&hpGodrgK}Z3(^-d+ z51$5KZvPl6WT23lS`S*!2dddpHalE+bYl#r=G42l1J&MAF9kLzFnxNn@76%Iztj)3 zGLG!FHx7e&Py2Od4GVcI>De6(WE!35)yi8oR9+YKfz`P|pIKfP$O7qOi`jOBu0Nd5 z{J%8PxEQq9VjwK)KsKhJc0~2+lA`v@*gn{B5k~ux_MQ^WP_jiH2BOUZf0tGcio*x_ zFyfk9r9G0Y75A1FJe9D|?r84O0GZ?=CXy;= z63LNva&4Q@RS@V3&7C=#(~N3Lf37ZODStUhU5Puj@4|2@Q1jLJ_0Ec!F6&%;g^MP} zhF9Du%ESd?o_I*SBEAy4M1_7+mx$}c8u5a7Lwq52h&~$TBr#3Q6D!11;x+M$80rBy zNqwO9apxHnd;U5exjMU*FP literal 0 HcmV?d00001 diff --git a/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v2.1.8/486711390490296321.sst b/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v2.1.8/486711390490296321.sst new file mode 100644 index 0000000000000000000000000000000000000000..293bb1681412984ee3d6927c2fe4f43696e6f33a GIT binary patch literal 1115 zcmaKr&ubGw6vtnh#-voRwoO|p1(jG)>{!7+z#mvqJSY}=C{iZ7ugTKcnRRB;CZz`{ z2sQylJ&70bAc*&(U@ziLuk|A0!HYM+Q$eA=*><<7Vh29U?t5?Ee&;(g{2C1A)|x9T z_n`RxWA}@9KYO||(3L4|j6PnJtbM8o8wevq4DfJD4VXZaFi-B2K_)A?bv5 zT9@Qv#uz(8t3*~2s_ee;wp6h_?r|h_^WO>csGX1wSJ#*RZOgIve*i;m;W}-Db4Z}9 z*ebxlPEP5NTejT~pf}BIZJ5)30tWJl_7HUinTjyBOxA>mtcJmKmaK&=UUYl{Or}kp zOQEznK$C^xT{AS76&&+XpmWhcTSFy$G&WYhW$s9-W&50vJDPj6flTrc6G_8I6UmWw za&3drQ4r_~&GaPg(*;$N{$yEPr~JhvH4%4e+=Bj8pymtl@0}Hw+N^W&c`j-g8-BoT z#Q<@Nm?G{HtHdW_lNhEub(Xk7EE3O&H^e&egUHcl9wW{XQ^Y*+ka$gO5IuA;4^bcZ zyuKVwDG|sz6TzLBJ^wTf-!^?$|3++9o Me%)!6YL`F%0hjz+YybcN literal 0 HcmV?d00001 diff --git a/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v2.1.8/486711390560256001.sst b/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v2.1.8/486711390560256001.sst new file mode 100644 index 0000000000000000000000000000000000000000..a7062c34980cb04c8e06a0eca3fe3afd7161a5cb GIT binary patch literal 1124 zcmaKszfTlF6vy8kKaPV&@8HB-RN#OZA`?t(>j5bu3HYRFgVM0tSEi5##z&8uK92%MAv$=ck&D-yM-`w532BrLVyZs1; z-o6`nQTQ^I&pO&Omu!SAK6HEo zOr}GfOQEznK+^@~LpwB=b)5E5pmWhcM?)oiG&WYhXYQ#~%l~sJ203E)O;;|y|ZGj!#WpV<)VSH z;bk|95^;`LBpwj2h)={WQKsM21>!ofPCO&t5Fd%}L=O#ff|w;1iB;kW@tXKW9P0u& zNqykQ&hwv8==$q;oXb-$fN3@Q0ZQ5J_U55~0+*t7(>>z4t&R7mzvV`@`YJAT O^jiGAu{qwj`uPtjgH&Sx literal 0 HcmV?d00001 diff --git a/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v2.1.8/486711390640865281.sst b/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v2.1.8/486711390640865281.sst new file mode 100644 index 0000000000000000000000000000000000000000..61de417fd73cc8a1e819828097af760a94b3de33 GIT binary patch literal 1165 zcmaJ>O=uHA6n>jFX_|VlNgG>>^{26iYR8IRv`~tIKRpRXDvE^J-ATH1c6ObeZIhn# zAhebu;?08xMeyuF^kOUYqIf8xf_nGrK|w)ieY4G`5vc>;{>=B@n{U21yQ{`I$S0Q< z7UrRQ^=-$K#OK}Z2DBT66L%&?GcEb{^_9;4^!H+^yMN^U%SZb&1BqNB(UrVCF$OzF zU;i*O<`|5;tGs%g8EDP5BD09hj*}e^E|te%eC^2IPnm&KE(JIYjkR>{D|DnSX`4g^ z#HH0}U0k7g))BE9D6~q?nhr4BPuF3s-*02 z#iS!DBQX4*=-DX`%TKe~R>D!}Nygc znkrddJO{Z{bD(5F;CSMqpuS&&?uIH$GOCau%5iajoejdKiju&YV<8Nt<4UZG`x2Cw zf_=D!IuOllzs9hYmYw&_^39zajibQRm69pX$>)D%E| zi}R#Q6{*()$TpbGhH34GAm0{icbTj(;`uDHOoW{KO2V#2mIz%EZ8|yufzhN+Q%or7 zupk9}e`QE2Ds0$hoM{&WXe#J&n*}OTziIBChL-v{L3bpzuz^HSm&KC$4#bjuspYB~ z!K1*L4h*gqu}`PGP}oJ2UqJqJOr41;)viHTBF>j|HdQLLl!wcq{efUuwN1R8@As!-LAU+_zA(Gh5gNSj&Bw_|}7x5hN3(?sE(2I58 z@|$@D*_K#eeSc_cc&|nrge1HWASW7LPQ@6p@MpN0EN>3vVr RgE6=Lee(B>#X@-Y%OBd+WHtZ* literal 0 HcmV?d00001 diff --git a/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v2.1.8/BACKUP b/pkg/ccl/backupccl/testdata/restore_old_versions/exports/v2.1.8/BACKUP new file mode 100644 index 0000000000000000000000000000000000000000..58d9ac654567ee4aba0fd6e01acb43300897e57a GIT binary patch literal 1702 zcmc(eZERCj7{|}Kw_EQHSZ}(4H#N*l1)WP)&b__8FO4R95#0;wQe(`f+331d&YJjc;ax`F#fXbr`VA#qv8wN`XlJ1 zweIK@{HvJSqK~Zgy#%ifjeOCy`v?8(Z!`7d$Ah+8?U{|A9TW}Bqb%(N`5@sp*H5ya zhDA-bi^9BL=L6j@|8lAH;;G(G_#(?!qc^GT=TGz;-xu3g@Xgd)&rLO^vA&hL*YVMT zgIC)3U;Xy?j=!CITOR4!vGqc!Gkv&-_+@fQe)H5xehwLnQ=c=nt_JVvs=(ha)VN-ASRhDMUpC&cRDrMhu*Yx!C>r;FwI67#+rGV_9Ce11I4T9jr6 zqRBXo#HqJuwmi)}ZnATsk0kPRMj@mrzCdNfPS%<13R!0crN(6r-UH_HYq}V+(_kiV zC>Iv9_jOP&*2VG~cRpq+(Q7XsD~08UAYy95f=$3S#he-yZGug;q>EV4Heh7~R(U~n zih@%RtXV!$m3)Hi7DQ3qa>#p|}-yin*{2sr%E~m;LBJbYCZByk+S;}b$1w!G*P|n<^ VIW?h%DkF0mvMV`Shf*5^{{nnqsfhpp literal 0 HcmV?d00001 diff --git a/pkg/sql/sqlbase/structured.go b/pkg/sql/sqlbase/structured.go index 1c2cf155e54d..a9dca78aadd5 100644 --- a/pkg/sql/sqlbase/structured.go +++ b/pkg/sql/sqlbase/structured.go @@ -3315,6 +3315,7 @@ func (desc *TableDescriptor) maybeSetTimeFromMVCCTimestamp(ts hlc.Timestamp) { desc.Version == 1 { desc.CreateAsOfTime = ts } + // Ensure that if the table is in the process of being added and relies on // CreateAsOfTime that it is now set. if desc.Adding() && desc.IsAs() && desc.CreateAsOfTime.IsEmpty() { diff --git a/pkg/testutils/lint/passes/descriptormarshal/descriptor_marshal.go b/pkg/testutils/lint/passes/descriptormarshal/descriptor_marshal.go index 08b649abbb3c..4da350f2bc69 100644 --- a/pkg/testutils/lint/passes/descriptormarshal/descriptor_marshal.go +++ b/pkg/testutils/lint/passes/descriptormarshal/descriptor_marshal.go @@ -82,6 +82,7 @@ var allowedFunctions = []string{ "(*github.com/cockroachdb/cockroach/pkg/sql/sqlbase.Descriptor).Table", "github.com/cockroachdb/cockroach/pkg/sql/sqlbase.TestDefaultExprNil", "github.com/cockroachdb/cockroach/pkg/sql/sqlbase.TestKeysPerRow", + "github.com/cockroachdb/cockroach/pkg/ccl/backupccl.readBackupDescriptor", } func isAllowed(obj *types.Func) bool {