LuciferYang opened a new pull request, #9585:
URL: https://github.com/apache/paimon/pull/9585

   ### Purpose
   
   close #9584
   
   `JsonFileFormat.validateDataType` let ARRAY, VECTOR, MAP and ROW fall 
through to one `break`, so it only ever looked at the outermost type root. A 
type outside the supported set nested inside one of them, say `ARRAY<VARIANT>`, 
passed create-table validation and then failed on the first write with a cast 
error from `JsonFormatWriter`, or on read with a bare `NullPointerException` 
where `convertPrimitiveStringToType` dereferences a null cast executor. This 
recurses into the element, key, value and field types, so the rejection happens 
up front with the message that was already written for it.
   
   That is what the other formats do: ORC through 
`OrcTypeUtil.convertToOrcType`, Parquet through 
`ParquetSchemaConverter.convertToParquetType` and Avro through 
`AvroSchemaConverter.convertToSchema` all walk into nested types and reject 
what they cannot represent. JSON was written like CSV, whose whitelist has no 
container types at all, but with the container types added to it.
   
   No table that works today stops working. The types this now rejects are 
exactly the ones that already fail at runtime: none of `MULTISET`, `VARIANT`, 
`BLOB`, `GEOMETRY` or `GEOGRAPHY` has a cast rule to or from string, at any 
nesting depth. The comment claiming JSON supports all data types goes with 
them; it was never true, since the whitelist has always rejected that same set 
at the top level.
   
   Scope worth stating: `SchemaValidation` is the only entry point for 
`validateDataFields`, so this covers managed tables. A format table is created 
through `CatalogUtils.validateCreateTable`, which does not call it, and is 
unaffected either way.
   
   ### Tests
   
   `JsonFileFormatTest.testValidateRejectsUnsupportedNestedType` asserts 
rejection at five nested positions: `ARRAY<VARIANT>`, `MAP<VARIANT, STRING>`, 
`MAP<STRING, VARIANT>`, `ROW<INT, VARIANT>` and `ARRAY<ROW<VARIANT>>`. The two 
MAP shapes are separate because the key and the value are two independent 
recursion calls, and the last one covers two levels.
   
   It then validates the same shapes filled with supported types. That control 
is not decoration: it pins the `break;` this change had to add to the primitive 
cases. Without it the primitives fall into `case ARRAY` and die with a 
`ClassCastException`, and all five rejection assertions would still pass in 
that state.
   
   Against the unfixed validator the first assertion fails with "Expecting code 
to raise a throwable".
   
   `mvn -pl paimon-format test` on JDK 8: 597 tests, 0 failures. 
`spotless:check` and `checkstyle:check` are 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]

Reply via email to