this doesn't actually do anything, right?

Correct, I don't think it does anything. In fact, I think in the old schema changer, we didn't even set this field after validating a check constraint:

cockroach/pkg/sql/backfill.go

Lines 714 to 816 in 397d5da

	func (sc *SchemaChanger) validateConstraints(
	ctx context.Context, constraints []catalog.ConstraintToUpdate,
	) error {
	if lease.TestingTableLeasesAreDisabled() {
	return nil
	}
	log.Infof(ctx, "validating %d new constraints", len(constraints))
	_, err := sc.updateJobRunningStatus(ctx, RunningStatusValidation)
	if err != nil {
	return err
	}
	if fn := sc.testingKnobs.RunBeforeConstraintValidation; fn != nil {
	if err := fn(constraints); err != nil {
	return err
	}
	}
	readAsOf := sc.clock.Now()
	var tableDesc catalog.TableDescriptor
	if err := sc.fixedTimestampTxn(ctx, readAsOf, func(
	ctx context.Context, txn *kv.Txn, descriptors *descs.Collection,
	) error {
	flags := tree.ObjectLookupFlagsWithRequired()
	flags.AvoidLeased = true
	tableDesc, err = descriptors.GetImmutableTableByID(ctx, txn, sc.descID, flags)
	return err
	}); err != nil {
	return err
	}
	grp := ctxgroup.WithContext(ctx)
	// The various checks below operate at a fixed timestamp.
	runHistoricalTxn := sc.makeFixedTimestampRunner(readAsOf)
	for i := range constraints {
	c := constraints[i]
	grp.GoCtx(func(ctx context.Context) error {
	// Make the mutations public in a private copy of the descriptor
	// and add it to the Collection, so that we can use SQL below to perform
	// the validation. We wouldn't have needed to do this if we could have
	// updated the descriptor and run validation in the same transaction. However,
	// our current system is incapable of running long running schema changes
	// (the validation can take many minutes). So we pretend that the schema
	// has been updated and actually update it in a separate transaction that
	// follows this one.
	descI, err := tableDesc.MakeFirstMutationPublic(catalog.IgnoreConstraints)
	if err != nil {
	return err
	}
	desc := descI.(*tabledesc.Mutable)
	// Each check operates at the historical timestamp.
	return runHistoricalTxn(ctx, func(ctx context.Context, txn *kv.Txn, evalCtx *extendedEvalContext, ie sqlutil.InternalExecutor) error {
	// If the constraint is a check constraint that fails validation, we
	// need a semaContext set up that can resolve types in order to pretty
	// print the check expression back to the user.
	evalCtx.Txn = txn
	// Use the DistSQLTypeResolver because we need to resolve types by ID.
	collection := evalCtx.Descs
	resolver := descs.NewDistSQLTypeResolver(collection, txn)
	semaCtx := tree.MakeSemaContext()
	semaCtx.TypeResolver = &resolver
	// TODO (rohany): When to release this? As of now this is only going to get released
	// after the check is validated.
	defer func() { collection.ReleaseAll(ctx) }()
	if c.IsCheck() {
	if err := validateCheckInTxn(
	ctx, &semaCtx, evalCtx.SessionData(), desc, txn, ie, c.Check().Expr,
	); err != nil {
	return err
	}
	} else if c.IsForeignKey() {
	if err := validateFkInTxn(ctx, desc, txn, ie, collection, c.GetName()); err != nil {
	return err
	}
	} else if c.IsUniqueWithoutIndex() {
	if err := validateUniqueWithoutIndexConstraintInTxn(ctx, desc, txn, ie, evalCtx.SessionData().User(), c.GetName()); err != nil {
	return err
	}
	} else if c.IsNotNull() {
	if err := validateCheckInTxn(
	ctx, &semaCtx, evalCtx.SessionData(), desc, txn, ie, c.Check().Expr,
	); err != nil {
	// TODO (lucy): This should distinguish between constraint
	// validation errors and other types of unexpected errors, and
	// return a different error code in the former case
	return errors.Wrap(err, "validation of NOT NULL constraint failed")
	}
	} else {
	return errors.Errorf("unsupported constraint type: %d", c.ConstraintToUpdateDesc().ConstraintType)
	}
	return nil
	})
	})
	}
	if err := grp.Wait(); err != nil {
	return err
	}
	log.Info(ctx, "finished validating new constraints")
	return nil
	}

I added it simply bc I think this is a sane thing to do.

I believe it simply involves emitting UpdateTableBackReferencesInTypes and UpdateBackReferencesInSequences operations just like on the dropping path, in which case you might as well do so right now.

Since this is revertible, please add a test case which covers rolling back a drop after PUBLIC -> VALIDATED (the rollback will exercise VALIDATED -> PUBLIC on the adding path), if nothing else.

Shouldn't revertible(false) apply to WRITE_ONLY?

Actually there's foreign keys, index-backed uniques, and also UniqueWithoutIndex as well as not-NULL.

Wouldn't this be a good place for a TODO to expand this predicate to include all non-index-backed constraints?

nit: whitespace