diff --git a/pkg/sql/crdb_internal.go b/pkg/sql/crdb_internal.go index 18ce4a62c102..3f0afd4f111a 100644 --- a/pkg/sql/crdb_internal.go +++ b/pkg/sql/crdb_internal.go @@ -84,6 +84,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/util/tracing/collector" "github.com/cockroachdb/cockroach/pkg/util/tracing/tracingpb" "github.com/cockroachdb/errors" + "github.com/cockroachdb/redact" ) // CrdbInternalName is the name of the crdb_internal schema. @@ -5011,7 +5012,8 @@ CREATE TABLE crdb_internal.invalid_objects ( database_name STRING, schema_name STRING, obj_name STRING, - error STRING + error STRING, + error_redactable STRING NOT VISIBLE )`, populate: func( ctx context.Context, p *planner, dbContext catalog.DatabaseDescriptor, addRow func(...tree.Datum) error, @@ -5061,6 +5063,7 @@ CREATE TABLE crdb_internal.invalid_objects ( tree.NewDString(scName), tree.NewDString(objName), tree.NewDString(validationError.Error()), + tree.NewDString(string(redact.Sprint(validationError))), ) } diff --git a/pkg/upgrade/upgrades/precondition_before_starting_an_upgrade.go b/pkg/upgrade/upgrades/precondition_before_starting_an_upgrade.go index 0cdbcd9a6241..ce63fdc2ac27 100644 --- a/pkg/upgrade/upgrades/precondition_before_starting_an_upgrade.go +++ b/pkg/upgrade/upgrades/precondition_before_starting_an_upgrade.go @@ -12,8 +12,6 @@ package upgrades import ( "context" - "fmt" - "strings" "github.com/cockroachdb/cockroach/pkg/clusterversion" "github.com/cockroachdb/cockroach/pkg/kv" @@ -25,6 +23,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/sqlutil" "github.com/cockroachdb/cockroach/pkg/upgrade" "github.com/cockroachdb/cockroach/pkg/util/log" + "github.com/cockroachdb/redact" ) func preconditionBeforeStartingAnUpgrade( @@ -48,10 +47,10 @@ func preconditionBeforeStartingAnUpgrade( func preconditionNoInvalidDescriptorsBeforeUpgrading( ctx context.Context, cs clusterversion.ClusterVersion, d upgrade.TenantDeps, ) error { - var errMsg strings.Builder + var errMsg redact.StringBuilder err := d.InternalExecutorFactory.DescsTxnWithExecutor(ctx, d.DB, d.SessionData, func(ctx context.Context, txn *kv.Txn, descriptors *descs.Collection, ie sqlutil.InternalExecutor) error { - query := `SELECT * FROM crdb_internal.invalid_objects` + query := `SELECT id, database_name, schema_name, obj_name, error_redactable FROM crdb_internal.invalid_objects` rows, err := ie.QueryIterator( ctx, "check-if-there-are-any-invalid-descriptors", txn /* txn */, query) if err != nil { @@ -68,13 +67,16 @@ func preconditionNoInvalidDescriptorsBeforeUpgrading( tree.Name(tree.MustBeDString(row[3])), ) tableID := descpb.ID(tree.MustBeDInt(row[0])) - errString := string(tree.MustBeDString(row[4])) + // Note: the cast from the `error` column to RedactableString here is only valid because + // the `error` column is populated via a RedactableString too. Conversion from string + // to RedactableString is not safe in general. + errString := redact.RedactableString(tree.MustBeDString(row[4])) // TODO(ssd): Remove in 23.1 once we are sure that the migration which fixes this corruption has run. - if veryLikelyKnownUserfileBreakage(ctx, txn, descriptors, tableID, errString) { - log.Infof(ctx, "ignoring invalid descriptor %v (%v) with error %q because it looks like known userfile-related corruption", - descName.String(), tableID, errString) + if veryLikelyKnownUserfileBreakage(ctx, txn, descriptors, tableID, errString.StripMarkers()) { + log.Infof(ctx, "ignoring invalid descriptor %v (%v) with error because it looks like known userfile-related corruption: %v", + &descName, tableID, errString) } else { - errMsg.WriteString(fmt.Sprintf("invalid descriptor: %v (%v) because %v\n", descName.String(), row[0], row[4])) + errMsg.Printf("invalid descriptor: %v (%d) because %v\n", &descName, tableID, errString) } } return err @@ -87,5 +89,5 @@ func preconditionNoInvalidDescriptorsBeforeUpgrading( } return pgerror.Newf(pgcode.ObjectNotInPrerequisiteState, "there exists invalid descriptors as listed below; fix these descriptors "+ - "before attempting to upgrade again:\n%v", errMsg.String()) + "before attempting to upgrade again:\n%v", errMsg) } diff --git a/pkg/upgrade/upgrades/precondition_before_starting_an_upgrade_external_test.go b/pkg/upgrade/upgrades/precondition_before_starting_an_upgrade_external_test.go index c24fc22c1e45..e029a782bd59 100644 --- a/pkg/upgrade/upgrades/precondition_before_starting_an_upgrade_external_test.go +++ b/pkg/upgrade/upgrades/precondition_before_starting_an_upgrade_external_test.go @@ -119,8 +119,8 @@ func TestPreconditionBeforeStartingAnUpgrade(t *testing.T) { require.Error(t, err, "upgrade should be refused because precondition is violated.") require.Equal(t, "pq: verifying precondition for version 22.1-2: "+ "there exists invalid descriptors as listed below; fix these descriptors before attempting to upgrade again:\n"+ - "invalid descriptor: defaultdb.public.t (104) because 'relation \"t\" (104): invalid depended-on-by relation back reference: referenced descriptor ID 53: referenced descriptor not found'\n"+ - "invalid descriptor: defaultdb.public.temp_tbl (104) because 'no matching name info found in non-dropped relation \"t\"'", + "invalid descriptor: defaultdb.public.t (104) because relation \"t\" (104): invalid depended-on-by relation back reference: referenced descriptor ID 53: referenced descriptor not found\n"+ + "invalid descriptor: defaultdb.public.temp_tbl (104) because no matching name info found in non-dropped relation \"t\"", strings.ReplaceAll(err.Error(), "1000022", "22")) // The cluster version should remain at `v0`. ts.tdb.CheckQueryResults(t, "SHOW CLUSTER SETTING version", [][]string{{v0.String()}})