cockroachdb / cockroachdb/cockroach
crosscluster/logical: JSONB breaks equal⟹same-hash in transactional LDR lock derivation (shared Fingerprint defect)
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
**Describe the problem**
Transactional LDR lock derivation asserts `equal(rowA, rowB) ⟹ hash(rowA) ==
hash(rowB)`. `hash()` uses `EncDatum.Fingerprint`; `equal()` uses
`Datum.Compare`. JSONB `'1'`, `'1.0'`, `'1.00'` all compare equal
(`jsonNumber.Compare` → `apd.Decimal.Cmp`, scale-insensitive) and collide in a
source forward index (`keyside.EncodeForwardIndex` key-encodes JSON numbers via
`EncodeDecimalAscending`, which normalizes scale). But `Fingerprint` forces JSON
through *value* encoding (`mustUseValueEncodingForFingerprinting`), which does
**not** reduce the scale, so the three produce different hashes.
**Impact**
Missed conflicts on JSONB unique/FK columns → duplicate-key errors / divergence.
Nothing in compatibility checking forbids JSONB unique indexes.
Measured fingerprints (via `EncDatum.Fingerprint`):
DEC 1 = 2a0200 1.0 = 2a0200 1.00 = 2a0200 (identical)
JSON 1 = ...3348901 1.0 = ...334890a 1.00 = ...3348964 (all differ)
Decimal is safe because `valueside.Encode` reduces the `apd.Decimal` first; JSON
is not reduced.
**Shared defect — also affects DistSQL.** This is a defect in the shared
`Fingerprint`, not LDR-local. Every consumer relying on `equal ⟹ same
fingerprint` inherits it: DISTINCT (`rowexec/distinct.go` uses the fingerprint as
the seen-set key with no follow-up `Compare`), GROUP BY / hash aggregation, hash
routers, and windower PARTITION BY.
Minimal SQL repro (portable — runs on both CockroachDB and PostgreSQL) — three
scale-differing values that CockroachDB treats as `=`. DECIMAL collapses to one
group; JSONB does not:
```sql
-- DECIMAL DISTINCT: 1 row on both CRDB and Postgres.
SELECT count(*) FROM (
SELECT DISTINCT d FROM (VALUES (1::decimal), (1.0::decimal), (1.00::decimal)) v(d)
) s;
-- JSONB DISTINCT: CRDB returns 3, but CRDB's own '=' says they are equal
-- (below) — that internal inconsistency is the bug. On Postgres, jsonb '=' is
-- scale-sensitive so 3 is self-consistent and expected.
SELECT count(*) FROM (
SELECT DISTINCT j FROM (VALUES ('1'::jsonb), ('1.0'::jsonb), ('1.00'::jsonb)) v(j)
) s;
-- Equality check. CRDB: true (contradicts the DISTINCT=3 above).
-- Postgres: false (consistent with its DISTINCT=3).
SELECT '1'::jsonb = '1.00'::jsonb;
```
The defect is the *contradiction within CockroachDB*: `'1'::jsonb = '1.00'::jsonb`
returns true, yet `DISTINCT` keeps them apart — so `equal` and the fingerprint
disagree. (Postgres is self-consistent: jsonb equality is scale-sensitive, so both
are 3/false.)
Existing JSON fingerprint regression tests (#118380 tuple key-encoding, #113103
ordering) do not cover the DISTINCT/GROUP BY scale case. NB: the historical
reason `Fingerprint` value-encodes JSON is mixed-version hash-router stability,
not correctness.
**Code references**
- [txnlock/column_set.go:43-68](https://github.com/cockroachdb/cockroach/blob/master/pkg/crosscluster/logical/txnlock/column_set.go#L43-L68) (hash) vs [column_set.go:86-119](https://github.com/cockroachdb/cockroach/blob/master/pkg/crosscluster/logical/txnlock/column_set.go#L86-L119) (equal)
- [rowenc/encoded_datum.go:328-397](https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/rowenc/encoded_datum.go#L328-L397) — `mustUseValueEncodingForFingerprinting` / `Fingerprint`
- [rowexec/distinct.go:180](https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/rowexec/distinct.go#L180) — fingerprint used as seen-set key
**Suggested fix**
- LDR: hash JSON via the forward-index key encoding (`keyside`), which matches
what the source unique index enforces — not `Fingerprint`, which models value
equality; or reject JSONB constraint columns in txn-mode validation.
- DistSQL (separate concern, tracked here): reduce/normalize composite JSON in
`Fingerprint` as decimal already is, or document + test the divergence.
Epic CRDB-65552
Contributor guide
Assessment
This issue has not been assessed yet.