Skip to content

[api] Require field ids for schema fields, auto-assign them for parameter lists - #9767

Draft
LuciferYang wants to merge 5 commits into
apache:masterfrom
LuciferYang:fix/datatypejsonparser-null-fieldid
Draft

[api] Require field ids for schema fields, auto-assign them for parameter lists#9767
LuciferYang wants to merge 5 commits into
apache:masterfrom
LuciferYang:fix/datatypejsonparser-null-fieldid

Conversation

@LuciferYang

@LuciferYang LuciferYang commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Purpose

close #9766

DataTypeJsonParser.parseDataField(JsonNode) passed a null counter into the private overload, so a field whose json carries no "id" hit fieldId.incrementAndGet() and threw a bare NullPointerException. That is reachable from create_function: ParameterUtils.parseDataFieldArray parses the user-written input_params / return_params json, and a parameter list like [{"name":"x","type":"INT"}] has no ids to carry.

The fix is not to make that entry auto-assign. parseDataField(JsonNode) is also how SchemaSerializer.deserialize reads a stored table schema field by field, and how the globally registered Jackson deserializer for DataField works. Table field ids drive projection and schema evolution, so quietly turning a missing id into 0 there would replace a loud failure with a wrong table. That entry stays strict; it now says so instead of throwing an NPE.

What changes instead is the caller that genuinely has an id-less list. The counter-taking overload becomes public, and parseDataFieldArray creates one counter for the whole array, so sibling fields get 0, 1, 2. Threading it per element would have handed every field id 0.

Behaviour per input, all of which previously threw an NPE as soon as one id was missing:

parameter list before after
no ids anywhere NullPointerException ids 0, 1, 2
every field has an id works unchanged
some fields have ids NullPointerException Partial field id is not allowed.
a table schema field with no id NullPointerException Field id is required but the field carries none.

Tests

DataTypeJsonParserTest covers the strict entry rejecting an id-less field, one counter handing out 0, 1, 2 across successive fields, an explicit id being kept, and the existing ROW path still numbering its fields sequentially.

ParameterUtilsTest covers parseDataFieldArray end to end: an id-less array becomes 0, 1, 2; explicit ids are preserved; a partially-id'd array is rejected with a message rather than an NPE.

Verified fail-on-base on JDK 11: with ParameterUtils reverted to master the id-less array test errors with Field id is required but the field carries none., and on master itself the same input throws NullPointerException at DataTypeJsonParser.parseDataField. paimon-api and paimon-common build clean with checkstyle and spotless.

The public parseDataField(JsonNode) entry passed a null field
counter, so parsing a field json without an "id" node threw a bare
NullPointerException from the counter increment — while the sibling
public parseDataType entry auto-assigns ids for exactly such input.
Reachable from the create-function procedures via
ParameterUtils.parseDataFieldArray on user-provided parameter json.

Mirror parseDataType: pass a fresh counter so missing ids are
auto-assigned and explicit ids keep working (the private overload's
partial-id check is unchanged).

Assisted-by: GLM-5.3
@LuciferYang
LuciferYang marked this pull request as draft September 13, 2026 03:06
…ists

The previous version made the public parseDataField(JsonNode) auto-assign, but
that entry is also how SchemaSerializer.deserialize and the global Jackson
DataField deserializer read table schemas, where a missing id would silently
become 0 and table field ids drive projection and schema evolution. Keep that
entry strict (now with a message instead of an NPE) and let the caller that
genuinely has an id-less list, ParameterUtils.parseDataFieldArray, pass one
counter for the whole array so sibling fields get 0, 1, 2 rather than all 0.

Co-Authored-By: Claude Code <noreply@anthropic.com>
@LuciferYang LuciferYang changed the title [api] Auto-assign field id in DataTypeJsonParser.parseDataField [api] Require field ids for schema fields, auto-assign them for parameter lists Sep 13, 2026
LuciferYang and others added 3 commits September 13, 2026 14:21
The guard only fired when an id-less field came first: with the explicit id
first the counter was still at -1, so the check passed and the id-less fields
that followed drew 0 from it, colliding silently. That is the defect the change
set out to remove. Decide up front instead: a list that carries any id at all
gets no counter, so every field in it must carry one, and that also stops a
nested id-less row inside a numbered list from drawing a colliding id.

Co-Authored-By: Claude Code <noreply@anthropic.com>
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] DataTypeJsonParser.parseDataField throws NPE on field json without an id

1 participant