pingcap / pingcap/tidb

[import] conflicted-row files can be lossy or silently incomplete

Open
#69,804 0 comments 0 reactions 0 assignees View on GitHub
component/DXF component/global-sort component/import component/observability found-by-ai severity/moderate type/bug
Dominant language
Go
Stars
40.5k
Forks
6.2k
PR merge metrics
PR metrics pending

Description

## Bug Report

### 1. Minimal reproduce step (Required)

This report is based on static analysis of current master; it is **not** a claim of a completed runtime reproduction.

#### Scenario A: a captured row contains a value longer than 2048 bytes or an embedded newline

Create a table whose duplicate unique key causes both input rows to be captured:

```sql
CREATE TABLE t (
id BIGINT PRIMARY KEY CLUSTERED,
uk BIGINT NOT NULL UNIQUE,
payload LONGTEXT
);
```

Generate a CSV in which one conflicted row has a 2050-byte value and the other has a newline inside a quoted field:

```sh
python3 - <<'PY'
import csv

with open("conflicts.csv", "w", newline="") as f:
csv.writer(f).writerows([
(1, 10, "A" * 2050),
(2, 10, "line 1\nline 2"),
])
PY
```

Upload the CSV to supported external storage, then run global-sort conflict capture:

```sql
IMPORT INTO t
FROM '/conflicts.csv'
WITH cloud_storage_uri='',
on_duplicate_key='capture';
```

Inspect the conflicted-row files produced under the sort storage. The code documents these files as being for users to resolve conflicts manually. This report does **not** assume that they have a stable re-import grammar; the issue is that a documented manual-resolution artifact cannot faithfully identify the original row values or even its physical record boundaries.

#### Scenario B: recording exceeds the 1 GiB cap

Repeat duplicate-key groups with near-2-KiB payloads until the formatted conflicted-row output in one collect-conflicts subtask exceeds 1 GiB. Let the import finish, then compare the number of rows present in the conflicted-row files with the conflicted-row count and `Result_Message` from:

```sql
SHOW IMPORT JOB ;
```

The existing unit-test equivalent is to lower `mockTotalConflictRowFileSizeLimit`: it demonstrates that file recording stops while row count and full-KV checksum accounting continue.

### 2. What did you expect to see? (Required)

For scenario A, the user-facing conflicted-row artifact should preserve the complete original value and unambiguous row/field boundaries. If a faithful representation is intentionally unavailable, the artifact and SQL job result should explicitly tell the user that the value is lossy and provide a supported way to locate the full row.

For scenario B, `SHOW IMPORT JOB` should explicitly say that conflicted-row recording was capped and that the files contain only a prefix/subset of the reported conflicted rows. The number of reported conflicts must not imply that every row is present in the manual-resolution files.

### 3. What did you see instead (Required)

Static analysis of the audited commit shows three artifact-fidelity/observability problems:

1. [`recordRowToFile`](https://github.com/pingcap/tidb/blob/59f6e85cd01756d53f799c33afba0a185d956d3f/pkg/dxf/importinto/conflictedkv/collector.go#L177-L210) uses `types.DatumsToString(row, true)`. [`datumsToString`](https://github.com/pingcap/tidb/blob/59f6e85cd01756d53f799c33afba0a185d956d3f/pkg/types/datum.go#L2441-L2500) keeps only the first 2048 bytes of each converted datum. The first sample value is therefore represented approximately as:

```text
(1, 10, "AAAA..." len(2050))
```

The `len(2050)` suffix signals local truncation, but the omitted bytes cannot be recovered from the artifact.

2. The same formatter writes string contents without record-safe escaping. The newline sample is physically written as:

```text
(2, 10, "line 1
line 2")
```

`recordRowToFile` then appends another newline as its row delimiter. Embedded quotes, other control characters, and binary/non-printable data have the same general ambiguity because this call does not use a record-safe encoding.

3. The collector has a hardcoded 1 GiB per-subtask cap and, after it is crossed, [`onTotalSizeLimitExceeded`](https://github.com/pingcap/tidb/blob/59f6e85cd01756d53f799c33afba0a185d956d3f/pkg/dxf/importinto/conflictedkv/collector.go#L238-L258) stops writing rows. [`HandleEncodedRow`](https://github.com/pingcap/tidb/blob/59f6e85cd01756d53f799c33afba0a185d956d3f/pkg/dxf/importinto/conflictedkv/collector.go#L154-L174) still increments `RowCount` and updates the checksum for every later full KV. The cap flag is copied only to [`CollectConflictsStepMeta.ConflictedRowRecordingCapped`](https://github.com/pingcap/tidb/blob/59f6e85cd01756d53f799c33afba0a185d956d3f/pkg/dxf/importinto/proto.go#L164-L181). The scheduler propagates the conflict count and `TooManyConflicts` but not the recording-cap flag into the [job `Summary`](https://github.com/pingcap/tidb/blob/59f6e85cd01756d53f799c33afba0a185d956d3f/pkg/executor/importer/import.go#L391-L419), and [`SHOW IMPORT JOB`](https://github.com/pingcap/tidb/blob/59f6e85cd01756d53f799c33afba0a185d956d3f/pkg/executor/show.go#L2535-L2543) can still show only `N conflicted rows.` with no indication that the files are incomplete.

Scope: this report does not assert direct table corruption. Conflict deletion and checksum accounting operate on the re-encoded full KV pairs (for example, [`Deleter.HandleEncodedRow`](https://github.com/pingcap/tidb/blob/59f6e85cd01756d53f799c33afba0a185d956d3f/pkg/dxf/importinto/conflictedkv/deleter.go#L182-L184)); the defect is the fidelity and SQL observability of the user-facing manual-resolution artifact.

### 4. What is your TiDB version? (Required)

Current master audited at commit `59f6e85cd01756d53f799c33afba0a185d956d3f`.

Contributor guide

Open the contributing guide

Research direction

Start in pkg/dxf/importinto/conflictedkv/collector.go, especially recordRowToFile, HandleEncodedRow, and onTotalSizeLimitExceeded, then trace CollectConflictsStepMeta through pkg/executor/importer/import.go and SHOW IMPORT JOB in pkg/executor/show.go. Run the existing unit-test equivalent using mockTotalConflictRowFileSizeLimit. Done means conflicted-row artifacts preserve unambiguous values and boundaries, and capped recording is visible in the job result.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, sql
Domain
databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.