Extract analytics-api module for JDK 21 downstream consumers

[bowenlan-amzn]

What

Extract sandbox/libs/analytics-api (JDK 21) holding just the interfaces downstream plugins compile against:

• org.opensearch.analytics.exec.QueryPlanExecutor — moved from analytics-framework
• org.opensearch.analytics.schema.OpenSearchSchemaBuilder — moved from analytics-engine

Why

analytics-framework / analytics-engine target JDK 25 (they use FFM). Published Gradle metadata pins jvm.version=25, which blocks JDK-21 consumers like opensearch-sql from depending on these modules — even though opensearch-sql only needs the two interfaces above, not FFM.

A thin JDK-21 module unblocks those consumers without downgrading the engine.

Design

• analytics-api declares Calcite as compileOnly. The types appear in its public signatures, but the module never loads standalone — consumers bring their own Calcite.
• Consumers declare analytics-api explicitly (compileOnly in sandbox plugins, implementation in analytics-engine). No transitive api re-exports.
• Package paths unchanged — no source edits in downstream sandbox modules.

Verification

• ./gradlew check -p sandbox -Dsandbox.enabled=true — BUILD SUCCESSFUL.
• Published jar: class version 65, module metadata declares jvm.version: 21.
• opensearch-sql PR compiles against the artifact from mavenLocal and passes jarHell. Switch sandbox deps to analytics-api (JDK 21 surface) sql#5426

Check List

• Functionality includes testing.
• API changes companion pull request created, if applicable.
• Public documentation issue/PR created, if applicable.

[github-actions]

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit 34366be.

Path	Line	Severity	Description
sandbox/libs/analytics-api/build.gradle	28	high	New external dependency added: 'org.apiguardian:apiguardian-api:1.1.2'. Mandatory flag per supply chain policy — maintainers must verify artifact authenticity and namespace ownership before merging.
sandbox/libs/analytics-api/build.gradle	29	high	New external dependency added: 'org.checkerframework:checker-qual:3.43.0'. Mandatory flag per supply chain policy — maintainers must verify artifact authenticity and namespace ownership before merging.

The table above displays the top 10 most important findings.

Total: 2 | Critical: 0 | High: 2 | 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]

PR Reviewer Guide 🔍

(Review updated until commit 2a375f5)

Here are some key observations to aid the review process:

🧪 No relevant tests

🔒 No security concerns identified

📝 TODO sections sandbox/libs/analytics-api/src/main/java/org/opensearch/analytics/schema/OpenSearchSchemaBuilder.java [28]: Use version sql plugin has built and re-purpose to not call node-client

🔀 No multiple PR themes

⚡ Recommended focus areas for review Unchecked Cast Lines 52-55 and 134 perform unchecked casts from Object to Map<String, Object> without validation. If the mapping structure is malformed or unexpected (e.g., properties is a List or String instead of a Map), a ClassCastException will be thrown at runtime. This can occur with custom or corrupted index mappings. @SuppressWarnings("unchecked") Map<String, Object> sourceMap = mapping.sourceAsMap(); @SuppressWarnings("unchecked") Map<String, Object> properties = (Map<String, Object>) sourceMap.get("properties"); Unchecked Cast Line 134 performs an unchecked cast from Object to Map<String, Object> for fieldEntry.getValue() without validation. If a field's value in the properties map is not a Map (e.g., it's a primitive or List), a ClassCastException will be thrown. This can happen with malformed or non-standard mappings. Map<String, Object> fieldProps = (Map<String, Object>) fieldEntry.getValue();

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 2a375f5
Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Add null check for sourceMap Add null check for sourceMap before accessing it. If sourceAsMap() returns null, calling get("properties") will throw a NullPointerException. This could happen with malformed or empty mappings. sandbox/libs/analytics-api/src/main/java/org/opensearch/analytics/schema/OpenSearchSchemaBuilder.java [52-55] @SuppressWarnings("unchecked") Map<String, Object> sourceMap = mapping.sourceAsMap(); +if (sourceMap == null) { + continue; +} @SuppressWarnings("unchecked") Map<String, Object> properties = (Map<String, Object>) sourceMap.get("properties"); Suggestion importance[1-10]: 7 __ Why: Adding a null check for sourceMap prevents a potential NullPointerException when sourceAsMap() returns null. This is a valid defensive programming practice that improves robustness when handling malformed mappings.	Medium
	Validate type before casting fieldEntry value Add type safety check before casting fieldEntry.getValue() to Map. If the value is not a Map (e.g., a primitive or null), the unchecked cast will cause a ClassCastException at runtime when accessing fieldProps. sandbox/libs/analytics-api/src/main/java/org/opensearch/analytics/schema/OpenSearchSchemaBuilder.java [132-134] for (Map.Entry<String, Object> fieldEntry : properties.entrySet()) { String fieldName = pathPrefix.isEmpty() ? fieldEntry.getKey() : pathPrefix + "." + fieldEntry.getKey(); - Map<String, Object> fieldProps = (Map<String, Object>) fieldEntry.getValue(); + Object value = fieldEntry.getValue(); + if (!(value instanceof Map)) { + continue; + } + Map<String, Object> fieldProps = (Map<String, Object>) value; Suggestion importance[1-10]: 7 __ Why: Adding type validation before casting fieldEntry.getValue() to Map prevents ClassCastException at runtime. This defensive check ensures the code handles unexpected data structures gracefully, improving robustness.	Medium
	Validate nested properties type before casting Add type safety check before casting fieldProps.get("properties") to Map. If the "properties" value exists but is not a Map, the unchecked cast will cause a ClassCastException during recursive processing. sandbox/libs/analytics-api/src/main/java/org/opensearch/analytics/schema/OpenSearchSchemaBuilder.java [138-143] if (fieldType == null || "object".equals(fieldType)) { - Map<String, Object> nested = (Map<String, Object>) fieldProps.get("properties"); - if (nested != null) { + Object nestedObj = fieldProps.get("properties"); + if (nestedObj instanceof Map) { + Map<String, Object> nested = (Map<String, Object>) nestedObj; addLeafFields(builder, typeFactory, nested, fieldName); } continue; } Suggestion importance[1-10]: 7 __ Why: Adding type validation before casting fieldProps.get("properties") to Map prevents potential ClassCastException. This improves error handling when processing nested object structures with unexpected data types.	Medium

---

Previous suggestions

Suggestions up to commit 0833d01

Category	Suggestion	Impact
Possible issue	Add null check for sourceMap Add null check for sourceMap before accessing its properties. If sourceAsMap() returns null, calling get("properties") will throw a NullPointerException. Validate the map exists before proceeding with property extraction. sandbox/libs/analytics-api/src/main/java/org/opensearch/analytics/schema/OpenSearchSchemaBuilder.java [52-55] @SuppressWarnings("unchecked") Map<String, Object> sourceMap = mapping.sourceAsMap(); +if (sourceMap == null) { + continue; +} @SuppressWarnings("unchecked") Map<String, Object> properties = (Map<String, Object>) sourceMap.get("properties"); Suggestion importance[1-10]: 7 __ Why: Adding a null check for sourceMap prevents a potential NullPointerException when mapping.sourceAsMap() returns null. This is a valid defensive programming practice that improves robustness.	Medium
	Validate fieldProps before type access Add null check for fieldProps before accessing its properties. The cast from fieldEntry.getValue() could result in null or an incompatible type, leading to ClassCastException or NullPointerException when calling get("type"). sandbox/libs/analytics-api/src/main/java/org/opensearch/analytics/schema/OpenSearchSchemaBuilder.java [134-135] -Map<String, Object> fieldProps = (Map<String, Object>) fieldEntry.getValue(); +Object fieldValue = fieldEntry.getValue(); +if (!(fieldValue instanceof Map)) { + continue; +} +@SuppressWarnings("unchecked") +Map<String, Object> fieldProps = (Map<String, Object>) fieldValue; String fieldType = (String) fieldProps.get("type"); Suggestion importance[1-10]: 7 __ Why: Validating fieldProps before casting prevents potential ClassCastException or NullPointerException. The suggestion adds type safety by checking if the value is actually a Map before casting.	Medium
General	Handle empty field builder case Handle the case where addLeafFields adds no fields to the builder. Calling builder.build() on an empty builder may produce an invalid or unexpected schema. Consider adding a validation or default field to ensure the table has at least one column. sandbox/libs/analytics-api/src/main/java/org/opensearch/analytics/schema/OpenSearchSchemaBuilder.java [118-120] RelDataTypeFactory.Builder builder = typeFactory.builder(); addLeafFields(builder, typeFactory, properties, ""); +if (builder.getFieldCount() == 0) { + builder.add("_dummy", typeFactory.createSqlType(SqlTypeName.VARCHAR)); +} return builder.build(); Suggestion importance[1-10]: 5 __ Why: While handling empty schemas could prevent issues, the suggestion assumes getFieldCount() exists on the builder (which may not be standard Calcite API) and adds a dummy field which may not be the desired behavior. The concern is valid but the implementation needs verification.	Low

[github-actions]

Persistent review updated to latest commit 2a375f5

[github-actions]

✅ Gradle check result for 2a375f5: SUCCESS

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.50%. Comparing base (d09b185) to head (2a375f5).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #21574      +/-   ##
============================================
+ Coverage     73.45%   73.50%   +0.05%     
- Complexity    74591    74636      +45     
============================================
  Files          5980     5980              
  Lines        338779   338777       -2     
  Branches      48848    48848              
============================================
+ Hits         248849   249030     +181     
+ Misses        70079    69862     -217     
- Partials      19851    19885      +34     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.