[api] Require field ids for schema fields, auto-assign them for parameter lists - #9767
Draft
LuciferYang wants to merge 5 commits into
Draft
[api] Require field ids for schema fields, auto-assign them for parameter lists#9767LuciferYang wants to merge 5 commits into
LuciferYang wants to merge 5 commits into
Conversation
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
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>
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>
…arser-null-fieldid
…arser-null-fieldid
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
close #9766
DataTypeJsonParser.parseDataField(JsonNode)passed anullcounter into the private overload, so a field whose json carries no"id"hitfieldId.incrementAndGet()and threw a bareNullPointerException. That is reachable fromcreate_function:ParameterUtils.parseDataFieldArrayparses the user-writteninput_params/return_paramsjson, 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 howSchemaSerializer.deserializereads a stored table schema field by field, and how the globally registered Jackson deserializer forDataFieldworks. 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
parseDataFieldArraycreates 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:
NullPointerExceptionNullPointerExceptionPartial field id is not allowed.NullPointerExceptionField id is required but the field carries none.Tests
DataTypeJsonParserTestcovers 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.ParameterUtilsTestcoversparseDataFieldArrayend 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
ParameterUtilsreverted to master the id-less array test errors withField id is required but the field carries none., and on master itself the same input throwsNullPointerExceptionatDataTypeJsonParser.parseDataField.paimon-apiandpaimon-commonbuild clean with checkstyle and spotless.