cockroachdb / cockroachdb/cockroach

sql: stabilize test expectations wrt schema descriptor IDs

Open
#110,487 0 comments 0 reactions 0 assignees View on GitHub
C-enhancement quality-friday T-sql-foundations
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

Currently every time we add a new object in the system database, it shifts the start descriptor ID for all the non-system tables and other SQL objects. Then all the test expectations throughout the code base become invalid.

So the work needed to add a new object in the system database is abnormally complex/excessive, as the person doing this needs to revisit every other test in many areas they are not concerned with.

The solution that was discussed with @rafiss and @fqazi is to bump the descriptor ID sequence in **test servers** so that the first SQL object defined by tests starts at a stable ID independent of how many objects are included in the system database.

An example way to do this is this:
```go
ie := tenant.InternalExecutor().(*sql.InternalExecutor)
_, err := ie.ExecEx(⋄, "force-desc-id", nil, /* txn */
sessiondata.NodeUserSessionDataOverride,
`SELECT setval('system.descriptor_id_seq', 10000)`)
require.NoError(t, err)
```

We need special consideration (to be documented as comment in the code) about the choice of 10000 vs another integer offset.

We need a value such that, for many IDs generated sequentially from it, we preserve the nice property that `MakeTablePrefix(ID).PrefixEnd() == MakeTablePrefix(ID+1)`.

This is true of all values between 1 and 108, but e.g not true of 109, 2xx, 511, 7xx, 1023.

So for example, 1000 would be a poor offset choice because as soon as 23 SQL objects are defined, we'd get artifacts in test expectations (e.g. "/PrefixEnd" appears in key string representations).

10000 is better because the first value after 10000 that doesn't work is 10239, so we get 239 IDs that behave peacefully.

Jira issue: CRDB-31457
Epic: CRDB-28893

Contributor guide

Open the contributing guide

Research direction

Locate the test-server setup and the code that initializes or advances system.descriptor_id_seq, then review the InternalExecutor example in the issue. The change is complete when SQL objects created by tests start from a stable offset, the offset rationale is documented in code, and affected test expectations remain stable as system objects change.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, sql
Domain
databases, testing-qa
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.