cockroachdb / cockroachdb/cockroach

sql/importer: parquet IMPORT error-message and edge-case polish (struct dump in errors, duplicate columns, family names, empty-string→NULL)

Open
#172,343 1 comment 0 reactions 0 assignees View on GitHub
A-import C-bug 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**

A batch of small message-quality and edge-case problems in parquet IMPORT, collected during 26.3 QA. Filed together since each is minor; can be split if useful.

**1. Row errors leak a Go struct dump.** Importing `40000` into an `INT2` column:

```
error parsing row 1: generate insert row: integer out of range for type int2 (row: &{[0x51e48f337900] 1 0})
```

The `(row: &{[0x…] 1 0})` part is a raw pointer/struct print — useless to users (CSV prints the offending row's data). NULL-in-NOT-NULL errors have the same leak: `(row: &{[0x51e49eca68c0 ] 3 0})`. Other paths (invalid WKB, invalid enum value) print `(row: row 2)` — redundant wording, but no struct dump.

**2. Duplicate column names in a parquet file are silently accepted, last column wins — even with `strict_validation`.** A file with two `a` INT64 columns (values 1 and 2) imports `a = 2` with no warning. Should be an error at least in strict mode, in the spirit of the existing case-conflict check (which produces an excellent error).

**3. Internal type-family names leak into user-facing errors**, e.g. `double type can only be converted to FLOAT or DECIMAL, got IntFamily`, `...cannot be converted to TupleFamily`, `...got StringFamily`. Should be SQL type names. Also the doubled prefix in `duplicate key: duplicate key: /Table/281/1/2/0` (shared with CSV).

**4. Zero-length BYTE_ARRAY (present, non-null empty string) silently becomes NULL for non-string targets.** The `len(v)==0 → DNull` short-circuit in the JSON/Timestamp/Decimal/Geo/Enum/Interval conversion branches rewrites an empty string to NULL — verified: `''` into JSONB → NULL with a `succeeded` job, while `''` into STRING is preserved. Present-but-empty should be a parse error (or preserved), not a silent NULL; for NOT NULL columns it surfaces as a misleading null-constraint violation. Arguably more than message polish — flagging here rather than as its own corruption issue since real writers rarely put `''` in non-string columns.

**Expected behavior**

1. Print the row's data (like CSV) or drop the suffix.
2. Error on duplicate file columns, at minimum under `strict_validation`.
3. SQL type names in errors; single `duplicate key:` prefix.
4. Empty string into a non-string target → parse error, not NULL.

**Environment:**

- Current master (26.3 dev).

Jira issue: CRDB-65570

Epic CRDB-66113

Contributor guide

Open the contributing guide

Research direction

Start by tracing the parquet IMPORT row-error, duplicate-column validation, type-conversion, and empty-value conversion paths described in the report; no file or test names are provided. Compare parquet behavior with CSV and strict_validation handling, then add focused coverage for the four expected behaviors: useful row errors or no suffix, SQL type names, one duplicate-key prefix, and parse errors instead of NULL for empty strings.

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
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.