Skip to content

[format] Validate nested types for JSON tables - #9585

Open
LuciferYang wants to merge 2 commits into
apache:masterfrom
LuciferYang:fix/json-nested-type-validation
Open

[format] Validate nested types for JSON tables#9585
LuciferYang wants to merge 2 commits into
apache:masterfrom
LuciferYang:fix/json-nested-type-validation

Conversation

@LuciferYang

Copy link
Copy Markdown
Contributor

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.

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. Recurse into the element, key, value and field
types so the rejection happens up front with the message that was already
written for it.

This 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, since none of MULTISET,
VARIANT, BLOB, GEOMETRY or GEOGRAPHY has a cast rule to or from string.
The comment claiming JSON supports all data types is removed with them; it
was never true, the whitelist has always rejected the same set at the top
level.

Note this covers managed tables only. SchemaValidation is the only caller
of validateDataFields, so a format table, which goes through
CatalogUtils.validateCreateTable, is unaffected either way.

Assisted-by: GLM-5.3
@JingsongLi

Copy link
Copy Markdown
Contributor

+1

@JingsongLi

Copy link
Copy Markdown
Contributor

Please rebase master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] JSON format accepts unsupported types nested inside ARRAY/MAP/ROW at create time

2 participants