[analytics-engine] Wire PPL spath end-to-end through the analytics-engine route

[ahkcs]

Summary

Closes the analytics-engine gap for the PPL spath command. The path-mode variant (spath path=...) already worked via the existing json_extract wiring; this PR adds the auto-extract mode (spath input=doc → JSON_EXTRACT_ALL returning MAP<VARCHAR, VARCHAR>) and its downstream operators (ITEM lookup, WHERE on extracted values), plus a self-contained QA IT.

Pass rate

IT	Before	After
sql/integ-test/.../CalcitePPLSpathCommandIT (analytics-engine route)	0 / 16	16 / 16
sql/integ-test/.../CalcitePPLSpathCommandIT (default v2 / Calcite route)	16 / 16	16 / 16 (no regression)
sandbox/qa/analytics-engine-rest/.../SpathCommandIT (new)	—	16 / 16

Analytics-engine route uses -Dtests.analytics.force_routing=true -Dtests.analytics.parquet_indices=true on :integ-test:integTestRemote.

Baseline failure modes (before this PR):

• 15 tests: OpenSearchProjectRule.annotateExpr → IllegalStateException: No backend supports scalar function [JSON_EXTRACT_ALL] among [datafusion].
• 1 test (testSimpleSpath): EngineBackedIndexer.acquireReader → UnsupportedOperationException — a test-infra issue, fixed on the SQL plugin side in opensearch-project/sql#5441.

Changes

1. json_extract_all Rust UDF

sandbox/plugins/analytics-backend-datafusion/rust/src/udf/json_extract_all.rs — ~550 lines plus 16 unit tests. Returns Arrow Map<Utf8, Utf8>; mirrors JsonExtractAllFunctionImpl's legacy contract:

• Nested objects flatten to dot-paths (user.name).
• Arrays append a {} segment (tags{}).
• Duplicate logical keys merge into a Java-List.toString() rendering ([a, b, c]).
• JSON nulls render as the literal string "null".
• Top-level scalar / null / empty / whitespace input → NULL map.
• Malformed JSON → empty map (legacy swallowed the parse error).

2. SPI enum additions in analytics-framework

• ScalarFunction.JSON_EXTRACT_ALL enum constant.
• FieldType.MAP enum constant + case MAP -> FieldType.MAP in fromSqlTypeName.

3. Capability registrations in DataFusionAnalyticsBackendPlugin

• New MAP_RETURNING_PROJECT_OPS set (mirrors ARRAY_RETURNING_PROJECT_OPS) registered with FieldType.MAP. Required because OpenSearchProjectRule.resolveScalarViableBackends keys on the call's return type, and JSON_EXTRACT_ALL's MAP<VARCHAR, VARCHAR> return wouldn't match SUPPORTED_FIELD_TYPES.
• STANDARD_FILTER_OPS registered against FieldType.MAP so where doc.user.name = 'John' (which references the underlying MAP column through ITEM) survives the filter-rule's field-index-keyed viability check.
• Adapter binding ScalarFunction.JSON_EXTRACT_ALL → JsonExtractAllAdapter.

4. Substrait wiring

• opensearch_scalar_functions.yaml — entries for json_extract_all and map_extract.
• DataFusionFragmentConvertor.ADDITIONAL_SCALAR_SIGS — function mappings for both names.
• JsonFunctionAdapters.JsonExtractAllAdapter — name-mapping adapter.

5. ITEM(Map, key) dispatch in ArrayElementAdapter

PPL's result.user.name lowers to ITEM(JSON_EXTRACT_ALL(doc), 'user.name'). Two transforms apply for the MAP-input branch:

• Route to map_extract (DataFusion's native map accessor) instead of array_element. map_extract returns List<value> because maps semantically permit duplicate keys, so we wrap the call in array_element(..., 1) to project the singleton list back to a scalar.
• Coerce the lookup key (CHAR(N) literal) to VARCHAR before emission so it unifies with the substrait any1 type-variable binding the YAML declares.

6. ArrowValues.MapVector flattening in analytics-engine

Arrow MapVector is laid out as List<Struct{key, value}>, so MapVector.getObject(i) returns a JsonStringArrayList of entry structs rather than a proper map. This change reassembles entries into a LinkedHashMap<String, Object> with Text → String normalization on both keys and values, so the SQL-plugin response marshaller sees the same shape as a legacy v2 Map<String, Object> column.

7. QA-side SpathCommandIT

Under sandbox/qa/analytics-engine-rest/. Mirrors CalcitePPLSpathCommandIT one test method to one, sends queries via POST /_analytics/ppl, no SQL-plugin dependency. Verifies the full spath surface end-to-end:

• Both modes (path / auto-extract).
• ITEM-on-MAP through eval / where / stats / sort.
• Edge cases (empty / malformed JSON, duplicate keys, array suffix, null input).

Four small datasets under resources/datasets/spath_{simple,auto,cmd,null}/.

Knock-on coverage

Every piece in this PR is reusable beyond spath:

• MAP_RETURNING_PROJECT_OPS + MAP filter capability are generic for any future PPL function emitting a Calcite MAP RelDataType.
• ArrayElementAdapter's ITEM-on-MAP branch + the map_extract YAML entry handle every result['key'] / result.field access on a map column, not just spath's.
• ArrowValues.MapVector flattening unblocks any UDF returning Map<Utf8, Utf8> from the analytics-engine route.

Paired SQL plugin PR

opensearch-project/sql#5441 — test-infra fix that routes the v2 / Calcite IT's test indices through TestUtils.createIndexByRestClient, so the tests.analytics.parquet_indices=true toggle has a chance to inject parquet/composite settings before any doc PUTs land.

How to verify

QA-side IT (no SQL plugin needed):

./gradlew :sandbox:qa:analytics-engine-rest:integTest \
  -Dsandbox.enabled=true --tests "*SpathCommandIT"

End-to-end through the SQL plugin (requires #5441 applied):

# Start the cluster with all sandbox plugins. The transport.stream feature
# gate is required when analytics-engine and the SQL plugin co-install.
./gradlew :run -Dsandbox.enabled=true \
  -Dopensearch.experimental.feature.transport.stream.enabled=true \
  -PinstalledPlugins="['opensearch-job-scheduler:3.7.0.0-SNAPSHOT', \
    'arrow-flight-rpc', 'analytics-engine', 'parquet-data-format', \
    'analytics-backend-datafusion', 'analytics-backend-lucene', \
    'composite-engine', 'opensearch-sql-plugin:3.7.0.0-SNAPSHOT']"

# In the SQL plugin checkout
./gradlew :integ-test:integTestRemote \
  -Dtests.rest.cluster=localhost:9200 \
  -Dtests.cluster=localhost:9300 \
  -Dtests.clustername=runTask \
  -Dtests.analytics.force_routing=true \
  -Dtests.analytics.parquet_indices=true \
  --tests "org.opensearch.sql.calcite.remote.CalcitePPLSpathCommandIT"

Also addressed

Rebased onto current main. Per @mch2's review, no gradle build files are modified by this PR — the streaming-transport feature gate needed for the analytics-engine + SQL-plugin co-install must be passed at run time as -Dopensearch.experimental.feature.transport.stream.enabled=true.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 4dc07cb)

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 When the container is a MAP and the key type does not match, the code creates a nullable key type but does not verify that the cast succeeds or that the key is actually castable. If the cast fails at runtime (e.g., incompatible types), the query will error. This is a narrow scenario but could occur if a caller constructs a MAP with a non-VARCHAR key type and attempts a string-literal lookup. if (!key.getType().equals(keyType)) { RelDataType nullableKeyType = typeFactory.createTypeWithNullability(keyType, key.getType().isNullable()); key = rexBuilder.makeCast(nullableKeyType, key, true, false); } Possible Issue The MapVector branch casts vector.getObject(index) to List<?> without verifying the cast succeeds. If the Arrow MapVector implementation changes or returns a different type, this will throw a ClassCastException at runtime. The instanceof check on MapVector does not guarantee the getObject return type. if (vector instanceof MapVector && vector.getObject(index) instanceof List<?> entries) { LinkedHashMap<String, Object> map = new LinkedHashMap<>(); for (Object entry : entries) { if (!(entry instanceof Map<?, ?> e)) continue; Object k = e.get(MapVector.KEY_NAME); Object v = e.get(MapVector.VALUE_NAME); map.put(k instanceof Text t ? t.toString() : String.valueOf(k), v instanceof Text t ? t.toString() : v); } return map; }

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 4dc07cb

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Add null checks for MAP types Verify that container.getType().getKeyType() and container.getType().getValueType() do not return null before dereferencing. If the MAP type is malformed or incomplete, these methods could return null, leading to a NullPointerException in subsequent operations. sandbox/plugins/analytics-backend-datafusion/src/main/java/org/opensearch/be/datafusion/ArrayElementAdapter.java [102-113] if (container.getType().getSqlTypeName() == SqlTypeName.MAP) { RelDataType keyType = container.getType().getKeyType(); RelDataType valueType = container.getType().getValueType(); + if (keyType == null || valueType == null) { + return rexBuilder.makeCall(original.getType(), LOCAL_ARRAY_ELEMENT_OP, operands); + } ... if (!key.getType().equals(keyType)) { RelDataType nullableKeyType = typeFactory.createTypeWithNullability(keyType, key.getType().isNullable()); key = rexBuilder.makeCast(nullableKeyType, key, true, false); } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a potential NullPointerException if getKeyType() or getValueType() return null. However, this is a defensive check that may not be strictly necessary if the MAP type is always well-formed in practice. The fallback behavior (returning the original call) is reasonable.	Medium
General	Skip null keys in map conversion Handle the case where k is null before calling String.valueOf(k) in the map key conversion. A null key will produce the string "null" which may cause unexpected behavior or key collisions in the resulting map. sandbox/plugins/analytics-engine/src/main/java/org/opensearch/analytics/exec/ArrowValues.java [54-62] if (vector instanceof MapVector && vector.getObject(index) instanceof List<?> entries) { LinkedHashMap<String, Object> map = new LinkedHashMap<>(); for (Object entry : entries) { if (!(entry instanceof Map<?, ?> e)) continue; Object k = e.get(MapVector.KEY_NAME); Object v = e.get(MapVector.VALUE_NAME); + if (k == null) continue; map.put(k instanceof Text t ? t.toString() : String.valueOf(k), v instanceof Text t ? t.toString() : v); } return map; } Suggestion importance[1-10]: 6 __ Why: The suggestion addresses a potential issue where null keys could produce the string "null" and cause key collisions. However, Arrow's MapVector typically enforces non-null keys at the schema level (the key field is declared nullable=false in the MAP type definition), so this may be a rare edge case.	Low

---

Previous suggestions

Suggestions up to commit a5ac2f9

Category	Suggestion	Impact
Possible issue	Prevent null key insertion into map Handle the case where k is null before calling String.valueOf(k) or using it as a map key. A null key will cause LinkedHashMap.put() to throw a NullPointerException, breaking the response marshalling for MAP columns with null keys. sandbox/plugins/analytics-engine/src/main/java/org/opensearch/analytics/exec/ArrowValues.java [54-63] if (vector instanceof MapVector && vector.getObject(index) instanceof List<?> entries) { LinkedHashMap<String, Object> map = new LinkedHashMap<>(); for (Object entry : entries) { if (!(entry instanceof Map<?, ?> e)) continue; Object k = e.get(MapVector.KEY_NAME); + if (k == null) continue; Object v = e.get(MapVector.VALUE_NAME); map.put(k instanceof Text t ? t.toString() : String.valueOf(k), v instanceof Text t ? t.toString() : v); } return map; } Suggestion importance[1-10]: 8 __ Why: Skipping null keys prevents NullPointerException when inserting into LinkedHashMap. Arrow's MAP specification allows null keys in some contexts, so this is a valid edge case that could break response marshalling if not handled.	Medium
	Add null checks for MAP types Add null-safety checks before calling getKeyType() and getValueType() on the MAP type. If the container type is malformed or the key/value types are unexpectedly null, the adapter will throw a NullPointerException at runtime. sandbox/plugins/analytics-backend-datafusion/src/main/java/org/opensearch/be/datafusion/ArrayElementAdapter.java [102-113] if (container.getType().getSqlTypeName() == SqlTypeName.MAP) { RelDataType keyType = container.getType().getKeyType(); RelDataType valueType = container.getType().getValueType(); + if (keyType == null || valueType == null) { + return rexBuilder.makeCall(original.getType(), LOCAL_ARRAY_ELEMENT_OP, operands); + } ... if (!key.getType().equals(keyType)) { RelDataType nullableKeyType = typeFactory.createTypeWithNullability(keyType, key.getType().isNullable()); key = rexBuilder.makeCast(nullableKeyType, key, true, false); } Suggestion importance[1-10]: 7 __ Why: Adding null checks for keyType and valueType prevents potential NullPointerException at runtime if the MAP type is malformed. However, Calcite's type system typically ensures well-formed types, making this a defensive measure rather than addressing a likely bug.	Medium

Suggestions up to commit 2c8bf2b

Category	Suggestion	Impact
General	Remove unreachable conditional branch The conditional logic for inserting dots after array markers is redundant. The else if s.ends_with(ARRAY_MARKER) branch will never execute because it's nested inside the else block that already checks !s.ends_with(ARRAY_MARKER). Simplify to avoid confusion. sandbox/plugins/analytics-backend-datafusion/rust/src/udf/json_extract_all.rs [305-319] fn build_path(path: &[String]) -> String { let mut s = String::with_capacity(path.iter().map(|p| p.len() + 1).sum::<usize>()); for segment in path { if segment == ARRAY_MARKER { s.push_str(ARRAY_MARKER); } else { - if !s.is_empty() && !s.ends_with(ARRAY_MARKER) { - s.push('.'); - } else if s.ends_with(ARRAY_MARKER) { + if !s.is_empty() { s.push('.'); } s.push_str(segment); } } s } Suggestion importance[1-10]: 8 __ Why: The else if s.ends_with(ARRAY_MARKER) branch is unreachable due to the parent condition checking !s.ends_with(ARRAY_MARKER). However, the suggested fix incorrectly removes the logic for inserting dots after array markers (e.g., tags{}.field), which would break path construction for nested structures.	Medium
Possible issue	Guard against null map keys Handle the case where k is null before calling String.valueOf(k) or using it as a map key. A null key will cause LinkedHashMap.put() to throw a NullPointerException, breaking the response marshalling for MAP columns with null keys. sandbox/plugins/analytics-engine/src/main/java/org/opensearch/analytics/exec/ArrowValues.java [54-62] if (vector instanceof MapVector && vector.getObject(index) instanceof List<?> entries) { LinkedHashMap<String, Object> map = new LinkedHashMap<>(); for (Object entry : entries) { if (!(entry instanceof Map<?, ?> e)) continue; Object k = e.get(MapVector.KEY_NAME); + if (k == null) continue; Object v = e.get(MapVector.VALUE_NAME); map.put(k instanceof Text t ? t.toString() : String.valueOf(k), v instanceof Text t ? t.toString() : v); } return map; } Suggestion importance[1-10]: 7 __ Why: Arrow's MapVector specification requires non-null keys (the key field is declared nullable=false in map_data_type()), but adding a defensive check prevents potential runtime failures if the upstream data violates this contract.	Medium
	Add null checks for MAP types Add null-safety checks before calling getKeyType() and getValueType() on the MAP type. If the container type is malformed or the key/value types are unexpectedly null, the adapter will throw a NullPointerException at runtime. sandbox/plugins/analytics-backend-datafusion/src/main/java/org/opensearch/be/datafusion/ArrayElementAdapter.java [102-104] if (container.getType().getSqlTypeName() == SqlTypeName.MAP) { RelDataType keyType = container.getType().getKeyType(); RelDataType valueType = container.getType().getValueType(); + if (keyType == null || valueType == null) { + return rexBuilder.makeCall(original.getType(), LOCAL_ARRAY_ELEMENT_OP, operands); + } ... if (!key.getType().equals(keyType)) { RelDataType nullableKeyType = typeFactory.createTypeWithNullability(keyType, key.getType().isNullable()); key = rexBuilder.makeCast(nullableKeyType, key, true, false); } Suggestion importance[1-10]: 5 __ Why: While adding null checks for keyType and valueType improves defensive programming, Calcite's type system guarantees that MAP types have both key and value types. The fallback to LOCAL_ARRAY_ELEMENT_OP may not be semantically correct for MAP inputs.	Low

Suggestions up to commit 594f756

Category	Suggestion	Impact
Possible issue	Validate MAP key/value types The code assumes getKeyType() and getValueType() return non-null values for MAP types. If the MAP type is malformed or incompletely initialized, these methods could return null, causing a NullPointerException when calling methods on keyType or valueType. Add null checks before using these types. sandbox/plugins/analytics-backend-datafusion/src/main/java/org/opensearch/be/datafusion/ArrayElementAdapter.java [102-113] if (container.getType().getSqlTypeName() == SqlTypeName.MAP) { RelDataType keyType = container.getType().getKeyType(); RelDataType valueType = container.getType().getValueType(); + if (keyType == null || valueType == null) { + throw new IllegalStateException("MAP type must have non-null key and value types"); + } ... if (!key.getType().equals(keyType)) { RelDataType nullableKeyType = typeFactory.createTypeWithNullability(keyType, key.getType().isNullable()); key = rexBuilder.makeCast(nullableKeyType, key, true, false); } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that getKeyType() and getValueType() could potentially return null for malformed MAP types. Adding null checks would prevent potential NullPointerExceptions and improve robustness, though this scenario is unlikely in normal operation.	Medium
General	Handle null map keys explicitly The code uses String.valueOf(k) as a fallback when the key is not a Text instance. If k is null, String.valueOf(null) returns the string "null", which could create a valid map entry with a literal "null" key instead of handling the null case explicitly. Consider validating that keys are non-null or handling null keys appropriately. sandbox/plugins/analytics-engine/src/main/java/org/opensearch/analytics/exec/ArrowValues.java [54-62] if (vector instanceof MapVector && vector.getObject(index) instanceof List<?> entries) { LinkedHashMap<String, Object> map = new LinkedHashMap<>(); for (Object entry : entries) { if (!(entry instanceof Map<?, ?> e)) continue; Object k = e.get(MapVector.KEY_NAME); + if (k == null) continue; Object v = e.get(MapVector.VALUE_NAME); map.put(k instanceof Text t ? t.toString() : String.valueOf(k), v instanceof Text t ? t.toString() : v); } return map; } Suggestion importance[1-10]: 6 __ Why: The suggestion identifies a valid edge case where null keys would be converted to the string "null". However, the current behavior may be intentional to match legacy semantics, and the impact is limited since Arrow MapVector keys are typically non-null by schema definition.	Low

Suggestions up to commit ca7a567

Category	Suggestion	Impact
Possible issue	Validate MAP type components exist Verify that container.getType().getKeyType() and container.getType().getValueType() do not return null before dereferencing. If the MAP type is malformed or incomplete, these methods could return null, leading to a NullPointerException when keyType or valueType are used in subsequent operations. sandbox/plugins/analytics-backend-datafusion/src/main/java/org/opensearch/be/datafusion/ArrayElementAdapter.java [102-119] if (container.getType().getSqlTypeName() == SqlTypeName.MAP) { RelDataType keyType = container.getType().getKeyType(); RelDataType valueType = container.getType().getValueType(); + if (keyType == null || valueType == null) { + throw new IllegalStateException("MAP type missing key or value type"); + } ... RelDataType nullableValueType = typeFactory.createTypeWithNullability(valueType, true); RelDataType listOfValue = typeFactory.createArrayType(nullableValueType, -1); RexNode mapResult = rexBuilder.makeCall(listOfValue, LOCAL_MAP_EXTRACT_OP, List.of(container, key)); RexNode one = rexBuilder.makeLiteral(java.math.BigDecimal.ONE, typeFactory.createSqlType(SqlTypeName.BIGINT), false); return rexBuilder.makeCall(original.getType(), LOCAL_ARRAY_ELEMENT_OP, List.of(mapResult, one)); } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a potential NPE risk if getKeyType() or getValueType() return null. However, this is a defensive check that may not be strictly necessary if the type system guarantees non-null types for well-formed MAP types.	Medium
General	Skip map entries with null keys Handle the case where e.get(MapVector.KEY_NAME) returns null. If a map entry has a null key, String.valueOf(k) will produce the string "null", which could collide with legitimate keys. Consider skipping entries with null keys or throwing an exception to prevent silent data corruption. sandbox/plugins/analytics-engine/src/main/java/org/opensearch/analytics/exec/ArrowValues.java [54-63] if (vector instanceof MapVector && vector.getObject(index) instanceof List<?> entries) { LinkedHashMap<String, Object> map = new LinkedHashMap<>(); for (Object entry : entries) { if (!(entry instanceof Map<?, ?> e)) continue; Object k = e.get(MapVector.KEY_NAME); + if (k == null) continue; Object v = e.get(MapVector.VALUE_NAME); map.put(k instanceof Text t ? t.toString() : String.valueOf(k), v instanceof Text t ? t.toString() : v); } return map; } Suggestion importance[1-10]: 6 __ Why: Valid concern about null keys potentially causing collisions with the string "null". Skipping null-key entries is a reasonable defensive measure, though the Arrow MapVector specification may already prevent null keys.	Low
	Optimize duplicate-key insertion performance The linear search in insert has O(n) complexity for each insertion, resulting in O(n²) worst-case performance when many duplicate keys exist. For large JSON documents with many collisions, consider using a HashMap or IndexMap for O(1) lookups during insertion, then converting to Vec at the end if order preservation is required. sandbox/plugins/analytics-backend-datafusion/rust/src/udf/json_extract_all.rs [294-300] +// Use IndexMap for O(1) lookups during insertion +use indexmap::IndexMap; +type OrderedEntries = IndexMap<String, Slot>; + fn insert(out: &mut OrderedEntries, key: String, value: Option<String>) { - if let Some((_, existing)) = out.iter_mut().find(|(k, _)| *k == key) { - existing.extend(value); - } else { - out.push((key, Slot::Single(value))); - } + out.entry(key) + .and_modify(|existing| existing.extend(value.clone())) + .or_insert(Slot::Single(value)); } Suggestion importance[1-10]: 5 __ Why: The performance concern is valid for pathological cases with many duplicate keys. However, the PR comments explicitly note that duplicate-key insertion is rare in realistic inputs, and the current O(n) approach avoids adding indexmap as a dependency.	Low

Suggestions up to commit f25feda

Category	Suggestion	Impact
General	Handle null map keys properly Handle the case where k is null before calling String.valueOf(k). If the key is null, String.valueOf(null) returns the string "null", which may not be the intended behavior for a map key. Consider throwing an exception or skipping the entry to prevent invalid map states. sandbox/plugins/analytics-engine/src/main/java/org/opensearch/analytics/exec/ArrowValues.java [54-64] if (vector instanceof MapVector && vector.getObject(index) instanceof List<?> entries) { LinkedHashMap<String, Object> map = new LinkedHashMap<>(); for (Object entry : entries) { if (!(entry instanceof Map<?, ?> e)) continue; Object k = e.get(MapVector.KEY_NAME); Object v = e.get(MapVector.VALUE_NAME); + if (k == null) continue; map.put( k instanceof Text t ? t.toString() : String.valueOf(k), v instanceof Text t ? t.toString() : v); } return map; } Suggestion importance[1-10]: 7 __ Why: Valid concern about null keys in maps. Skipping entries with null keys (as suggested) is a reasonable defensive measure that prevents invalid map states. The suggestion correctly identifies that String.valueOf(null) produces "null" which may not be intended for map keys.	Medium
	Simplify redundant conditional branches The conditional logic for adding dots has redundant branches. When s.ends_with(ARRAY_MARKER) is true, a dot is always added, making the first condition's !s.ends_with(ARRAY_MARKER) check unnecessary. Simplify to avoid confusion and potential logic errors. sandbox/plugins/analytics-backend-datafusion/rust/src/udf/json_extract_all.rs [305-320] fn build_path(path: &[String]) -> String { let mut s = String::with_capacity(path.iter().map(|p| p.len() + 1).sum::<usize>()); for segment in path { if segment == ARRAY_MARKER { s.push_str(ARRAY_MARKER); } else { - if !s.is_empty() && !s.ends_with(ARRAY_MARKER) { - s.push('.'); - } else if s.ends_with(ARRAY_MARKER) { + if !s.is_empty() { s.push('.'); } s.push_str(segment); } } s } Suggestion importance[1-10]: 3 __ Why: The suggestion misunderstands the logic. The original code intentionally adds a dot after ARRAY_MARKER segments (line 314), which is different from adding a dot when the string is non-empty and doesn't end with ARRAY_MARKER. The simplified version would break the path construction for array markers. The suggestion is incorrect.	Low
Possible issue	Add null checks for MAP types Verify that container.getType().getKeyType() and container.getType().getValueType() do not return null before dereferencing. If the MAP type is malformed or incomplete, these methods could return null, leading to a NullPointerException when calling methods on keyType or valueType. sandbox/plugins/analytics-backend-datafusion/src/main/java/org/opensearch/be/datafusion/ArrayElementAdapter.java [102-112] if (container.getType().getSqlTypeName() == SqlTypeName.MAP) { RelDataType keyType = container.getType().getKeyType(); RelDataType valueType = container.getType().getValueType(); + if (keyType == null || valueType == null) { + throw new IllegalStateException("MAP type missing key or value type"); + } ... if (!key.getType().equals(keyType)) { RelDataType nullableKeyType = typeFactory.createTypeWithNullability(keyType, key.getType().isNullable()); key = rexBuilder.makeCast(nullableKeyType, key, true, false); } Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies a potential NPE risk when getKeyType() or getValueType() return null. However, this is a defensive check that may not be necessary if the type system guarantees non-null types for well-formed MAP types. The score reflects moderate importance for robustness.	Low

[github-actions]

✅ Gradle check result for dc48574: SUCCESS

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.42%. Comparing base (74a2766) to head (4dc07cb).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #21664      +/-   ##
============================================
- Coverage     73.45%   73.42%   -0.03%     
+ Complexity    74811    74808       -3     
============================================
  Files          5997     5997              
  Lines        339688   339688              
  Branches      48961    48961              
============================================
- Hits         249515   249418      -97     
- Misses        70305    70385      +80     
- Partials      19868    19885      +17     

☔ 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.

[github-actions]

Persistent review updated to latest commit d5ece62

[github-actions]

Persistent review updated to latest commit f25feda

[github-actions]

✅ Gradle check result for f25feda: SUCCESS

[github-actions]

Persistent review updated to latest commit ca7a567

[github-actions]

✅ Gradle check result for ca7a567: SUCCESS

[github-actions]

Persistent review updated to latest commit 594f756

[github-actions]

Persistent review updated to latest commit 2c8bf2b

[github-actions]

✅ Gradle check result for 2c8bf2b: SUCCESS

[ahkcs]

Done @mch2 — dropped the afterEvaluate block in sandbox/plugins/analytics-engine/build.gradle. PR no longer touches any gradle file. The opensearch.experimental.feature.transport.stream.enabled=true flag is now expected as a CLI -D arg when running the co-install (docs updated).

[github-actions]

Persistent review updated to latest commit a5ac2f9

[github-actions]

Persistent review updated to latest commit 4dc07cb

[github-actions]

✅ Gradle check result for 4dc07cb: SUCCESS