Add PPL multikv command (fixed-schema)

[noCharger]

Description

Adds the PPL multikv command: a streaming, row-multiplying command that extracts fields from an input field and emits one row per source record. It requires the Calcite (v3) engine.

The input field is selected with field=<name> (defaults to _raw); output columns are declared with fields <col>.... multikv dispatches on the input field's plan-time type:

• Text (VARCHAR): parse table-formatted text (for example ps / top / netstat / df output) into columns. Extracted values are typed string.
• Array of objects (ARRAY<ANY>): explode into one row per element, reading each declared column from the element and preserving its type.
• Single object (MAP<VARCHAR,ANY>): read each declared column, preserving its type; emits one row.

Nested container values are returned serialized (matching the merged makeresults convention); extract deeper fields downstream with spath or another multikv field=<subfield>.

Design, semantics, and deferred scope (runtime auto-header, aligned-offset parsing, filter/rmorig, parse-to-MAP optimization) are in the RFC: #5640.

Changes

• Grammar + Multikv AST + Calcite lowering, including the field=<name> input selector, forceheader, and noheader.
• Type-dispatch in visitMultikv: text → MULTIKV_SPLIT / mvexpand / MULTIKV_EXTRACT; array-of-objects → mvexpand + INTERNAL_ITEM; single object → INTERNAL_ITEM only.
• Bare multikv (no fields, no noheader) is rejected at the semantic layer with actionable guidance (v1 is fixed-schema).
• V2 (non-Calcite) engine returns an unsupported-command error, matching the merged makeresults convention.
• User docs: docs/user/ppl/cmd/multikv.md and an index.md row.

Test coverage

• Unit: CalcitePPLMultikvTest, AstBuilderTest#testMultikvCommand (incl. field=), PPLQueryDataAnonymizerTest.
• IT: CalcitePPLMultikvCommandIT (covering field=, text, array-of-objects, and single-object modes), NewAddedCommandsIT#testMultikv.

Check List

• New functionality includes testing.
• New functionality has been documented.
• Commits are signed per the DCO using --signoff.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 3a0aee8)

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 forceHeader parameter is converted to a 1-based line number but passed as-is to MultikvParser.parse, which expects a 1-based index. However, line 4788 sets forceHeader = -1 when null, and line 4799 passes this directly to relBuilder.literal(forceHeader). If forceHeader is null in the AST node, this becomes -1, which is correct. But if the user provides forceheader=0 or a negative number, the grammar validation in AstBuilder.java (line 1149-1152) rejects it. The issue is that the code at line 4788 assumes node.getForceHeader() returns null when absent, but if it returns 0 (which is <= 0 per the validation), the literal becomes 0 instead of -1, causing incorrect behavior in the parser. This occurs when a user somehow bypasses validation or if the validation logic changes. final int forceHeader = node.getForceHeader() == null ? -1 : node.getForceHeader(); final String filterJoined = (node.getFilterTerms() == null || node.getFilterTerms().isEmpty()) ? "" : String.join(MultikvParser.FS, node.getFilterTerms()); RexNode split = PPLFuncImpTable.INSTANCE.resolve( context.rexBuilder, BuiltinFunctionName.MULTIKV_SPLIT, relBuilder.field(node.getInField()), relBuilder.literal(forceHeader), relBuilder.literal(node.isNoHeader()), Possible Issue In the parse method, when forceHeader > 0, line 69 computes hIdx = Math.min(forceHeader - 1, lines.size() - 1). If forceHeader is larger than the number of lines, hIdx becomes lines.size() - 1, meaning the last line is used as the header. This silently uses the wrong line as a header instead of signaling an error or returning empty results. For example, if the user specifies forceheader=10 but there are only 3 lines, line 3 becomes the header and no data rows are returned (since dataStart = 3 + 1 = 4 exceeds lines.size()). This is confusing behavior that should either be documented or rejected with a clear error. int hIdx = Math.min(forceHeader - 1, lines.size() - 1); header = splitCols(lines.get(hIdx)); dataStart = hIdx + 1; } else { Possible Issue The extract method splits the record string on the FS delimiter with FS_SPLIT.split(record, -1) (line 94). The -1 limit preserves trailing empty strings, which is correct. However, the loop at lines 94-101 does not handle the case where a pair contains multiple KV separators. If a cell value itself contains the KV character (U+0002), pair.indexOf(KV) finds the first occurrence, and pair.substring(idx + 1) returns everything after it, including any subsequent KV characters. This means a cell value like "a\u0002b" would be incorrectly parsed. While KV is a control character unlikely to appear in normal data, if it does (e.g., from binary data or malformed input), the extraction silently returns incorrect values instead of escaping or rejecting the input. for (String pair : FS_SPLIT.split(record, -1)) { int idx = pair.indexOf(KV); if (idx < 0) { continue; } if (pair.substring(0, idx).equals(col)) { return pair.substring(idx + 1); }

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 3a0aee8

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Escape delimiters in serialized records The serialize method doesn't escape FS or KV delimiters that may appear in column names or cell values. If a column name contains the KV separator or a cell value contains FS, the serialized record becomes ambiguous and extract may return incorrect results. core/src/main/java/org/opensearch/sql/expression/function/multikv/MultikvParser.java [123-135] private static String serialize(List<String> header, List<String> cells) { int n = (header != null) ? Math.max(header.size(), cells.size()) : cells.size(); StringBuilder sb = new StringBuilder(); for (int i = 0; i < n; i++) { String name = (header != null && i < header.size()) ? header.get(i) : ("Column_" + (i + 1)); String val = (i < cells.size()) ? cells.get(i) : ""; if (sb.length() > 0) { sb.append(FS); } - sb.append(name).append(KV).append(val); + sb.append(escape(name)).append(KV).append(escape(val)); } return sb.toString(); } +private static String escape(String s) { + return s.replace(FS, "\\u001f").replace(KV, "\\u0002"); +} + Suggestion importance[1-10]: 7 __ Why: Important suggestion addressing a potential data corruption issue. If column names or cell values contain the FS or KV delimiters, the serialized format becomes ambiguous and extract may return incorrect results. Adding escaping would prevent this issue, though the impact depends on whether such characters are likely in practice.	Medium
	Validate forceHeader against available lines When forceHeader exceeds the number of lines, hIdx is clamped to lines.size() - 1, making dataStart = hIdx + 1 equal to lines.size(). This causes the loop to process zero data rows silently. Validate that forceHeader is within bounds and throw an exception if it exceeds the available lines. core/src/main/java/org/opensearch/sql/expression/function/multikv/MultikvParser.java [63-72] -public static List<String> parse( - String raw, int forceHeader, boolean noHeader, List<String> filterTerms) { - if (raw == null) { - return Collections.emptyList(); +int dataStart; +if (noHeader) { + header = null; + dataStart = 0; +} else if (forceHeader > 0) { + if (forceHeader > lines.size()) { + throw new IllegalArgumentException( + "forceheader=" + forceHeader + " exceeds available lines (" + lines.size() + ")"); } - ... - int dataStart; - if (noHeader) { - header = null; - dataStart = 0; - } else if (forceHeader > 0) { - int hIdx = Math.min(forceHeader - 1, lines.size() - 1); - header = splitCols(lines.get(hIdx)); - dataStart = hIdx + 1; - } + int hIdx = forceHeader - 1; + header = splitCols(lines.get(hIdx)); + dataStart = hIdx + 1; +} Suggestion importance[1-10]: 6 __ Why: Valid suggestion to add validation for forceHeader exceeding available lines. Currently, when forceHeader is too large, the code silently clamps it and may process zero data rows. Adding an explicit check and error message would improve user experience by catching configuration errors early.	Low
	Handle non-numeric forceheader values gracefully The forceHeader parsing uses Integer.parseInt without catching NumberFormatException. If the user provides a non-numeric value, the exception propagates uncaught. Wrap the parse in a try-catch and throw a more descriptive error message. ppl/src/main/java/org/opensearch/sql/ppl/parser/AstBuilder.java [1147-1153] -for (OpenSearchPPLParser.MultikvParameterContext p : ctx.multikvParameter()) { - if (p.fields != null) { - ... - } else if (p.forceHeader != null) { +} else if (p.forceHeader != null) { + try { forceHeader = Integer.parseInt(p.forceHeader.getText()); if (forceHeader <= 0) { throw new IllegalArgumentException( "multikv forceheader must be a positive line number, got: " + forceHeader); } + } catch (NumberFormatException e) { + throw new IllegalArgumentException( + "multikv forceheader must be a valid integer, got: " + p.forceHeader.getText()); } } Suggestion importance[1-10]: 5 __ Why: Valid suggestion to improve error handling. While Integer.parseInt will throw NumberFormatException for invalid input, catching it and providing a more descriptive error message would improve user experience. However, the impact is moderate since the exception would still be caught somewhere in the error handling chain.	Low

---

Previous suggestions

Suggestions up to commit 0ff1dd0

Category	Suggestion	Impact
Possible issue	Validate separator characters in serialization The serialize method does not validate or sanitize the name and val strings before appending them. If these contain the special separators FS or KV, the serialized format becomes ambiguous and extract may return incorrect results. Add validation to reject or escape separator characters. core/src/main/java/org/opensearch/sql/expression/function/multikv/MultikvParser.java [123-135] private static String serialize(List<String> header, List<String> cells) { int n = (header != null) ? Math.max(header.size(), cells.size()) : cells.size(); StringBuilder sb = new StringBuilder(); for (int i = 0; i < n; i++) { String name = (header != null && i < header.size()) ? header.get(i) : ("Column_" + (i + 1)); String val = (i < cells.size()) ? cells.get(i) : ""; + if (name.contains(FS) || name.contains(KV) || val.contains(FS) || val.contains(KV)) { + throw new IllegalArgumentException("Column names and values cannot contain separator characters"); + } if (sb.length() > 0) { sb.append(FS); } sb.append(name).append(KV).append(val); } return sb.toString(); } Suggestion importance[1-10]: 8 __ Why: This addresses a correctness issue where column names or values containing the special separators FS or KV would break the serialization format and cause extract to return incorrect results. The validation prevents data corruption.	Medium
Security	Add input size validation The LINE.split(raw) operation can produce a large array for very large input strings, potentially causing memory issues. Consider using a streaming approach or adding a size limit check before splitting to prevent excessive memory consumption from malicious or malformed input. core/src/main/java/org/opensearch/sql/expression/function/multikv/MultikvParser.java [48-61] public static List<String> parse( String raw, int forceHeader, boolean noHeader, List<String> filterTerms) { if (raw == null) { return Collections.emptyList(); + } + if (raw.length() > MAX_INPUT_SIZE) { + throw new IllegalArgumentException("Input exceeds maximum allowed size"); } List<String> lines = new ArrayList<>(); for (String l : LINE.split(raw)) { if (!l.trim().isEmpty()) { lines.add(l); } } if (lines.isEmpty()) { return Collections.emptyList(); } Suggestion importance[1-10]: 7 __ Why: Adding input size validation prevents potential memory exhaustion from very large inputs. However, the suggestion doesn't define MAX_INPUT_SIZE, and the severity depends on deployment context and upstream input validation.	Medium
General	Validate fields before building child The error check for missing fields occurs after the child plan is built (node.getChild().get(0).accept(this, context)), which means resources are allocated before validation. Move this validation earlier to fail fast and avoid unnecessary processing when the query is invalid. core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java [4690-4712] +List<Field> fields = node.getFields(); +boolean noFields = (fields == null || fields.isEmpty()); + if (noFields && !node.isNoHeader()) { throw ErrorReport.wrap( new SemanticCheckException( "multikv has no declared output columns. Add an explicit fields clause, for" + " example: multikv fields <col1> <col2>")) .code(ErrorCode.FIELD_NOT_FOUND) .location("while resolving the output schema for multikv") .context("command", "multikv") .build(); } +RelNode probe = node.getChild().get(0).accept(this, context); + Suggestion importance[1-10]: 6 __ Why: Moving validation before building the child plan is a minor optimization that fails fast for invalid queries. However, the performance impact is likely small since the validation is quick and the error would be caught anyway.	Low

Suggestions up to commit 9296e74

Category	Suggestion	Impact
Possible issue	Escape separator characters in values The serialize method does not escape or validate cell values that might contain the special separators FS or KV. If a cell value contains these characters, the serialized record will be corrupted and extract will return incorrect results. Add escaping or validation to prevent separator injection. core/src/main/java/org/opensearch/sql/expression/function/multikv/MultikvParser.java [123-135] private static String serialize(List<String> header, List<String> cells) { int n = (header != null) ? Math.max(header.size(), cells.size()) : cells.size(); StringBuilder sb = new StringBuilder(); for (int i = 0; i < n; i++) { String name = (header != null && i < header.size()) ? header.get(i) : ("Column_" + (i + 1)); String val = (i < cells.size()) ? cells.get(i) : ""; + // Escape special separators to prevent injection + val = val.replace(FS, "").replace(KV, ""); if (sb.length() > 0) { sb.append(FS); } sb.append(name).append(KV).append(val); } return sb.toString(); } Suggestion importance[1-10]: 9 __ Why: This identifies a critical data corruption issue where cell values containing FS or KV separators would break the serialization format, causing extract to return incorrect results. This is a correctness bug that could lead to silent data corruption.	High
General	Validate forceHeader bounds When forceHeader is set to a very large value and lines.size() is small, hIdx will be clamped to lines.size() - 1, causing the last line to be treated as the header. This silently changes behavior instead of validating the input. Validate that forceHeader is within bounds and throw an exception if it exceeds the available lines. core/src/main/java/org/opensearch/sql/expression/function/multikv/MultikvParser.java [65-75] if (noHeader) { header = null; dataStart = 0; } else if (forceHeader > 0) { - int hIdx = Math.min(forceHeader - 1, lines.size() - 1); + if (forceHeader > lines.size()) { + throw new IllegalArgumentException( + "forceheader line " + forceHeader + " exceeds available lines: " + lines.size()); + } + int hIdx = forceHeader - 1; header = splitCols(lines.get(hIdx)); dataStart = hIdx + 1; } else { header = splitCols(lines.get(0)); dataStart = 1; } Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that forceHeader values exceeding available lines are silently clamped rather than validated. This could lead to unexpected behavior. Explicit validation with a clear error message improves correctness and user experience.	Medium
Security	Add line count limit The LINE.split(raw) operation can produce a very large array when raw contains many newlines, potentially causing memory exhaustion. Consider adding a limit on the number of lines processed or implementing streaming line processing to prevent resource exhaustion attacks. core/src/main/java/org/opensearch/sql/expression/function/multikv/MultikvParser.java [48-61] +private static final int MAX_LINES = 10000; + public static List<String> parse( String raw, int forceHeader, boolean noHeader, List<String> filterTerms) { if (raw == null) { return Collections.emptyList(); } List<String> lines = new ArrayList<>(); for (String l : LINE.split(raw)) { if (!l.trim().isEmpty()) { lines.add(l); + if (lines.size() >= MAX_LINES) { + break; + } } } if (lines.isEmpty()) { return Collections.emptyList(); } Suggestion importance[1-10]: 7 __ Why: The suggestion addresses a potential resource exhaustion vulnerability where processing very large inputs could cause memory issues. Adding a line limit is a reasonable safeguard, though the specific limit value should be configurable.	Medium

Suggestions up to commit e146e91

Category	Suggestion	Impact
Possible issue	Validate forceHeader bounds explicitly When forceHeader is set to a value greater than the number of lines, the code silently uses the last line as the header. This could lead to incorrect parsing without any indication to the user. Consider validating that forceHeader is within bounds and providing clear feedback. core/src/main/java/org/opensearch/sql/expression/function/multikv/MultikvParser.java [65-75] if (noHeader) { header = null; dataStart = 0; } else if (forceHeader > 0) { - int hIdx = Math.min(forceHeader - 1, lines.size() - 1); + if (forceHeader > lines.size()) { + throw new IllegalArgumentException( + "forceheader line " + forceHeader + " exceeds available lines (" + lines.size() + ")"); + } + int hIdx = forceHeader - 1; header = splitCols(lines.get(hIdx)); dataStart = hIdx + 1; } else { header = splitCols(lines.get(0)); dataStart = 1; } Suggestion importance[1-10]: 8 __ Why: The current code silently uses the last line when forceHeader exceeds available lines, which could lead to incorrect parsing. Explicit validation with a clear error message improves correctness and user experience.	Medium
Security	Add input size validation The LINE.split(raw) operation can produce a large array when processing very large input strings, potentially causing memory issues. Consider using a streaming approach or adding a size limit to prevent excessive memory consumption from malicious or malformed input. core/src/main/java/org/opensearch/sql/expression/function/multikv/MultikvParser.java [48-61] public static List<String> parse( String raw, int forceHeader, boolean noHeader, List<String> filterTerms) { if (raw == null) { return Collections.emptyList(); + } + if (raw.length() > 1_000_000) { // Add reasonable limit + throw new IllegalArgumentException("Input text exceeds maximum size limit"); } List<String> lines = new ArrayList<>(); for (String l : LINE.split(raw)) { if (!l.trim().isEmpty()) { lines.add(l); } } if (lines.isEmpty()) { return Collections.emptyList(); } Suggestion importance[1-10]: 7 __ Why: Adding input size validation prevents potential memory exhaustion from very large inputs. However, the hardcoded limit of 1MB may be too restrictive for legitimate use cases and should ideally be configurable.	Medium
General	Optimize column extraction performance The extract method performs a linear search through all field-value pairs for each column lookup. When extracting multiple columns from the same record (common use case), this results in O(n*m) complexity. Consider parsing the record into a map once and reusing it for multiple extractions. core/src/main/java/org/opensearch/sql/expression/function/multikv/MultikvParser.java [90-104] public static String extract(String record, String col) { if (record == null || col == null) { return null; } + String searchKey = col + KV; for (String pair : FS_SPLIT.split(record, -1)) { - int idx = pair.indexOf(KV); - if (idx < 0) { - continue; - } - if (pair.substring(0, idx).equals(col)) { - return pair.substring(idx + 1); + if (pair.startsWith(searchKey)) { + return pair.substring(searchKey.length()); } } return null; } Suggestion importance[1-10]: 6 __ Why: The optimization using startsWith is a minor improvement that reduces substring operations. However, the performance gain is marginal for typical use cases, and the original implementation is already reasonably efficient for single-column extraction.	Low

Suggestions up to commit e1b3333

Category	Suggestion	Impact
General	Validate forceHeader bounds strictly When forceHeader exceeds the number of available lines, the code silently uses the last line as the header. This could lead to incorrect parsing where a data row is misinterpreted as a header. Consider throwing an exception or logging a warning when forceHeader is out of bounds. core/src/main/java/org/opensearch/sql/expression/function/multikv/MultikvParser.java [65-72] if (noHeader) { header = null; dataStart = 0; } else if (forceHeader > 0) { - int hIdx = Math.min(forceHeader - 1, lines.size() - 1); + if (forceHeader > lines.size()) { + throw new IllegalArgumentException( + "forceheader line " + forceHeader + " exceeds available lines (" + lines.size() + ")"); + } + int hIdx = forceHeader - 1; header = splitCols(lines.get(hIdx)); dataStart = hIdx + 1; } Suggestion importance[1-10]: 8 __ Why: The current code silently uses the last line when forceHeader exceeds available lines, which could lead to incorrect parsing. Strict validation prevents data misinterpretation and provides clear error messages.	Medium
	Use appropriate error code The error code FIELD_NOT_FOUND is misleading for this validation failure. The issue is not that a field is missing, but that the schema cannot be determined at plan time. Use a more appropriate error code like SEMANTIC_CHECK_EXCEPTION or create a specific code for schema resolution failures. core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java [4503-4512] if (noFields && !node.isNoHeader()) { throw ErrorReport.wrap( new SemanticCheckException( "multikv has no declared output columns. Add an explicit fields clause, for" + " example: multikv fields <col1> <col2>")) - .code(ErrorCode.FIELD_NOT_FOUND) + .code(ErrorCode.SEMANTIC_CHECK_EXCEPTION) .location("while resolving the output schema for multikv") .context("command", "multikv") .build(); } Suggestion importance[1-10]: 5 __ Why: While using SEMANTIC_CHECK_EXCEPTION instead of FIELD_NOT_FOUND is more semantically accurate, this is a minor improvement in error reporting clarity and doesn't affect functionality.	Low
Security	Add input size validation The LINE.split(raw) operation can produce a large array for very large input strings, potentially causing memory issues. Consider using a streaming approach or adding a size limit check before splitting to prevent excessive memory consumption from malicious or malformed input. core/src/main/java/org/opensearch/sql/expression/function/multikv/MultikvParser.java [48-61] public static List<String> parse( String raw, int forceHeader, boolean noHeader, List<String> filterTerms) { if (raw == null) { return Collections.emptyList(); + } + if (raw.length() > 10_000_000) { // 10MB limit + throw new IllegalArgumentException("Input text exceeds maximum size limit"); } List<String> lines = new ArrayList<>(); for (String l : LINE.split(raw)) { if (!l.trim().isEmpty()) { lines.add(l); } } if (lines.isEmpty()) { return Collections.emptyList(); } Suggestion importance[1-10]: 7 __ Why: Adding input size validation prevents potential memory exhaustion from very large inputs. This is a reasonable security enhancement, though the specific limit value may need tuning based on actual use cases.	Medium

Suggestions up to commit eedb47b

Category	Suggestion	Impact
General	Validate schema before building child The error is thrown after building the child node (probe), which may have already performed expensive operations. Move this validation earlier in the method, before building the child, to fail fast and avoid unnecessary computation when the query is invalid. core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java [4495-4519] +List<Field> fields = node.getFields(); +boolean noFields = (fields == null || fields.isEmpty()); + if (noFields && !node.isNoHeader()) { throw ErrorReport.wrap( new SemanticCheckException( "multikv has no declared output columns. Add an explicit fields clause, for" + " example: multikv fields <col1> <col2>")) .code(ErrorCode.FIELD_NOT_FOUND) .location("while resolving the output schema for multikv") .context("command", "multikv") .build(); } +boolean savedProjectVisited = context.isProjectVisited(); +RelNode probe = node.getChild().get(0).accept(this, context); + Suggestion importance[1-10]: 8 __ Why: Moving validation before building the child node is a good optimization that fails fast and avoids unnecessary computation. This improves performance when queries are invalid.	Medium
Security	Add input size validation The LINE.split(raw) operation can produce a large array when processing very large input strings, potentially causing memory issues. Consider using a streaming approach or adding a size limit check before splitting to prevent excessive memory consumption from malicious or malformed input. core/src/main/java/org/opensearch/sql/expression/function/multikv/MultikvParser.java [48-61] public static List<String> parse( String raw, int forceHeader, boolean noHeader, List<String> filterTerms) { if (raw == null) { return Collections.emptyList(); + } + if (raw.length() > 10_000_000) { // 10MB limit + throw new IllegalArgumentException("Input text exceeds maximum allowed size"); } List<String> lines = new ArrayList<>(); for (String l : LINE.split(raw)) { if (!l.trim().isEmpty()) { lines.add(l); } } if (lines.isEmpty()) { return Collections.emptyList(); } Suggestion importance[1-10]: 7 __ Why: Adding input size validation prevents potential memory exhaustion from very large inputs. However, the hardcoded 10MB limit may be too restrictive for legitimate use cases and should ideally be configurable.	Medium
	Limit maximum field count The FS_SPLIT.split(record, -1) can produce a large array for maliciously crafted records with many field separators, leading to potential memory exhaustion. Add a limit on the number of fields to prevent denial-of-service attacks through excessive field counts. core/src/main/java/org/opensearch/sql/expression/function/multikv/MultikvParser.java [90-104] +private static final int MAX_FIELDS = 1000; + public static String extract(String record, String col) { if (record == null || col == null) { return null; } - for (String pair : FS_SPLIT.split(record, -1)) { + String[] pairs = FS_SPLIT.split(record, MAX_FIELDS + 1); + if (pairs.length > MAX_FIELDS) { + throw new IllegalArgumentException("Record exceeds maximum field count"); + } + for (String pair : pairs) { int idx = pair.indexOf(KV); if (idx < 0) { continue; } if (pair.substring(0, idx).equals(col)) { return pair.substring(idx + 1); } } return null; } Suggestion importance[1-10]: 7 __ Why: Adding a field count limit prevents potential DoS attacks through excessive field separators. However, the hardcoded limit of 1000 fields may need adjustment based on actual use cases.	Medium

[github-actions]

Persistent review updated to latest commit e1b3333

[github-actions]

Persistent review updated to latest commit e146e91

[github-actions]

Persistent review updated to latest commit 9296e74

[songkant-aws]

Avoid Splunk wording in comments.

[songkant-aws]

We don't have such metadata field yet. Is it expected to be explicitly generated by user in makeresults command?

[songkant-aws]

We need to print real output for each example ppl queries.

[songkant-aws]

nit: version 3.9?

[songkant-aws]

Avoid Splunk wording in comments.

[songkant-aws]

Per the RFC discussion, we should call out that users must explicitly cast expanded key-value fields from VARCHAR or ANY to the expected types when type-sensitive processing is required.

Please confirm that this reflects an outcome discussed and agreed upon with users.

[songkant-aws]

Add more complex ITs to let downstream commands correctly consume multikv results after casting

[songkant-aws]

Please also add tests to CalciteExplainIT to assert printed logical plans are correct.

[songkant-aws]

For input types other than ARRAY and MAP, the current implementation silently falls back to the text path and treats them as strings. Should we validate the input type here and reject unsupported types during planning?

[songkant-aws]

The type-based dispatch is reasonable, but the current implementation visits the child twice. The first visit mutates the shared RelBuilder and CalcitePlanContext; text mode then calls build() and only restores isProjectVisited before rebuilding the child. This is not a full rollback and may affect features such as correlation binding or plan-node tracking. Could we build the child once and lower text mode from the resulting RelNode? Is there a specific reason to discard the build in text mode? The schema is known at stack peek and it seems feasible to build text mode specific MvExpand Relnode on top of it.

[github-actions]

Persistent review updated to latest commit 0ff1dd0

[github-actions]

Persistent review updated to latest commit 3a0aee8