Skip to content

Fix PathHierarchyTokenizer optional fields incorrectly marked required causing deserialization failure - #2038

Open
gingeekrishna wants to merge 9 commits into
opensearch-project:mainfrom
gingeekrishna:fix/1797-path-hierarchy-tokenizer-optional-fields
Open

Fix PathHierarchyTokenizer optional fields incorrectly marked required causing deserialization failure#2038
gingeekrishna wants to merge 9 commits into
opensearch-project:mainfrom
gingeekrishna:fix/1797-path-hierarchy-tokenizer-optional-fields

Conversation

@gingeekrishna

@gingeekrishna gingeekrishna commented Jul 5, 2026

Copy link
Copy Markdown

Description

Fixes #1797

Problem: Any index using PathHierarchyTokenizer configured with OpenSearch defaults (omitting optional parameters) cannot be retrieved via the Java client:

java.lang.RuntimeException: Missing required property 'PathHierarchyTokenizer.bufferSize'

Root cause: _common.analysis___PathHierarchyTokenizer in opensearch-openapi.yaml listed buffer_size, delimiter, reverse, and skip in required. However these are all optional parameters with documented OpenSearch defaults:

  • buffer_size — default 1024
  • delimiter — default /
  • reverse — default false
  • skip — default 0

When an index is created without explicitly setting these, OpenSearch omits them from the response and the client throws MissingRequiredPropertyException.

Fix: Remove buffer_size, delimiter, reverse, skip from required (only type remains required) and update the generated PathHierarchyTokenizer.java to use @Nullable types with null guards in serialization, hashCode, and equals.

Changes

  • java-codegen/opensearch-openapi.yaml — remove 4 fields from required
  • java-client/src/generated/java/org/opensearch/client/opensearch/_types/analysis/PathHierarchyTokenizer.java — make all 4 fields nullable throughout

@gingeekrishna
gingeekrishna requested a review from reta as a code owner July 5, 2026 07:43
Copilot AI review requested due to automatic review settings July 5, 2026 07:43
Copilot AI reviewed Jul 5, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@gingeekrishna gingeekrishna mentioned this pull request Jul 5, 2026
Open
gingeekrishna pushed a commit to gingeekrishna/opensearch-java that referenced this pull request Jul 5, 2026
Radhakrishnan Pachyappan
Add CHANGELOG entry for opensearch-project#2038
2fed78c 
Signed-off-by: Radhakrishnan Pachyappan <gingeekrishnan@gmail.com>
gingeekrishna pushed a commit to gingeekrishna/opensearch-java that referenced this pull request Aug 16, 2026
Radhakrishnan Pachyappan @gingeekrishna
Add CHANGELOG entry for opensearch-project#2038
2dd5949 
Signed-off-by: Radhakrishnan Pachyappan <gingeekrishnan@gmail.com>
@gingeekrishna
gingeekrishna force-pushed the fix/1797-path-hierarchy-tokenizer-optional-fields branch from 2fed78c to 2dd5949 Compare August 16, 2026 16:12
reta
reta requested changes Aug 16, 2026
View reviewed changes
Comment thread java-codegen/opensearch-openapi.yaml

@reta reta Aug 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please submit a change against https://github.com/opensearch-project/openSearch-api-specification, thank you

@gingeekrishna gingeekrishna Aug 17, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Submitted the spec fix upstream: opensearch-project/opensearch-api-specification#1195

@gingeekrishna gingeekrishna mentioned this pull request Aug 17, 2026
Open
@gingeekrishna

gingeekrishna commented Aug 17, 2026

Copy link
Copy Markdown
Author

Submitted the spec fix upstream: opensearch-project/opensearch-api-specification#1195

gingeekrishna pushed a commit to gingeekrishna/opensearch-java that referenced this pull request Aug 30, 2026
Radhakrishnan Pachyappan @gingeekrishna
Add CHANGELOG entry for opensearch-project#2038
0d49134 
Signed-off-by: Radhakrishnan Pachyappan <gingeekrishnan@gmail.com>
@gingeekrishna
gingeekrishna force-pushed the fix/1797-path-hierarchy-tokenizer-optional-fields branch from 2dd5949 to 0d49134 Compare August 30, 2026 06:27
gingeekrishna pushed a commit to gingeekrishna/opensearch-java that referenced this pull request Aug 30, 2026
Radhakrishnan Pachyappan @gingeekrishna
Add CHANGELOG entry for opensearch-project#2038
293baef 
Signed-off-by: Radhakrishnan Pachyappan <gingeekrishnan@gmail.com>
@gingeekrishna
gingeekrishna force-pushed the fix/1797-path-hierarchy-tokenizer-optional-fields branch from 0d49134 to 293baef Compare August 30, 2026 06:35
gingeekrishna and others added 7 commits September 4, 2026 11:24
@gingeekrishna
Fix DerivedField serializing unsupported name field causing mapper_pa…
f967661 
…rsing_exception

The _common___DerivedField schema in opensearch-openapi.yaml included a
required name property. This caused DerivedField.java to serialize a
name key into the JSON mapping body, but the OpenSearch API does not
accept name inside a derived field definition and returns:
  mapper_parsing_exception: unknown parameter [name] on mapper

In the API, the derived field's name is expressed as the map key in the
parent derived object, not as a property inside the field definition.

Remove name from the _common___DerivedField schema (both from
properties and required) and update the generated DerivedField.java
accordingly. The Java client no longer serializes name inside derived
field definitions.

Fixes opensearch-project#1937

Signed-off-by: Radhakrishnan Pachyappan <gingeekrishna@gmail.com>
@gingeekrishna
Fix ShardFailure.shard incorrectly required causing deserialization f…
5ff8adf 
…ailure

ShardFailure.shard was marked as required in the spec, so the generated
class threw MissingRequiredPropertyException when OpenSearch returned a
ShardFailure without the shard field — masking the real failure.

Per the OpenSearch API, shard is an optional integer in ShardFailure
(the primary failure reason is always present; the shard number may
be absent for certain failure types).

Remove shard from the required list in _common___ShardFailure and
update the generated ShardFailure.java to use @nullable Integer with
proper null guards in serialization, hashCode, and equals.

Fixes opensearch-project#1799

Signed-off-by: Radhakrishnan Pachyappan <gingeekrishna@gmail.com>
@gingeekrishna
Add CHANGELOG entry for opensearch-project#2037
9ec59cc 
Signed-off-by: Radhakrishnan Pachyappan <gingeekrishna@gmail.com>
@gingeekrishna
Correct ShardFailure fix: rename fields to match server, not optional
526bc2c 
The previous commit made shard optional, on the assumption the server
sometimes omits it. Per review on the corresponding spec change
(opensearch-project/opensearch-api-specification#1194), that was
wrong: ReplicationResponse.ShardInfo.Failure on the server always
writes the field. The real bug was a JSON key mismatch: the server
writes _index, _shard, _node (underscore-prefixed), but the spec and
this client used index, shard, node, so deserialization looked up the
wrong key and threw MissingRequiredPropertyException on _shard/shard,
or silently dropped _index/_node.

Revert shard to a required, non-null int, and rename the three JSON
keys (index/node/shard -> _index/_node/_shard) in both
opensearch-openapi.yaml and the generated ShardFailure.java. Java
accessor/builder names are unchanged (index(), node(), shard()) per
the existing convention for underscore-prefixed API fields (see
Hit.index() for _index).

Signed-off-by: Radhakrishnan P <gingeekrishna@gmail.com>
@gingeekrishna
Update CHANGELOG wording for corrected ShardFailure fix
d19fd14 
Signed-off-by: Radhakrishnan P <gingeekrishna@gmail.com>
@gingeekrishna
Fix PathHierarchyTokenizer required fields causing deserialization fa…
e68ebf5 
…ilure

buffer_size, delimiter, reverse, and skip were marked required in the
spec, so any index using a PathHierarchyTokenizer configured with
OpenSearch defaults (omitting these optional parameters) could not be
retrieved — the client threw:
  Missing required property 'PathHierarchyTokenizer.bufferSize'

All four parameters are optional in the OpenSearch API with documented
defaults (buffer_size=1024, delimiter=/, reverse=false, skip=0).
Only type is actually required.

Remove buffer_size, delimiter, reverse, skip from required in the
spec and update the generated class to use @nullable types with
proper null guards in serialization, hashCode, and equals.

Fixes opensearch-project#1797

Signed-off-by: Radhakrishnan Pachyappan <gingeekrishna@gmail.com>
Radhakrishnan Pachyappan @gingeekrishna
Add CHANGELOG entry for opensearch-project#2038
edabcb6 
Signed-off-by: Radhakrishnan Pachyappan <gingeekrishnan@gmail.com>
@gingeekrishna
gingeekrishna force-pushed the fix/1797-path-hierarchy-tokenizer-optional-fields branch from 293baef to edabcb6 Compare September 4, 2026 05:55
@gingeekrishna
Remove unused ApiTypeHelper import from PathHierarchyTokenizer
c26c06d 
Left over from making buffer_size/delimiter/reverse/skip optional -
none of them use ApiTypeHelper.requireNonNull anymore, so the import
became dead. Caught by CI's spotlessJavaCheck and the
"Ensure Generated Code Is Up To Date" job (the real code generator
doesn't emit unused imports).

Signed-off-by: Radhakrishnan P <gingeekrishna@gmail.com>
@gingeekrishna
Match generated setter signatures to the optional-field convention
62858c5 
The previous commits made buffer_size/delimiter/reverse/skip nullable
fields but left their Builder setters taking primitive int/boolean (or
non-@nullable String) parameters - the required-field shape. CI's
"Ensure Generated Code Is Up To Date" job still reported the file as
dirty after the unused-import fix, since the real generator wouldn't
produce that mismatch.

Match the convention used elsewhere for optional fields (e.g.
EdgeNGramTokenFilter.maxGram/minGram): boxed, @nullable setter
parameters, and @nullable on the corresponding Builder field.

Signed-off-by: Radhakrishnan P <gingeekrishna@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@reta reta reta requested changes
@Bukhtawar Bukhtawar Awaiting requested review from Bukhtawar Bukhtawar is a code owner
@szczepanczykd szczepanczykd Awaiting requested review from szczepanczykd szczepanczykd is a code owner
@madhusudhankonda madhusudhankonda Awaiting requested review from madhusudhankonda madhusudhankonda is a code owner
@saratvemulapalli saratvemulapalli Awaiting requested review from saratvemulapalli saratvemulapalli is a code owner
@VachaShah VachaShah Awaiting requested review from VachaShah VachaShah is a code owner
@Xtansia Xtansia Awaiting requested review from Xtansia Xtansia is a code owner

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG] Indices with a PathHierarchyTokenizer without optional config params cannot be deserialized

3 participants

@gingeekrishna @reta