cockroachdb / cockroachdb/cockroach

sql/importer: no datum-type check before KV encoding — converter bugs become silent corruption or unreadable rows

Open
#172,334 0 comments 0 reactions 0 assignees View on GitHub
A-import C-enhancement O-agent O-qa T-sql-foundations
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

**Describe the problem**

IMPORT performs no check that a converted datum's type matches the target column's type before value-encoding it into a KV. `tree.AdjustValueToType` passes through datums whose kind doesn't match the column family, and value encoding is by-datum. So whenever a format converter emits a datum kind that disagrees with the target column, the outcome is decided by accidental wire-format overlap:

- **Same wire format** → silently works (e.g. timestamp vs. timestamptz, bytes vs. string). Tests pass by luck.
- **Different but decodable** → **silent garbage** (#172328 string→TIMESTAMPTZ, #172332 unsigned ints, and int-backed DECIMAL(scale>0)→INT where `123.45` imports as `2`).
- **Undecodable** → rows that "import" successfully but can never be read, poisoning the table and ultimately crashing nodes via auto stats (#172330).

All of those are individually fixable converter/validation bugs — but the class keeps recurring because nothing downstream catches a mismatched datum. A single defensive check in the parquet `FillDatums` path (or generally in `DatumRowConverter.Row`) that the datum's `ResolvedType` family is compatible with the target column family would convert every current and future bug of this class from silent corruption into a clean per-row error.

**Expected behavior**

A converter emitting a wrong-kind datum should produce an import error naming the column and both types — never encoded KVs.

**Additional context**

Found while root-causing the four linked corruption bugs: the individual fixes close today's holes; this check is what prevents the next one. Cost is one family comparison per datum; if that's measurable, it could be debug-build-only or sampled, but given IMPORT's per-row costs it likely isn't.

Jira issue: CRDB-65561

Epic CRDB-66113

Contributor guide

Open the contributing guide

Research direction

Start by reading the parquet FillDatums path and the general DatumRowConverter.Row entry point named in the issue, then trace how converted datums reach value encoding. Done means a wrong-kind datum produces a per-row import error naming the column and both types, with no KV encoded.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, sql
Domain
backend, databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.