Skip to content

fix(_common.analysis): make PathHierarchyTokenizer fields optional - #1195

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

fix(_common.analysis): make PathHierarchyTokenizer fields optional#1195
gingeekrishna wants to merge 2 commits into
opensearch-project:mainfrom
gingeekrishna:fix/path-hierarchy-tokenizer-optional-fields

Conversation

@gingeekrishna

Copy link
Copy Markdown

Description

Remove buffer_size, delimiter, reverse, and skip from the required list in the PathHierarchyTokenizer schema in spec/schemas/_common.analysis.yaml.

All four fields are optional in the OpenSearch API with documented defaults (buffer_size=1024, delimiter=/, reverse=false, skip=0). Marking them required caused MissingRequiredPropertyException when retrieving any index whose PathHierarchyTokenizer was configured with defaults, since OpenSearch omits default values from the response.

Only type is actually required. The four fields remain defined as properties and continue to be serialized/deserialized when present.

Related Issues

Reported via opensearch-java#1797 and the corresponding fix opensearch-java#2038, where reviewer @reta requested this upstream spec change.

Signed-off-by: Radha Krishnan P gingeekrishna@gmail.com

buffer_size, delimiter, reverse, and skip are optional in the OpenSearch
API with documented defaults (buffer_size=1024, delimiter=/, reverse=false,
skip=0). Marking them required caused MissingRequiredPropertyException when
retrieving any index whose PathHierarchyTokenizer was configured with
defaults (omitting those parameters).

Only type is actually required. Remove the four optional fields from the
required list; they remain defined as properties so they are still
serialized/deserialized when present.

Reported via opensearch-project/opensearch-java#1797 and fixed in
opensearch-project/opensearch-java#2038.

Signed-off-by: Radha Krishnan P <gingeekrishna@gmail.com>
Copilot AI lite review requested due to automatic review settings August 17, 2026 03:42

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.

skip:
$ref: '_common.yaml#/components/schemas/StringifiedInteger'
required:
- buffer_size

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this fixes path_hierarchy but AnalysisTokenizerPathHierarchyReverse has the exact same problem, it also marks buffer_size, delimiter, reverse, and skip as required. both types use the same factory class with the same defaults.

Can you fix that one too in this? otherwise half the bug is still there.

@gingeekrishna gingeekrishna Aug 24, 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.

Thanks for the review! I searched the repo for AnalysisTokenizerPathHierarchyReverse (and for any other schema with the same buffer_size/delimiter/reverse/skip shape) and couldn't find a second definition anywhere - PathHierarchyTokenizer in _common.analysis.yaml is the only one, and this PR already drops all four from its required list.

I also checked the OpenSearch server source (CommonAnalysisModulePlugin.java): it registers path_hierarchy and the deprecated camelCase alias PathHierarchy, both backed by the same PathHierarchyTokenizerFactory. reverse is just a boolean setting on that one factory, there's no separate reverse-specific tokenizer type server-side.

Could you point me to the exact file/line (or a link) for AnalysisTokenizerPathHierarchyReverse? Want to make sure I fix the right thing rather than guessing.

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.

3 participants