cockroachdb / cockroachdb/cockroach
sql/importer: parquet IMPORT error-message and edge-case polish (struct dump in errors, duplicate columns, family names, empty-string→NULL)
- 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
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