cockroachdb / cockroachdb/cockroach
sql/opt/workloadindexrec: workload_index_recs drops the UNIQUE qualifier from replacement recommendations
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
**Describe the problem**
When a statement's recommendation replaces an existing **unique** index, `indexrec` marks the new index unique ([rec.go:385](https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/opt/indexrec/rec.go#L385)):
```go
Unique: recType == TypeReplaceIndex && existingIndex.IsUnique(),
```
`workload_index_recs()` then loses that. `collectIndexRecs` gates creates only on index type, predicate, and sharding — there is no uniqueness check — so a `CREATE UNIQUE INDEX` passes through ([workload_indexrecs.go:130](https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/opt/workloadindexrec/workload_indexrecs.go#L130)). `extractIndexCovering` then rebuilds the statement from the trie leaf without setting `Unique` ([workload_indexrecs.go:209](https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/opt/workloadindexrec/workload_indexrecs.go#L209)):
```go
index := tree.CreateIndex{
Table: table,
Columns: cisIndexedCols,
Storing: storingColsArray[i],
}
```
So the workload-level output recommends a **non-unique** index where the optimizer asked for a unique one. Following it produces an index that may not deliver the modeled benefit, since the plan that justified the recommendation could depend on the uniqueness guarantee.
**To Reproduce**
```sql
CREATE TABLE t (i INT, k INT);
CREATE UNIQUE INDEX t_i ON t (i);
SELECT k FROM t WHERE i = 1;
SELECT index_recommendations FROM crdb_internal.statement_statistics
WHERE metadata->>'query' LIKE 'SELECT k FROM t%';
-- {"replacement : CREATE UNIQUE INDEX ON t (i) STORING (k) /* ... */;"}
SELECT * FROM workload_index_recs();
-- CREATE INDEX ON t (i) STORING (k); <- UNIQUE is gone
```
**Expected behavior**
The output should not silently weaken a recommendation. Emitting a non-unique index in place of a unique one is wrong regardless of which direction we take below.
**Additional data**
Possible directions, none decided:
1. **Carry `Unique` through the merge.** The trie would have to treat uniqueness as part of the shape and refuse to merge in a way that invalidates it — merging a unique `(a)` with another statement's `(a, b)` yields a leaf that cannot be unique on `(a)`. Keeps everything in one pipeline but the merge semantics get complicated.
2. **Handle unique recommendations outside the merge.** Keep them out of the trie and pass them through to the output unmerged, so `UNIQUE` is preserved and merge semantics stay simple. Cost is possible redundancy between a passed-through unique index and a merged index covering the same columns, with no dedup between the two sets.
3. **Exclude them.** Skip unique creates in `collectIndexRecs`. Simplest, but silently loses a valid recommendation.
There is currently no test coverage — `logic_test/workload_indexrecs` contains no `UNIQUE` cases at all.
Jira issue: CRDB-66576
Contributor guide
Research direction
Start with collectIndexRecs and extractIndexCovering in pkg/sql/opt/workloadindexrec/workload_indexrecs.go, then compare their handling with the Unique field set in pkg/sql/opt/indexrec/rec.go. Run the workload_indexrecs logic tests and add UNIQUE cases covering the reproduced recommendation. Done means workload_index_recs no longer weakens a unique recommendation and the chosen merge behavior is tested.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, sql
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100