Support PPL foreach command

[songkant-aws]

Description

Adds the PPL foreach command (Splunk-compatible), which runs a templated eval for each field in a field list, each element of a multivalue (array) field, or each element of a JSON array.

# multifield: double every matching field
source=idx | foreach value_* [ eval <<FIELD>>_double = <<FIELD>> * 2 ]

# multivalue: fold array elements into an accumulator
source=idx | eval nums = array(1,2,3), total = 0
           | foreach mode=multivalue nums [ eval total = total + <<ITEM>> ]

# json_array: parse JSON array text (literal or field) and iterate
source=idx | eval total = 0
           | foreach mode=json_array '[1,2,3]' [ eval total = total + <<ITEM>> ]

All four Splunk modes are supported: multifield (default), multivalue, json_array, and auto_collections (runtime detection). Placeholders <<FIELD>>/<<MATCHSTR>> (multifield) and <<ITEM>>/<<ITER>> (collection modes) are supported, with fieldstr/matchstr/itemstr/iterstr rename options.

Design

• Multifield mode expands each eval clause once per matching field via placeholder bindings on CalcitePlanContext; <<FIELD>> resolves to the current row field, <<MATCHSTR>> to the wildcard-captured segment.
• Collection modes rewrite each eval clause into a reduce call over the collection. Elements are packed into internal pairs [item, iter, captured-field...] by a foreach_pair_collection UDF so the two-parameter reduce lambda can reach the loop item, the loop index, and any referenced row fields; foreach_pair_item(pair, slot) extracts a slot with its plan-time type attached. Bindings are staged on the context and only activate inside the cloned lambda context, so the reduce call's own arguments resolve against the row normally.
• json_array element typing: json_array(...) calls and string literals are inspected at plan time (mixed string/number arrays rejected); for opaque expressions (a field holding JSON text — the primary Splunk use), the type is inferred from usage: arithmetic on <<ITEM>> selects DOUBLE, otherwise VARCHAR.
• Planning lives in a dedicated ForeachPlanner class rather than growing CalciteRelNodeVisitor.

Behavior verified against Splunk 10.4.0

Scenario	Splunk	This PR
mv field + multivalue	6	6 ✓
JSON-text field + json_array (sum of "[10,20,30]")	60	60 ✓
json_array mode fed a real array	silent no-op	iterates (more permissive, pinned in IT)
multivalue mode fed JSON text	silent no-op	plan-time error (louder, pinned in IT)

Known limitation (documented in tests and user doc): fields whose mapping is a scalar type (e.g. long) holding arrays are rejected by multivalue mode, because OpenSearch mappings do not declare array-ness and the plan-time type is scalar. Nested-typed fields map to ARRAY<ANY> and iterate correctly.

Related Issues

N/A (new PPL command)

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.

[songkant-aws]

Note for reviewers: two items from the new-command checklist are still pending and I'll push them to this PR shortly:

• PPLQueryDataAnonymizer.visitForeach (query anonymization)
• v2 Analyzer.visitForeach throwing the standard "only for Calcite" exception (currently the v2 path behavior is undefined)

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit e293461)

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 In wildcardSegments, the method returns null when the pattern does not match, but returns an empty list when the pattern matches without wildcards. The caller in multifieldBindings iterates patterns and breaks on the first non-null result. If a pattern matches without wildcards, the loop breaks and binds an empty segment list, but subsequent patterns that might have wildcards are never checked. This causes incorrect bindings when multiple patterns are provided and an earlier pattern is an exact match. private List<String> wildcardSegments(String pattern, String fieldName) { if (!WildcardUtils.matchesWildcardPattern(pattern, fieldName)) { return null; } if (!WildcardUtils.containsWildcard(pattern)) { return List.of(); } Matcher matcher = Pattern.compile(WildcardUtils.convertWildcardPatternToRegex(pattern)).matcher(fieldName); if (!matcher.matches()) { return null; } List<String> segments = new ArrayList<>(); for (int i = 1; i <= matcher.groupCount(); i++) { segments.add(matcher.group(i)); } return segments; Possible Issue In toList, when the input is an Object[] array, the method wraps it with List.of(array), creating a list containing a single array element instead of a list of the array's elements. This breaks foreach collection modes when the input is an Object array, as the iteration will see one array object instead of individual elements. Should use Arrays.asList(array) instead. private static List<?> toList(Object value) { if (value instanceof List<?> list) { return list; } if (value instanceof Object[] array) { return List.of(array); } throw new IllegalArgumentException("foreach pair collection requires an array input"); } Possible Issue In capturedFields, the method collects row fields referenced by eval clauses but excludes targetAliases (the foreach output field names). However, if a target alias shadows an existing row field name, and the expression references that field, the reference is excluded from capture. The lambda will then fail to resolve the field because it was not packed into the pair. This occurs when a foreach output overwrites an input field that the expression also reads. private List<RelDataTypeField> capturedFields( List<ForeachEvalClause> evalClauses, CalcitePlanContext context, String itemName, String iterName, List<String> targetAliases) { Set<String> excluded = Stream.concat( Stream.of(itemName, iterName, PLACEHOLDER_ITEM, PLACEHOLDER_ITER, PAIR_VAR), targetAliases.stream()) .map(name -> name.toUpperCase(Locale.ROOT)) .collect(Collectors.toSet()); Map<String, RelDataTypeField> rowFields = context.relBuilder.peek().getRowType().getFieldList().stream() .collect( Collectors.toMap( field -> field.getName().toUpperCase(Locale.ROOT), field -> field, (left, right) -> left, LinkedHashMap::new)); Map<String, RelDataTypeField> captured = new LinkedHashMap<>(); evalClauses.forEach( clause -> collectFieldReferences(clause.getExpression(), excluded, rowFields, captured)); return new ArrayList<>(captured.values()); }

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to e293461

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Log JSON parsing exceptions The method catches JsonSyntaxException but silently ignores it, returning VARCHAR as a fallback. This masks potential JSON parsing errors that could indicate data quality issues. Consider logging the exception to aid debugging. core/src/main/java/org/opensearch/sql/calcite/ForeachPlanner.java [310-324] private SqlTypeName jsonElementType( UnresolvedExpression collection, CalcitePlanContext context, List<ForeachEvalClause> evalClauses, String itemName) { if (collection instanceof Function function && BuiltinFunctionName.JSON_ARRAY .getName() .getFunctionName() .equalsIgnoreCase(function.getFuncName())) { return elementTypeOf( function.getFuncArgs().stream() .map(arg -> rexVisitor.analyze(arg, context).getType()) .map(RelDataType::getFamily) .collect(Collectors.toSet())); } if (collection instanceof Literal literal && literal.getType() == DataType.STRING) { try { List<?> values = gson.fromJson(String.valueOf(literal.getValue()), List.class); if (values != null) { return elementTypeOf( values.stream() .filter(Objects::nonNull) .map(v -> v instanceof Number ? SqlTypeFamily.NUMERIC : SqlTypeFamily.CHARACTER) .collect(Collectors.toSet())); } - } catch (JsonSyntaxException ignored) { - ... + } catch (JsonSyntaxException e) { + log.debug("Failed to parse JSON literal for type inference: {}", literal.getValue(), e); } return SqlTypeName.VARCHAR; } return itemUsedInArithmetic(evalClauses, itemName) ? SqlTypeName.DOUBLE : SqlTypeName.VARCHAR; } Suggestion importance[1-10]: 5 __ Why: Adding logging for the caught JsonSyntaxException would aid debugging and improve observability. However, the comment in the code indicates that malformed JSON literals are expected to return null at runtime, suggesting this is intentional behavior rather than an error condition. The improvement is moderate.	Low
	Handle conversion exceptions gracefully The method does not validate that toList successfully converts the input. If toList throws an exception for unsupported types, it will propagate unchecked. Add explicit validation or handle the exception to prevent unexpected runtime failures. core/src/main/java/org/opensearch/sql/expression/function/CollectionUDF/ForeachPairCollectionFunctionImpl.java [57-73] public static Object eval(Object... args) { if (args.length == 0 || args[0] == null) { return null; } - List<?> source = toList(args[0]); - List<Object[]> pairs = new ArrayList<>(); - for (int i = 0; i < source.size(); i++) { - Object[] pair = new Object[args.length + 1]; - pair[0] = source.get(i); - pair[1] = i; - for (int j = 1; j < args.length; j++) { - pair[j + 1] = args[j]; + try { + List<?> source = toList(args[0]); + List<Object[]> pairs = new ArrayList<>(); + for (int i = 0; i < source.size(); i++) { + Object[] pair = new Object[args.length + 1]; + pair[0] = source.get(i); + pair[1] = i; + for (int j = 1; j < args.length; j++) { + pair[j + 1] = args[j]; + } + pairs.add(pair); } - pairs.add(pair); + return pairs; + } catch (IllegalArgumentException e) { + return null; } - return pairs; } Suggestion importance[1-10]: 4 __ Why: The suggestion to catch IllegalArgumentException from toList and return null improves robustness. However, the toList method already throws IllegalArgumentException with a descriptive message, and catching it here would suppress that information. The improvement is minor and may hide useful error details.	Low
	Remove redundant pattern matching check The method performs redundant pattern matching. After checking matchesWildcardPattern returns true, the subsequent matcher.matches() check is unnecessary and will always succeed. Remove the second null check to avoid redundant regex evaluation. core/src/main/java/org/opensearch/sql/calcite/ForeachPlanner.java [153-170] private List<String> wildcardSegments(String pattern, String fieldName) { if (!WildcardUtils.matchesWildcardPattern(pattern, fieldName)) { return null; } if (!WildcardUtils.containsWildcard(pattern)) { return List.of(); } Matcher matcher = Pattern.compile(WildcardUtils.convertWildcardPatternToRegex(pattern)).matcher(fieldName); - if (!matcher.matches()) { - return null; - } + matcher.matches(); List<String> segments = new ArrayList<>(); for (int i = 1; i <= matcher.groupCount(); i++) { segments.add(matcher.group(i)); } return segments; } Suggestion importance[1-10]: 3 __ Why: The suggestion identifies a redundant check, but the second matcher.matches() call is necessary to populate the matcher's groups before calling matcher.group(i). Removing the null check without calling matches() would break the code, and calling matches() without checking its result doesn't eliminate the redundancy meaningfully.	Low

---

Previous suggestions

Suggestions up to commit 2c04b27

Category	Suggestion	Impact
Possible issue	Validate type before casting to List The cast (List<?>) args[0] assumes args[0] is a List if it's not an Object[], but this assumption is not validated. If args[0] is neither type, a ClassCastException will occur at runtime. Add explicit type checking or handle unexpected types gracefully. core/src/main/java/org/opensearch/sql/expression/function/CollectionUDF/ForeachPairItemFunctionImpl.java [55-62] public static Object eval(Object... args) { if (args.length < 2 || args[0] == null || args[1] == null) { return null; } int index = ((Number) args[1]).intValue(); - List<?> pair = args[0] instanceof Object[] array ? Arrays.asList(array) : (List<?>) args[0]; + List<?> pair; + if (args[0] instanceof Object[] array) { + pair = Arrays.asList(array); + } else if (args[0] instanceof List<?> list) { + pair = list; + } else { + return null; + } return index < 0 || index >= pair.size() ? null : pair.get(index); } Suggestion importance[1-10]: 7 __ Why: The suggestion addresses a potential ClassCastException by adding explicit type checking before casting. This improves robustness by handling unexpected input types gracefully, though the current code may work if the caller guarantees the input type.	Medium
General	Clarify null handling in JSON parsing When gson.fromJson returns null or throws JsonSyntaxException, the method falls through to return VARCHAR. However, if values is null, the code should explicitly return VARCHAR rather than falling through, making the logic clearer and preventing potential issues if the code structure changes. core/src/main/java/org/opensearch/sql/calcite/ForeachPlanner.java [287-326] -private SqlTypeName jsonElementType( - UnresolvedExpression collection, - CalcitePlanContext context, - List<ForeachEvalClause> evalClauses, - String itemName) { - if (collection instanceof Function function - && BuiltinFunctionName.JSON_ARRAY - .getName() - .getFunctionName() - .equalsIgnoreCase(function.getFuncName())) { +if (collection instanceof Literal literal && literal.getType() == DataType.STRING) { + try { + List<?> values = gson.fromJson(String.valueOf(literal.getValue()), List.class); + if (values == null) { + return SqlTypeName.VARCHAR; + } return elementTypeOf( - function.getFuncArgs().stream() - .map(arg -> rexVisitor.analyze(arg, context).getType()) - .map(RelDataType::getFamily) + values.stream() + .filter(Objects::nonNull) + .map(v -> v instanceof Number ? SqlTypeFamily.NUMERIC : SqlTypeFamily.CHARACTER) .collect(Collectors.toSet())); - } - if (collection instanceof Literal literal && literal.getType() == DataType.STRING) { - try { - List<?> values = gson.fromJson(String.valueOf(literal.getValue()), List.class); - if (values != null) { - return elementTypeOf( - values.stream() - .filter(Objects::nonNull) - .map(v -> v instanceof Number ? SqlTypeFamily.NUMERIC : SqlTypeFamily.CHARACTER) - .collect(Collectors.toSet())); - } - } catch (JsonSyntaxException ignored) { - // Malformed JSON literals return null at runtime; type choice is irrelevant. - } + } catch (JsonSyntaxException ignored) { return SqlTypeName.VARCHAR; } - return itemUsedInArithmetic(evalClauses, itemName) ? SqlTypeName.DOUBLE : SqlTypeName.VARCHAR; } Suggestion importance[1-10]: 6 __ Why: The suggestion improves code clarity by making the null handling explicit rather than relying on fall-through logic. This makes the code more maintainable and less prone to errors if the structure changes, though the current implementation is functionally correct.	Low
	Remove redundant pattern matching check The method checks matchesWildcardPattern first, then compiles a regex and checks matcher.matches() again. If the first check passes, the second should always pass. Remove the redundant second null check to avoid confusion and potential logic errors. core/src/main/java/org/opensearch/sql/calcite/ForeachPlanner.java [153-169] private List<String> wildcardSegments(String pattern, String fieldName) { if (!WildcardUtils.matchesWildcardPattern(pattern, fieldName)) { return null; } if (!WildcardUtils.containsWildcard(pattern)) { return List.of(); } Matcher matcher = Pattern.compile(WildcardUtils.convertWildcardPatternToRegex(pattern)).matcher(fieldName); - if (!matcher.matches()) { - return null; + List<String> segments = new ArrayList<>(); + for (int i = 1; i <= matcher.groupCount(); i++) { + segments.add(matcher.group(i)); } - ... + return segments; } Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies a redundant check after matchesWildcardPattern. However, the redundancy may be intentional for defensive programming or due to different implementations of matchesWildcardPattern and regex matching. The impact is minor since it only affects code clarity.	Low
	Handle invalid input types gracefully The method throws IllegalArgumentException for unexpected input types, but the eval method that calls it only checks if args[0] is null, not if it's a valid collection type. This can lead to runtime exceptions. Consider returning null instead of throwing an exception to maintain consistency with other null-handling patterns in the codebase. core/src/main/java/org/opensearch/sql/expression/function/CollectionUDF/ForeachPairCollectionFunctionImpl.java [75-83] private static List<?> toList(Object value) { if (value instanceof List<?> list) { return list; } if (value instanceof Object[] array) { return List.of(array); } - throw new IllegalArgumentException("foreach pair collection requires an array input"); + return null; } Suggestion importance[1-10]: 4 __ Why: The suggestion proposes returning null instead of throwing an exception for invalid input types. While this may improve consistency with other null-handling patterns, throwing an exception for invalid input is often the correct behavior to fail fast and provide clear error messages.	Low

Suggestions up to commit 38c6ca9

Category	Suggestion	Impact
General	Add type validation for robustness The cast (List<?>) args[0] assumes args[0] is a List when it's not an array, but this assumption is not validated. If args[0] is neither an array nor a List, a ClassCastException will occur at runtime. Add validation to handle unexpected types gracefully. core/src/main/java/org/opensearch/sql/expression/function/CollectionUDF/ForeachPairItemFunctionImpl.java [55-62] public static Object eval(Object... args) { if (args.length < 2 || args[0] == null || args[1] == null) { return null; } int index = ((Number) args[1]).intValue(); - List<?> pair = args[0] instanceof Object[] array ? Arrays.asList(array) : (List<?>) args[0]; + List<?> pair; + if (args[0] instanceof Object[] array) { + pair = Arrays.asList(array); + } else if (args[0] instanceof List<?> list) { + pair = list; + } else { + return null; + } return index < 0 || index >= pair.size() ? null : pair.get(index); } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a potential ClassCastException when args[0] is neither an array nor a List. Adding explicit type checking improves robustness and prevents runtime errors, though the current implementation may assume controlled input from the planner.	Medium
	Improve error message clarity When collection is null after attempting firstArrayField, the error message incorrectly states the mode requires "a field" even though AUTO_COLLECTIONS mode is designed to work without an explicit field by auto-detecting one. The error should clarify that no suitable array field was found when auto-detection fails. core/src/main/java/org/opensearch/sql/calcite/ForeachPlanner.java [174-182] private RelNode planCollection(Foreach node, CalcitePlanContext context) { Foreach.Mode mode = node.getMode(); UnresolvedExpression collection = node.getCollectionExpression(); if (collection == null && mode == Foreach.Mode.AUTO_COLLECTIONS) { collection = firstArrayField(context); } if (collection == null) { - throw new SemanticCheckException("foreach " + mode + " mode requires a field"); + String message = mode == Foreach.Mode.AUTO_COLLECTIONS + ? "foreach auto_collections mode requires a multivalue field or JSON array" + : "foreach " + mode + " mode requires a field"; + throw new SemanticCheckException(message); } Suggestion importance[1-10]: 6 __ Why: The suggestion improves error message clarity for AUTO_COLLECTIONS mode. However, the error message at line 255-256 already provides this clarity, so the improvement is moderate rather than critical.	Low
	Eliminate redundant pattern matching The method performs redundant pattern matching by calling matchesWildcardPattern first, then compiling and matching the regex again. Since the regex matcher already determines if the pattern matches, the initial check is unnecessary and wastes computation. Remove the first matchesWildcardPattern call and rely solely on matcher.matches(). core/src/main/java/org/opensearch/sql/calcite/ForeachPlanner.java [153-169] private List<String> wildcardSegments(String pattern, String fieldName) { - if (!WildcardUtils.matchesWildcardPattern(pattern, fieldName)) { - return null; - } if (!WildcardUtils.containsWildcard(pattern)) { - return List.of(); + return WildcardUtils.matchesWildcardPattern(pattern, fieldName) ? List.of() : null; } Matcher matcher = Pattern.compile(WildcardUtils.convertWildcardPatternToRegex(pattern)).matcher(fieldName); if (!matcher.matches()) { return null; } List<String> segments = new ArrayList<>(); for (int i = 1; i <= matcher.groupCount(); i++) { segments.add(matcher.group(i)); } return segments; } Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies redundant pattern matching, but the optimization is minor. The first check provides early exit for non-matching patterns before regex compilation, which may be intentional for performance in the non-matching case.	Low

Suggestions up to commit 0e74b5d

Category	Suggestion	Impact
General	Validate argument type before casting The method assumes args[1] is a Number without validation, which could cause a ClassCastException if an unexpected type is passed. Add type checking before casting to prevent runtime failures. core/src/main/java/org/opensearch/sql/expression/function/CollectionUDF/ForeachPairItemFunctionImpl.java [55-62] public static Object eval(Object... args) { if (args.length < 2 || args[0] == null || args[1] == null) { + return null; + } + if (!(args[1] instanceof Number)) { return null; } int index = ((Number) args[1]).intValue(); List<?> pair = args[0] instanceof Object[] array ? Arrays.asList(array) : (List<?>) args[0]; return index < 0 || index >= pair.size() ? null : pair.get(index); } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a potential ClassCastException if args[1] is not a Number. Adding type checking before casting improves robustness and prevents runtime failures. This is a valid defensive programming practice for a UDF that may receive unexpected inputs.	Medium
	Handle JSON parsing errors explicitly The method silently catches JsonSyntaxException and defaults to VARCHAR for malformed JSON, which may hide errors. Consider logging the exception or throwing a SemanticCheckException to provide clearer feedback when JSON parsing fails during plan time. core/src/main/java/org/opensearch/sql/calcite/ForeachPlanner.java [284-326] private SqlTypeName jsonElementType( UnresolvedExpression collection, CalcitePlanContext context, List<ForeachEvalClause> evalClauses, String itemName) { if (collection instanceof Function function && BuiltinFunctionName.JSON_ARRAY .getName() .getFunctionName() .equalsIgnoreCase(function.getFuncName())) { return elementTypeOf( function.getFuncArgs().stream() .map(arg -> rexVisitor.analyze(arg, context).getType()) .map(RelDataType::getFamily) .collect(Collectors.toSet())); } if (collection instanceof Literal literal && literal.getType() == DataType.STRING) { try { List<?> values = gson.fromJson(String.valueOf(literal.getValue()), List.class); if (values != null) { return elementTypeOf( values.stream() .filter(Objects::nonNull) .map(v -> v instanceof Number ? SqlTypeFamily.NUMERIC : SqlTypeFamily.CHARACTER) .collect(Collectors.toSet())); } - } catch (JsonSyntaxException ignored) { - // Malformed JSON literals return null at runtime; type choice is irrelevant. + } catch (JsonSyntaxException e) { + throw new SemanticCheckException("Invalid JSON array literal: " + literal.getValue(), e); } return SqlTypeName.VARCHAR; } return itemUsedInArithmetic(evalClauses, itemName) ? SqlTypeName.DOUBLE : SqlTypeName.VARCHAR; } Suggestion importance[1-10]: 4 __ Why: The suggestion proposes throwing a SemanticCheckException instead of silently catching JsonSyntaxException. However, the comment in the existing code explicitly states "Malformed JSON literals return null at runtime; type choice is irrelevant," indicating this is intentional behavior. The suggestion would change the semantics by failing at plan time rather than runtime, which may not align with the design intent.	Low
	Remove redundant pattern matching check The method performs redundant pattern matching. After checking matchesWildcardPattern returns true, the subsequent matcher.matches() check is unnecessary and will always succeed. Remove the second match check to avoid duplicate regex evaluation. core/src/main/java/org/opensearch/sql/calcite/ForeachPlanner.java [153-169] private List<String> wildcardSegments(String pattern, String fieldName) { if (!WildcardUtils.matchesWildcardPattern(pattern, fieldName)) { return null; } if (!WildcardUtils.containsWildcard(pattern)) { return List.of(); } Matcher matcher = Pattern.compile(WildcardUtils.convertWildcardPatternToRegex(pattern)).matcher(fieldName); - if (!matcher.matches()) { - return null; - } + matcher.matches(); // Must call matches() to populate groups List<String> segments = new ArrayList<>(); for (int i = 1; i <= matcher.groupCount(); i++) { segments.add(matcher.group(i)); } return segments; } Suggestion importance[1-10]: 3 __ Why: The suggestion correctly identifies that after matchesWildcardPattern returns true, the subsequent matcher.matches() check is redundant. However, the improved code still calls matcher.matches() (just without the conditional), which is necessary to populate the groups. The impact is minor since it only removes an unnecessary null check.	Low

Suggestions up to commit 93b0892

Category	Suggestion	Impact
Possible issue	Validate input type before casting The cast (List<?>) args[0] assumes args[0] is a List if it's not an array, but this is not validated. If args[0] is neither an array nor a List, a ClassCastException will occur at runtime. Add explicit type validation or handle unexpected types gracefully. core/src/main/java/org/opensearch/sql/expression/function/CollectionUDF/ForeachPairItemFunctionImpl.java [55-62] public static Object eval(Object... args) { if (args.length < 2 || args[0] == null || args[1] == null) { return null; } int index = ((Number) args[1]).intValue(); - List<?> pair = args[0] instanceof Object[] array ? Arrays.asList(array) : (List<?>) args[0]; + List<?> pair; + if (args[0] instanceof Object[] array) { + pair = Arrays.asList(array); + } else if (args[0] instanceof List<?> list) { + pair = list; + } else { + throw new IllegalArgumentException("foreach pair item requires an array or list input"); + } return index < 0 || index >= pair.size() ? null : pair.get(index); } Suggestion importance[1-10]: 7 __ Why: The unchecked cast (List<?>) args[0] could cause a ClassCastException at runtime if args[0] is neither an array nor a List. Adding explicit validation improves robustness and provides clearer error messages.	Medium
General	Remove redundant pattern match check The method checks matchesWildcardPattern first, then compiles a regex and calls matcher.matches() again. If the first check passes, the second matcher.matches() should always succeed, making the second null-return unreachable. Remove the redundant check or consolidate the logic to avoid confusion. core/src/main/java/org/opensearch/sql/calcite/ForeachPlanner.java [153-169] private List<String> wildcardSegments(String pattern, String fieldName) { if (!WildcardUtils.matchesWildcardPattern(pattern, fieldName)) { return null; } if (!WildcardUtils.containsWildcard(pattern)) { return List.of(); } Matcher matcher = Pattern.compile(WildcardUtils.convertWildcardPatternToRegex(pattern)).matcher(fieldName); - if (!matcher.matches()) { - return null; + matcher.matches(); // guaranteed to succeed + List<String> segments = new ArrayList<>(); + for (int i = 1; i <= matcher.groupCount(); i++) { + segments.add(matcher.group(i)); } - ... + return segments; } Suggestion importance[1-10]: 5 __ Why: The redundant matcher.matches() check is unnecessary since matchesWildcardPattern already validates the pattern. However, the code still functions correctly, so this is a minor optimization rather than a bug fix.	Low
	Handle empty type family set explicitly When families is empty (all nulls in the JSON array), the method returns SqlTypeName.VARCHAR by default. This silent fallback may mask data issues. Consider explicitly handling the empty set case or documenting this behavior to clarify the intended semantics. core/src/main/java/org/opensearch/sql/calcite/ForeachPlanner.java [357-365] private SqlTypeName elementTypeOf(Set<?> families) { boolean numeric = families.contains(SqlTypeFamily.NUMERIC); boolean character = families.contains(SqlTypeFamily.CHARACTER); if (numeric && character) { throw new SemanticCheckException( "foreach json_array elements must be consistently strings or numbers"); } + if (families.isEmpty()) { + return SqlTypeName.VARCHAR; // default for all-null arrays + } return numeric ? SqlTypeName.DOUBLE : SqlTypeName.VARCHAR; } Suggestion importance[1-10]: 4 __ Why: The suggestion adds explicit handling for empty families sets, but the current implicit behavior (returning VARCHAR) is reasonable for all-null arrays. This is more of a code clarity improvement than a functional issue.	Low

[songkant-aws]

Both pending checklist items are now in (0e74b5d34):

• Analyzer.visitForeach throws the standard only-for-Calcite UnsupportedOperationException on the v2 path, with a UT asserting the message.
• PPLQueryDataAnonymizer.visitForeach renders the command with masked options/targets/eval clauses (collection targets can embed literals such as JSON array strings); ForeachPlaceholder masks like a column reference. Covered by three anonymizer UTs (multifield, multivalue with options, json_array with a literal target).

The new-command checklist is now fully satisfied.

[github-actions]

Persistent review updated to latest commit 0e74b5d

[github-actions]

Persistent review updated to latest commit 38c6ca9

[github-actions]

Persistent review updated to latest commit 2c04b27

[songkant-aws]

CI notes on the two failures from run 29075374167:

• build-linux (25, unit): infrastructure — Got socket exception during request. It might be caused by SSL misconfiguration while fetching from Maven; unrelated to this change (same suites pass locally and on the fork's CI).
• build-linux (21, integration): testNoMvBasic/testNoMvWithEval failed under the randomized az-Cyrl test locale — their default-locale toLowerCase() turns ARRAY_JOIN into array_joın (dotless i). Pre-existing bug in those tests (added by the timewrap PR, Support PPL timewrap command  #5241); reproduced locally with the CI seed and fixed in e2934613a by using Locale.ROOT.

[github-actions]

Persistent review updated to latest commit e293461