cockroachdb / cockroachdb/cockroach

sql/opt/workloadindexrec: workload_index_recs drops the UNIQUE qualifier from replacement recommendations

Open
#173,254 1 comment 0 reactions 0 assignees View on GitHub
A-sql-optimizer C-bug O-agent T-sql-queries
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.