fmorillo7694 commented on PR #236: URL: https://github.com/apache/flink-connector-aws/pull/236#issuecomment-5908804557
Applied the review lenses from #206 (composition, swallowed conditions, read/write asymmetry) to the three format modules. Two commits on `2bc96bf`; the first fixes four places where the protobuf format produced wrong data silently, the second adds the nested-type coverage the JSON converter lacked. **Protobuf (63b3db5):** - `TIMESTAMP`/`TIMESTAMP_LTZ`/`TIME` travel as milliseconds, so precision above 3 lost digits on write with no way to restore them. Rejected at table creation now; the type table states `p <= 3`. (The Limitations text I had written yesterday documented the truncation instead of preventing it.) - `DECIMAL` is text with the writer's scale, read back with the reader's `DECIMAL(p, s)`. `DecimalData.fromBigDecimal` returns null when the integer part does not fit the reader's precision, which surfaced as a wrong value or a NOT NULL violation. The read now fails naming the value and the declared type; a narrower scale still rounds like `CAST`. - Two columns sanitizing to the same proto identifier (`` `a b` `` and `` `a-b` ``) produced a schema declaring the field twice: descriptor validation failed at `open()` on write and the reader mapped both to the first field. Rejected with both column names. - A null in a NOT NULL column was left unset, which proto3 implicit presence decodes as the type default. The writer fails the record; SQL sinks enforce NOT NULL upstream, DataStream users did not have that protection. Tests: writer `DECIMAL(12,4)` read as `DECIMAL(12,2)` (rounds) and as `DECIMAL(4,2)` (fails), precision 6/9 rejected while 0..3 accepted, colliding names, null in NOT NULL. **JSON (2bc96bf):** the converter recursed into containers correctly but nothing tested a parameterized, nullable or structured type inside one. Added `ARRAY<TIMESTAMP(3) NULL>`, `ARRAY<ROW>` with nested `required`, `ARRAY<ARRAY>`, `MAP<STRING, ARRAY<DECIMAL>>`, three-level `ROW` and an unsupported type nested in a container. Avro delegates to Flink's own `AvroSchemaConverter`, so there is no in-repo converter to audit there. Gates: avro 63, json 49, protobuf 35, checkstyle and spotless clean. -- 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]
