-
Notifications
You must be signed in to change notification settings - Fork 4k
Description
Is your feature request related to a problem? Please describe.
When the declarative schema changer does an index backfill, it runs validation queries on the new index. For an ALTER PRIMARY KEY operation, all of the indexes in the table need to be rebuilt.
The way it works right now is each secondary index is validated separately. See:
cockroach/pkg/sql/schemachanger/scexec/exec_validation.go
Lines 19 to 50 in 1497315
| func executeValidateIndex(ctx context.Context, deps Dependencies, op *scop.ValidateIndex) error { | |
| descs, err := deps.Catalog().MustReadImmutableDescriptors(ctx, op.TableID) | |
| if err != nil { | |
| return err | |
| } | |
| desc := descs[0] | |
| table, ok := desc.(catalog.TableDescriptor) | |
| if !ok { | |
| return catalog.WrapTableDescRefErr(desc.GetID(), catalog.NewDescriptorTypeError(desc)) | |
| } | |
| index, err := catalog.MustFindIndexByID(table, op.IndexID) | |
| if err != nil { | |
| return err | |
| } | |
| // Execute the validation operation as a node user. | |
| execOverride := sessiondata.NodeUserSessionDataOverride | |
| switch index.GetType() { | |
| case idxtype.FORWARD: | |
| err = deps.Validator().ValidateForwardIndexes(ctx, deps.TransactionalJobRegistry().CurrentJob(), table, []catalog.Index{index}, execOverride) | |
| case idxtype.INVERTED: | |
| err = deps.Validator().ValidateInvertedIndexes(ctx, deps.TransactionalJobRegistry().CurrentJob(), table, []catalog.Index{index}, execOverride) | |
| case idxtype.VECTOR: | |
| // TODO(drewk): consider whether we can perform useful validation for | |
| // vector indexes. | |
| default: | |
| return errors.AssertionFailedf("unexpected index type %v", index.GetType()) | |
| } | |
| if err != nil { | |
| return scerrors.SchemaChangerUserError(err) | |
| } | |
| return nil | |
| } |
When that code gets to populateExpectedCounts, it gets the count by scanning the original primary index:
Line 2039 in 07d4a96
| query := fmt.Sprintf(`SELECT count(1)%s FROM [%d AS t]@[%d]`, partialIndexCounts, desc.GetID(), desc.GetPrimaryIndexID()) |
This means that the primary index is going to be scanned once for each secondary index in the table.
Describe the solution you'd like
Only scan the primary index once.
Additional context
This won't be needed if we use INSPECT for validation (#156582).
Jira issue: CRDB-56071
Epic CRDB-55074