wombatu-kun opened a new pull request, #8708:
URL: https://github.com/apache/paimon/pull/8708
### Purpose
Malformed or unexpectedly shaped JSON in an `ARRAY`, `MAP` or `ROW` column
is currently not an error during CDC ingestion: the value is silently replaced
with an empty one and committed. Three of the four failure paths are completely
silent.
| | malformed JSON | valid non-object / non-array JSON (`[1,2]`, `123`,
`""`) |
| --- | --- | --- |
| **MAP** | commits `{}`, only a `LOG.info` | commits `{}`, **no log line at
all** |
| **ROW** | `GenericRow(0)` -> obscure `ArrayIndexOutOfBoundsException`
later, while writing | `GenericRow(N)` all-null, **no log** |
| **ARRAY** | legacy comma-split fallback, or throws (correct) | empty
`GenericArray`, **no log** |
The quiet non-container path exists because `OBJECT_MAPPER.readTree(s)`
succeeds for valid JSON that simply is not an object or array.
`JsonNode.fields()` on a non-`ObjectNode` returns an empty iterator, and
iterating a non-`ArrayNode` yields nothing, so an empty value is produced
without any exception being raised.
This matters because Paimon already has a complete, documented
corrupt-record policy in `CdcRecordStoreWriteOperator#processElement`: a
bounded retry loop (`cdc.schema-change-retry-max-num`) re-reading the latest
schema, then either `cdc.skip-corrupt-record` -> log and drop the record, or
fail with `"Unable to process element. Possibly a corrupt record"`. A malformed
`INT` reaches that policy. A malformed `MAP` does not: `TypeUtils` swallows the
`JsonProcessingException` and returns an empty map, so the designed guard in
`CdcRecordUtils#toGenericRow` never sees it and the row commits with `{}` where
the source had data.
This PR does not introduce a policy, it routes the complex types into the
one that already exists and that every other type already obeys.
Both swallowing branches came from #4246, which added complex type support
and made `ARRAY` strict but left `MAP`/`ROW` lenient.
Changes:
- `TypeUtils#castFromStringInternal`: drop the `catch
(JsonProcessingException)` blocks that returned `new GenericMap(emptyMap())` /
`new GenericRow(0)`, and add a node-shape guard after `readTree` in all three
complex branches. Both failure modes are wrapped by the pre-existing terminal
`catch (Exception e)` into the same `RuntimeException("Failed to parse Json
String ...")`, so no new exception type escapes.
- The `ARRAY` legacy comma-separated fallback is deliberately untouched: it
only fires when `readTree` throws, which the shape guard does not affect. The
guard changes exactly one path, "readTree succeeded but the node is not an
array", which today silently yields an empty array.
- Remove a stale TODO in `CdcRecordUtils` claiming `TypeUtils` cannot handle
complex types, which has been false since #4246.
Two behaviours are deliberately preserved and now pinned by tests: an empty
JSON object stays a legitimate empty map, and a ROW whose JSON omits a field
keeps mapping it to null, since a CDC record may legitimately not carry every
field. Only the container shape becomes strict.
Note `VECTOR` delegates to the `ARRAY` branch, so the guard also stops a
vector of the wrong dimension being silently built from an unparsable value.
`CdcRecordUtils#projectAsInsert` is intentionally left unguarded: its only
caller, `CdcRecordKeyAndBucketExtractor`, uses it for partition, bucket key and
trimmed primary key, all of which `SchemaValidation` already restricts to
primitive types.
### Behaviour change
A pipeline whose source emits malformed complex-type JSON currently gets a
row with an empty value and a `LOG.info`. With the default
`cdc.skip-corrupt-record=false` it will now fail. That is the intent of the
fix, but it may surface as "my job started failing after the upgrade" for users
who had not noticed the loss.
The escape hatch already exists and is public:
`cdc.skip-corrupt-record=true`. Its semantics differ from the old behaviour on
purpose: it drops the whole record and logs a warning, rather than fabricating
an empty value for one column. Fabricating `{}` was never a coherent policy,
and putting it behind a new option would preserve a data-loss bug as a
supported mode.
One cost worth naming: a permanently corrupt record now spends
`cdc.schema-change-retry-max-num` x `cdc.schema-change-retry-interval` in the
retry loop before failing. That is exactly what a malformed `INT` costs today,
so this makes complex types consistent rather than introducing a new stall.
### Tests
`TypeUtilsTest`: new cases covering malformed JSON and
valid-but-non-container JSON (`[1,2]`, `123`, `"abc"`, `null`, `""`) for `MAP`,
`ROW` and `ARRAY`, plus three guards against over-strictness that pass both
before and after: `{}` stays an empty map, a ROW with a missing field still
maps it to null, and `a,b,c` still reaches the legacy comma-separated fallback.
All 15 new strictness assertions fail on master and pass with the fix; the
existing cases, including the nested `ARRAY<ROW<MAP>>` one, stay green.
`CdcRecordStoreWriteOperatorTest`: the CDC module had no corrupt-record test
at all. Added three that exercise the policy end-to-end through the operator
harness: a malformed `MAP` and a malformed `ROW` now fail with the
corrupt-record message, and with `cdc.skip-corrupt-record=true` the record is
dropped instead of being committed with an empty map. On master the first two
do not throw the policy error at all (the `MAP` row commits silently with `{}`;
the `ROW` one dies with an `ArrayIndexOutOfBoundsException` that
`skip-corrupt-record` cannot skip), and the third commits a data file.
### Known follow-up
`readTree("")` returns `MissingNode` and is now rejected by the same guard.
Beyond that, this PR does not change how `ARRAY` handles input that is not JSON
at all, which still goes through the legacy comma-split path for `VARCHAR`
elements.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
