Migrate to Jackson 3.x APIs by reta · Pull Request #5703 · opensearch-project/sql

reta · 2026-08-19T17:06:46Z

Description

Migrate to Jackson 3.x APIs. The OpenSearch Core will stop bundling Jackson 2.x (planned for 3.9.0), and it does not prevent plugins from using Jackson 2.x if needed, however all plugins have been migrated to Jackson 3.x APIs.

Related Issues

Part of opensearch-project/OpenSearch#22197, fixes the migration gap after #5361

Check List

New functionality includes testing.
New functionality has been documented.
New functionality has javadoc added.
New functionality has a user manual doc added.
New PPL command checklist all confirmed.
API changes companion pull request created.
Commits are signed per the DCO using --signoff or -s.
Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

github-actions · 2026-08-19T17:07:40Z

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit 3d4ffb1.

Path	Line	Severity	Description
core/build.gradle	56	high	Three Jackson dependencies migrated from 'com.fasterxml.jackson' group to 'tools.jackson' group (jackson-core, jackson-databind, jackson-dataformat-yaml). Namespace change from an established artifact to a different group ID must be verified against official Jackson 3.x release artifacts.
opensearch/build.gradle	38	high	Three Jackson dependencies migrated from 'com.fasterxml.jackson.core/dataformat' to 'tools.jackson.core/dataformat' group (jackson-core, jackson-databind, jackson-dataformat-cbor). Supply chain risk: artifact authenticity cannot be confirmed without maintainer verification.
plugin/build.gradle	158	high	Two Jackson core dependencies (jackson-core, jackson-databind) migrated from 'com.fasterxml.jackson.core' to 'tools.jackson.core' group. Dependency group namespace change must be verified against official artifact registries.
prometheus/build.gradle	25	high	Three Jackson dependencies migrated from 'com.fasterxml.jackson' to 'tools.jackson' group (jackson-core, jackson-databind, jackson-dataformat-cbor). Namespace change requires maintainer verification of artifact provenance.
protocol/build.gradle	34	high	Three Jackson dependencies plus a resolutionStrategy.force directive migrated from 'com.fasterxml.jackson' to 'tools.jackson' group. The forced resolution override for the new namespace amplifies supply chain risk if the artifact is not legitimate.

The table above displays the top 10 most important findings.

Total: 5 | Critical: 0 | High: 5 | Medium: 0 | Low: 0

Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.

⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.

github-actions · 2026-08-19T18:00:54Z

PR Reviewer Guide 🔍

(Review updated until commit `84459ea`)

Here are some key observations to aid the review process:

🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ Recommended focus areas for review Possible Issue The YAML mapper configuration changed from using `setSerializationInclusion` and `configure` methods to a builder pattern with `changeDefaultPropertyInclusion` called twice. The second call at line 39 may overwrite the first call at line 37, potentially losing the `NON_NULL` value inclusion setting. This could cause null values to appear in YAML output where they were previously excluded. .changeDefaultPropertyInclusion( incl -> incl.withValueInclusion(JsonInclude.Include.NON_NULL)) .changeDefaultPropertyInclusion( incl -> incl.withContentInclusion(JsonInclude.Include.NON_NULL)) API Change The `JsonFactory.createParser` call now requires an `ObjectReadContext` parameter (line 110). If `ObjectReadContext.empty()` does not provide the same configuration as the default context in Jackson 2.x, this could alter JSON parsing behavior, potentially affecting how JSON is parsed or causing failures with certain input formats. try (JsonParser parser = JSON_FACTORY.createParser(ObjectReadContext.empty(), jsonStr)) { Test Mismatch The test now expects `StreamReadException` instead of `IOException` (line 26), and mocks `jsonNode.traverse(ObjectReadContext.empty())` instead of `jsonNode.traverse()` (line 25). If the production code in `OpenSearchJsonContent.geoValue()` still catches `IOException` or calls `traverse()` without `ObjectReadContext`, this test will not accurately verify error handling.

github-actions · 2026-08-19T18:01:34Z

PR Code Suggestions ✨

Latest suggestions up to 84459ea

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Handle potential null from asString The `asString()` method may return `null` for non-string nodes in Jackson 3.x. Verify that `ExprStringValue` handles null values correctly, or add null-checking to prevent potential `NullPointerException`. core/src/main/java/org/opensearch/sql/utils/JsonUtils.java [99] -return new ExprStringValue(jsonNode.asString()); +String value = jsonNode.asString(); +return value != null ? new ExprStringValue(value) : LITERAL_NULL; Suggestion importance[1-10]: 7 __ Why: The suggestion raises a valid concern about potential null values from `asString()` in Jackson 3.x. Adding null-checking would improve robustness, though the actual behavior depends on whether `ExprStringValue` already handles nulls.	Medium
	Optimize field iteration performance Using `propertyNames()` followed by `get(field)` performs two lookups per field. Use `properties()` iterator instead to access both key and value in a single iteration, improving performance. opensearch/src/main/java/org/opensearch/sql/opensearch/data/utils/OpenSearchJsonContent.java [77-79] -mapValue - .propertyNames() - .forEach(field -> map.put(field, new OpenSearchJsonContent(mapValue.get(field)))); +for (Map.Entry<String, JsonNode> entry : mapValue.properties()) { + map.put(entry.getKey(), new OpenSearchJsonContent(entry.getValue())); +} Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a performance issue where `propertyNames()` followed by `get(field)` performs two lookups. Using `properties()` iterator is more efficient and directly provides both key and value.	Medium
	Use proper ObjectReadContext configuration Creating a parser with `ObjectReadContext.empty()` may not properly configure deserialization settings. Verify that this context is appropriate for your use case, or consider using a properly configured `ObjectReadContext` to ensure correct parsing behavior. core/src/main/java/org/opensearch/sql/expression/function/jsonUDF/JsonExtractAllFunctionImpl.java [110] -try (JsonParser parser = JSON_FACTORY.createParser(ObjectReadContext.empty(), jsonStr)) { +ObjectReadContext context = mapper.createReadContext(); +try (JsonParser parser = JSON_FACTORY.createParser(context, jsonStr)) { Suggestion importance[1-10]: 7 __ Why: The suggestion raises a valid concern about using `ObjectReadContext.empty()` which may not properly configure deserialization settings. However, without knowing the specific requirements, it's unclear if this is actually problematic in this context.	Medium
	Simplify iteration over properties The `properties()` method returns an `Iterable`, not an `Iterator`. Calling `.iterator()` on it is correct, but the variable name `iter` suggests it's already an iterator. Consider using the enhanced for-loop directly with `properties()` for cleaner code. core/src/main/java/org/opensearch/sql/utils/JsonUtils.java [93-96] -for (var iter = jsonNode.properties().iterator(); iter.hasNext(); ) { - Map.Entry<String, JsonNode> entry = iter.next(); +for (Map.Entry<String, JsonNode> entry : jsonNode.properties()) { values.put(entry.getKey(), processJsonNode(entry.getValue())); } Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies that using an enhanced for-loop with `properties()` is cleaner than calling `.iterator()` explicitly. However, both approaches are functionally equivalent and the improvement is primarily stylistic.	Low

Previous suggestions

Suggestions up to commit 355903c

Category	Suggestion	Impact
General	Combine property inclusion configurations The `changeDefaultPropertyInclusion` method is called twice with different configurations. This may cause the first configuration to be overwritten by the second. Consider combining both inclusions into a single call to ensure both value and content inclusions are properly configured. core/src/main/java/org/opensearch/sql/utils/YamlFormatter.java [37-40] YAML_MAPPER = new ObjectMapper(builder.build()) .rebuild() .accessorNaming( new DefaultAccessorNamingStrategy.Provider().withFirstCharAcceptance(true, true)) .changeDefaultPropertyInclusion( - incl -> incl.withValueInclusion(JsonInclude.Include.NON_NULL)) - .changeDefaultPropertyInclusion( - incl -> incl.withContentInclusion(JsonInclude.Include.NON_NULL)) + incl -> incl.withValueInclusion(JsonInclude.Include.NON_NULL) + .withContentInclusion(JsonInclude.Include.NON_NULL)) .build(); Suggestion importance[1-10]: 7 __ Why: Calling `changeDefaultPropertyInclusion` twice may cause the first configuration to be overwritten. Combining both `withValueInclusion` and `withContentInclusion` in a single call would be more correct and efficient.	Medium
	Verify array iteration behavior consistency The migration from `elements()` to `values().iterator()` changes the iteration behavior. In Jackson 3.x, `values()` returns an `Iterable`, but verify that this provides the same iteration order and behavior as the previous `elements()` method to avoid subtle bugs in array processing. opensearch/src/main/java/org/opensearch/sql/opensearch/data/utils/OpenSearchJsonContent.java [85] @Override public Iterator<? extends Content> array() { + // Jackson 3.x: values() returns Iterable<JsonNode> for array elements return Iterators.transform(value.values().iterator(), OpenSearchJsonContent::new); } Suggestion importance[1-10]: 6 __ Why: The suggestion asks to verify the behavior change from `elements()` to `values().iterator()`, which is a valid concern during migration. However, it only adds a comment without providing concrete verification or testing approach.	Low
	Use configured ObjectReadContext for parsing The `createParser` method now requires an `ObjectReadContext` parameter in Jackson 3.x. Using `ObjectReadContext.empty()` may not provide proper configuration for parsing. Consider using a properly configured context from the `ObjectMapper` instance to ensure correct deserialization behavior. core/src/main/java/org/opensearch/sql/expression/function/jsonUDF/JsonExtractAllFunctionImpl.java [110] -try (JsonParser parser = JSON_FACTORY.createParser(ObjectReadContext.empty(), jsonStr)) { +ObjectMapper mapper = new ObjectMapper(); +try (JsonParser parser = JSON_FACTORY.createParser(mapper.createReadContext(), jsonStr)) { JsonToken token; while ((token = parser.nextToken()) != null) { switch (token) { ... case PROPERTY_NAME: String fieldName = parser.currentName(); pathStack.push(fieldName); break; Suggestion importance[1-10]: 5 __ Why: While using `ObjectReadContext.empty()` may work, using a properly configured context could provide better parsing behavior. However, the suggestion doesn't demonstrate that the current approach is incorrect, and the improved code introduces a new `ObjectMapper` instance which may not be the intended design.	Low

Suggestions up to commit 86829e4

Category	Suggestion	Impact
Possible issue	Use null-safe text extraction method In Jackson 3.x, `asString()` may return null for non-string nodes, whereas `asText()` provides a string representation for all node types. Using `asString()` could cause NullPointerException if the node type is unexpectedly non-string. Verify the node type or use a null-safe approach. core/src/main/java/org/opensearch/sql/utils/JsonUtils.java [99] case STRING: - return new ExprStringValue(jsonNode.asString()); + return new ExprStringValue(jsonNode.asText()); Suggestion importance[1-10]: 5 __ Why: The suggestion raises a valid concern about null safety. However, since this is within a `case STRING:` block, the node type is already verified. The change from `asText()` to `asString()` is intentional for Jackson 3.x migration, where `asString()` is the preferred method for string nodes.	Low

Suggestions up to commit fc56e79

Category	Suggestion	Impact
General	Verify ObjectReadContext configuration The `createParser` method now requires an `ObjectReadContext` parameter in Jackson 3.x. Verify that `ObjectReadContext.empty()` provides the correct configuration for your parsing needs. If custom deserialization features or type handling are required, consider using a properly configured `ObjectReadContext` instead of an empty one. core/src/main/java/org/opensearch/sql/expression/function/jsonUDF/JsonExtractAllFunctionImpl.java [110] -try (JsonParser parser = JSON_FACTORY.createParser(ObjectReadContext.empty(), jsonStr)) { +ObjectReadContext context = ObjectReadContext.empty(); // Or configure as needed +try (JsonParser parser = JSON_FACTORY.createParser(context, jsonStr)) { JsonToken token; while ((token = parser.nextToken()) != null) { switch (token) { ... case PROPERTY_NAME: String fieldName = parser.currentName(); pathStack.push(fieldName); break; Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that `ObjectReadContext.empty()` is used with the new Jackson 3.x API. However, since this is a migration from Jackson 2.x where no such context was needed, using `empty()` is likely the correct default behavior. The suggestion to verify is valid but the impact is moderate unless specific deserialization features are required.	Medium
	Verify properties iteration behavior The migration from `fields()` to `properties().iterator()` changes the iteration behavior. Ensure that `properties()` returns the expected field entries and maintains the same iteration order as the previous `fields()` method, especially if field ordering is critical for your application logic. core/src/main/java/org/opensearch/sql/utils/JsonUtils.java [93-96] +// Verify properties() maintains expected behavior for (var iter = jsonNode.properties().iterator(); iter.hasNext(); ) { Map.Entry<String, JsonNode> entry = iter.next(); values.put(entry.getKey(), processJsonNode(entry.getValue())); } Suggestion importance[1-10]: 7 __ Why: The migration from `fields()` to `properties().iterator()` is a valid API change concern. The suggestion to verify iteration order and behavior is reasonable, especially if field ordering matters. However, Jackson typically maintains insertion order for object fields, so this is more of a verification task than a critical issue.	Medium
	Verify propertyNames forEach compatibility The migration from `fieldNames().forEachRemaining()` to `propertyNames().forEach()` changes the iteration mechanism. Verify that `propertyNames()` returns an `Iterable` or `Stream` that supports `forEach`, as the API signature may differ from the previous `Iterator`-based approach. opensearch/src/main/java/org/opensearch/sql/opensearch/data/utils/OpenSearchJsonContent.java [78-80] +// Verify propertyNames() returns correct type for forEach mapValue .propertyNames() .forEach(field -> map.put(field, new OpenSearchJsonContent(mapValue.get(field)))); Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies the API change from `fieldNames().forEachRemaining()` to `propertyNames().forEach()`. Verifying that `propertyNames()` returns an appropriate type for `forEach` is valid, though Jackson 3.x's `propertyNames()` is designed to return an `Iterable<String>` which supports `forEach`, making this primarily a verification concern.	Medium
	Combine property inclusion settings The `changeDefaultPropertyInclusion` method is called twice with different inclusion settings. Verify that both calls are necessary and that the second call doesn't override the first. Consider combining them into a single call if both value and content inclusions need to be set together. core/src/main/java/org/opensearch/sql/utils/YamlFormatter.java [32-41] YAML_MAPPER = new ObjectMapper(builder.build()) .rebuild() .accessorNaming( new DefaultAccessorNamingStrategy.Provider().withFirstCharAcceptance(true, true)) .changeDefaultPropertyInclusion( - incl -> incl.withValueInclusion(JsonInclude.Include.NON_NULL)) - .changeDefaultPropertyInclusion( - incl -> incl.withContentInclusion(JsonInclude.Include.NON_NULL)) + incl -> incl.withValueInclusion(JsonInclude.Include.NON_NULL) + .withContentInclusion(JsonInclude.Include.NON_NULL)) .build(); Suggestion importance[1-10]: 6 __ Why: The suggestion identifies duplicate calls to `changeDefaultPropertyInclusion` and proposes combining them. While this could improve code clarity, both calls may be intentional to set different aspects (value vs content inclusion). The improved code shows a cleaner approach, but the functional impact is minimal since both settings are the same (`NON_NULL`).	Low

Suggestions up to commit 8e740fe

Category	Suggestion	Impact
General	Combine property inclusion configurations The `changeDefaultPropertyInclusion` method is called twice with different configurations. This may cause the first call to be overridden by the second. Consider combining both value and content inclusion settings in a single call to ensure both configurations are applied correctly. core/src/main/java/org/opensearch/sql/utils/YamlFormatter.java [32-41] YAML_MAPPER = new ObjectMapper(builder.build()) .rebuild() .accessorNaming( new DefaultAccessorNamingStrategy.Provider().withFirstCharAcceptance(true, true)) .changeDefaultPropertyInclusion( - incl -> incl.withValueInclusion(JsonInclude.Include.NON_NULL)) - .changeDefaultPropertyInclusion( - incl -> incl.withContentInclusion(JsonInclude.Include.NON_NULL)) + incl -> incl.withValueInclusion(JsonInclude.Include.NON_NULL) + .withContentInclusion(JsonInclude.Include.NON_NULL)) .build(); Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that `changeDefaultPropertyInclusion` is called twice, which could lead to the first configuration being overridden. Combining both `withValueInclusion` and `withContentInclusion` in a single call ensures both settings are applied correctly and improves code clarity.	Medium
	Verify ObjectReadContext configuration The `createParser` method signature has changed in Jackson 3.x. Verify that `ObjectReadContext.empty()` is the correct context for your use case. If custom deserialization features or type handling are needed, consider using `ObjectReadContext` with appropriate configuration instead of the empty context. core/src/main/java/org/opensearch/sql/expression/function/jsonUDF/JsonExtractAllFunctionImpl.java [110] -try (JsonParser parser = JSON_FACTORY.createParser(ObjectReadContext.empty(), jsonStr)) { +// Verify ObjectReadContext configuration matches requirements +ObjectReadContext context = ObjectReadContext.empty(); // or configure as needed +try (JsonParser parser = JSON_FACTORY.createParser(context, jsonStr)) { JsonToken token; while ((token = parser.nextToken()) != null) { switch (token) { ... case PROPERTY_NAME: String fieldName = parser.currentName(); pathStack.push(fieldName); break; Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that `ObjectReadContext.empty()` is used with the new Jackson 3.x API. While the code may work, verifying that an empty context is appropriate for the use case is a valid concern, especially if custom deserialization features are needed.	Medium
	Optimize property iteration performance The migration from `fieldNames()` to `propertyNames()` changes the return type from `Iterator` to `Stream`. While `forEach` works, this creates an intermediate stream that may impact performance. Consider using `properties().iterator()` directly for better efficiency. opensearch/src/main/java/org/opensearch/sql/opensearch/data/utils/OpenSearchJsonContent.java [74-80] @Override public Iterator<Map.Entry<String, Content>> map() { LinkedHashMap<String, Content> map = new LinkedHashMap<>(); final JsonNode mapValue = value(); - mapValue - .propertyNames() - .forEach(field -> map.put(field, new OpenSearchJsonContent(mapValue.get(field)))); + for (Iterator<Map.Entry<String, JsonNode>> it = mapValue.properties().iterator(); it.hasNext(); ) { + Map.Entry<String, JsonNode> entry = it.next(); + map.put(entry.getKey(), new OpenSearchJsonContent(entry.getValue())); + } return map.entrySet().iterator(); } Suggestion importance[1-10]: 6 __ Why: The suggestion identifies a potential performance optimization by avoiding the intermediate stream created by `propertyNames().forEach()`. Using `properties().iterator()` directly is more efficient and aligns with the pattern used elsewhere in the PR (e.g., in `JsonUtils.java`).	Low

Suggestions up to commit d601bd9

Category	Suggestion	Impact
Possible issue	Combine property inclusion configurations Calling `changeDefaultPropertyInclusion()` twice may cause the first configuration to be overwritten by the second. Combine both value and content inclusion settings in a single call to ensure both configurations are applied correctly. core/src/main/java/org/opensearch/sql/utils/YamlFormatter.java [32-38] YAML_MAPPER = YAMLMapper.builder(builder.build()) .changeDefaultPropertyInclusion( - incl -> incl.withValueInclusion(JsonInclude.Include.NON_NULL)) - .changeDefaultPropertyInclusion( - incl -> incl.withContentInclusion(JsonInclude.Include.NON_NULL)) + incl -> incl.withValueInclusion(JsonInclude.Include.NON_NULL) + .withContentInclusion(JsonInclude.Include.NON_NULL)) .build(); Suggestion importance[1-10]: 8 __ Why: This identifies a potential bug where calling `changeDefaultPropertyInclusion()` twice may cause the first configuration to be overwritten. Combining both settings in a single call ensures both `withValueInclusion` and `withContentInclusion` are applied correctly.	Medium
General	Verify properties iteration behavior The migration from `fields()` to `properties().iterator()` changes the iteration behavior. Verify that `properties()` returns the expected entries and maintains the same ordering as the previous `fields()` method to prevent data processing issues. core/src/main/java/org/opensearch/sql/utils/JsonUtils.java [93-96] +// Verify properties() maintains same behavior as fields() for (var iter = jsonNode.properties().iterator(); iter.hasNext(); ) { Map.Entry<String, JsonNode> entry = iter.next(); values.put(entry.getKey(), processJsonNode(entry.getValue())); } Suggestion importance[1-10]: 6 __ Why: The migration from `fields()` to `properties().iterator()` is a significant API change that warrants verification. While likely correct, confirming that ordering and behavior remain consistent is important for data processing correctness.	Low
	Verify ObjectReadContext configuration The `ObjectReadContext.empty()` may not provide necessary configuration for parsing. Consider using a properly configured `ObjectReadContext` with appropriate settings for the JSON parsing requirements, or verify that an empty context is sufficient for all use cases. core/src/main/java/org/opensearch/sql/expression/function/jsonUDF/JsonExtractAllFunctionImpl.java [110] try (JsonParser parser = JSON_FACTORY.createParser(ObjectReadContext.empty(), jsonString)) { JsonToken token; - + // Verify ObjectReadContext.empty() provides required parsing configuration while ((token = parser.nextToken()) != null) { switch (token) { Suggestion importance[1-10]: 5 __ Why: The suggestion to verify `ObjectReadContext.empty()` is valid but represents a precautionary check rather than a critical issue. The migration appears intentional and likely tested, making this a moderate-priority verification task.	Low
	Verify propertyNames forEach compatibility The migration from `fieldNames().forEachRemaining()` to `propertyNames().forEach()` changes the iteration mechanism. Verify that `propertyNames()` returns an `Iterable` or `Stream` that supports `forEach()` and maintains the same iteration order to prevent potential runtime errors. opensearch/src/main/java/org/opensearch/sql/opensearch/data/utils/OpenSearchJsonContent.java [78-80] +// Verify propertyNames() returns compatible type for forEach mapValue .propertyNames() .forEach(field -> map.put(field, new OpenSearchJsonContent(mapValue.get(field)))); Suggestion importance[1-10]: 5 __ Why: The migration from `fieldNames().forEachRemaining()` to `propertyNames().forEach()` changes the iteration mechanism. While likely correct, verifying API compatibility and iteration order is a reasonable precautionary measure.	Low

github-actions · 2026-08-20T12:26:52Z

Persistent review updated to latest commit 8e740fe

github-actions · 2026-08-20T17:16:32Z

Persistent review updated to latest commit fc56e79

Signed-off-by: Andriy Redko <drreta@gmail.com>

github-actions · 2026-08-20T19:54:42Z

Persistent review updated to latest commit 86829e4

Review feedback: the argument-name rule was open where the other OpenSearch functions enumerate their names, and the accepted set had to be mirrored in Java as a result. `bucketArgName` lists the four names this lowering handles -- `field`, `interval`, `fixed_interval`, `calendar_interval` -- so anything else is a parse error, which is the one exception RestSQLQueryAction falls back on. The handoff is the grammar's now, not a table's. `LEGACY_ONLY_ARGS` is gone with it, and so is the leftover-argument branch: once the four names are taken out of the map it is always empty. What remains are the three checks that cannot move downstream, because `spanFromSpanLengthLiteral` dereferences the interval on its first line. This also removes the failure mode behind the previous commit. That set had to list every parameter the legacy engine implements, and four were missing; with the grammar deciding, a name nobody listed falls back on its own. Two consequences worth stating. The quoted spelling now reaches the legacy engine rather than being lowered here -- which is where it went before this function was defined at all, so nothing that used to work stops working. And `missing` is dropped: `AggMaker` does not implement it either, so there is nothing to defer to, and the `MISSING` token was unreachable behind `MISSING_LITERAL` (ANTLR warns about this directly). `FIXED_INTERVAL` and `CALENDAR_INTERVAL` are new tokens, added to `keywordsCanBeId` so they can still name a column. Verified: 82 parser unit tests, none failing; `:sql:build` green including the coverage gate. Integration tests could not run locally -- the 3.9.0 distro no longer bundles Jackson 2.x, so the plugin fails to install with jar hell on `main` as well, pending opensearch-project#5703. Signed-off-by: Jialiang Liang <ryanleeang@gmail.com>

Signed-off-by: Andriy Redko <drreta@gmail.com>

github-actions · 2026-08-20T21:06:16Z

Persistent review updated to latest commit 355903c

Signed-off-by: Andriy Redko <drreta@gmail.com>

github-actions · 2026-08-20T22:19:43Z

Persistent review updated to latest commit 84459ea

reta requested review from LantaoJin, RyanL1997, Swiddis, acarbonetto, ahkcs, anirudha, dai-chen, joshuali925, mengweieric, noCharger, penghuo, ps48, qianheng-aws, songkant-aws, vamsimanohar, ykmr1224 and yuancu as code owners August 19, 2026 17:06

reta force-pushed the issue-5342 branch from 010ee23 to 3d4ffb1 Compare August 19, 2026 17:41

reta added maintenance Improves code quality, but not the product skip-diff-analyzer Maintainer to skip code-diff-analyzer check, after reviewing issues in AI analysis. v3.9.0 labels Aug 19, 2026

reta force-pushed the issue-5342 branch from 3d4ffb1 to d601bd9 Compare August 19, 2026 17:59

reta force-pushed the issue-5342 branch from d601bd9 to 8e740fe Compare August 20, 2026 12:25

reta force-pushed the issue-5342 branch from 8e740fe to fc56e79 Compare August 20, 2026 17:15

Migrate to Jackson 3.x APIs

86829e4

Signed-off-by: Andriy Redko <drreta@gmail.com>

reta force-pushed the issue-5342 branch from fc56e79 to 86829e4 Compare August 20, 2026 19:53

RyanL1997 approved these changes Aug 20, 2026

View reviewed changes

More test fixes (YAML formatting)

355903c

Signed-off-by: Andriy Redko <drreta@gmail.com>

Even more test fixes (YAML formatting)

84459ea

Signed-off-by: Andriy Redko <drreta@gmail.com>

Navigation Menu

Uh oh!

Migrate to Jackson 3.x APIs - #5703

Migrate to Jackson 3.x APIs#5703
reta wants to merge 3 commits into
opensearch-project:mainfrom
reta:issue-5342

reta commented Aug 19, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 19, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 19, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 19, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 20, 2026

Uh oh!

github-actions Bot commented Aug 20, 2026

Uh oh!

github-actions Bot commented Aug 20, 2026

Uh oh!

github-actions Bot commented Aug 20, 2026

Uh oh!

github-actions Bot commented Aug 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

reta commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Check List

Uh oh!

github-actions Bot commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Analyzer ❗

Uh oh!

github-actions Bot commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Reviewer Guide 🔍

(Review updated until commit 84459ea)

Uh oh!

github-actions Bot commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Suggestions ✨

Previous suggestions

Uh oh!

github-actions Bot commented Aug 20, 2026

Uh oh!

github-actions Bot commented Aug 20, 2026

Uh oh!

github-actions Bot commented Aug 20, 2026

Uh oh!

github-actions Bot commented Aug 20, 2026

Uh oh!

github-actions Bot commented Aug 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

reta commented Aug 19, 2026 •

edited

Loading

github-actions Bot commented Aug 19, 2026 •

edited

Loading

github-actions Bot commented Aug 19, 2026 •

edited

Loading

(Review updated until commit `84459ea`)

github-actions Bot commented Aug 19, 2026 •

edited

Loading