[Feature] Add PPL makeresults command

[noCharger]

Description

Adds the makeresults leading command on the Calcite (v3) path. It generates in-memory rows with no index scan.

Count path

• | makeresults / | makeresults count=N produces N rows (default 1), each with a single @timestamp (TIMESTAMP) column set to query time (now(), one value for the whole query).
• @timestamp is OpenSearch PPL's implicit time field (OpenSearchConstants.IMPLICIT_FIELD_TIMESTAMP), so the generated rows compose with the time-aware command family (sort, stats ... by span, timechart, reverse, timewrap). This intentionally uses @timestamp rather than a synthetic _time column.

format/data path

• format=csv|json data="..." parses an inline literal into typed rows. Column types are synthesized at plan time following OpenSearch dynamic-mapping semantics and surfaced through the same type path an index scan uses:
  • JSON integer → long (bigint), JSON decimal → float (REAL), JSON boolean → boolean, JSON string → keyword/string.
  • A typed CSV header token name:type uses the same vocabulary as cast (e.g. age:int); a bare CSV header token defaults to string.
  • Per-column heterogeneity widens to a common type; a cell that fails to parse to its declared type is a plan-time error.

Grammar lives in the ppl/ copies only. The command adds the MakeResults AST node, AstBuilder wiring with MakeResultsDataParser, CalciteRelNodeVisitor.visitMakeResults (builds LogicalValues + Project), a V2 Analyzer reject stub (V2 returns the standard "supported only when plugins.calcite.enabled=true" error), and anonymizer rendering.

Robustness / hardening

• count=0 and header-only CSV build an empty typed LogicalValues(tuples=[[]]) with the parsed rowType (previously threw a raw Calcite "Value count must be a positive multiple of field count"). A negative count also yields zero rows.
• All-null JSON column is rejected with a clean SyntaxCheckException (previously NPE via Map.merge).
• JSON big integers that overflow long keep full precision as a string column instead of silently wrapping.
• Nested JSON object/array values are rejected with a clear message (previously silently became "").
• CSV values containing commas are handled by a quote-aware splitter (RFC4180 "" escape).
• Caps: count ≤ 1,000,000; data ≤ 29,999 chars.

Notes / intentional divergences

• A JSON integer column reports its Calcite type name bigint in the response schema, because makeresults rows carry no index mapping to map bigint back to the PPL type name long.
• Inline object/array values and the date/time/timestamp/ip/json inline CSV types are not supported on this path (use string + cast).
• The count path output (@timestamp = now()) is non-deterministic, so it is not registered for doctest; the deterministic data= examples can be added after cluster validation.

Related Issues

Closes #3629
RFC: #5626

Check List

• New functionality includes testing (unit + integration).
• New functionality has been documented (docs/user/ppl/cmd/makeresults.md).
• Commits are signed per the DCO using --signoff.
• spotlessCheck passed.
• Unit tests passed (CalcitePPLMakeResultsTest 13/13, AstBuilderTest, PPLQueryDataAnonymizerTest).
• Integration tests passed on a local cluster (CalcitePPLMakeResultsIT 6/6, NewAddedCommandsIT.testMakeResults).

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 9dc6277)

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 uniquify, when a duplicate column name is encountered, the suffix starts at 0 and increments until a unique name is found. However, if the original name is "name" and "name0" already exists in the input list, the loop will produce "name0" again, which will then collide with the existing "name0" in the list. This happens because the method only checks the seen set (which tracks what has been added to out), not the original input list. For example, given input ["name", "name0", "name"], the method produces ["name", "name0", "name0"] instead of ["name", "name0", "name1"]. private static List<String> uniquify(List<String> names) { List<String> out = new ArrayList<>(); Set<String> seen = new HashSet<>(); for (String n : names) { String candidate = n; int suffix = 0; while (!seen.add(candidate)) { candidate = n + suffix++; } out.add(candidate); } return out; } Possible Issue In parseCsv, when a row has fewer cells than the header, missing cells are treated as null (empty string). However, when a row has more cells than the header, an exception is thrown. This asymmetry means that a CSV with a trailing comma (e.g., "name,age\nJohn,35,") will fail, even though trailing commas are common in CSV files and typically represent an empty trailing field. The current logic does not handle this gracefully. for (int li = 1; li < lines.length; li++) { if (lines[li].isEmpty()) { continue; } List<String> cells = splitCsvLine(lines[li]); if (cells.size() > names.size()) { throw new SyntaxCheckException( "makeresults CSV row has more columns than the header: " + lines[li]); } List<Object> out = new ArrayList<>(); for (int i = 0; i < names.size(); i++) { String cell = i < cells.size() ? cells.get(i).trim() : ""; out.add(coerce(cell.isEmpty() ? null : cell, types.get(i))); } rows.add(out); }

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 9dc6277

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Validate data size earlier The character limit validation in parse() occurs after the data string is already in memory. For very large inputs approaching or exceeding this limit, the validation happens too late. Consider enforcing this limit at the parser/lexer level or immediately upon receiving the input to prevent processing oversized data. ppl/src/main/java/org/opensearch/sql/ppl/utils/MakeResultsDataParser.java [321-323] -if (data.length() > 29999) { - throw new SyntaxCheckException("makeresults data must not exceed 29999 characters"); -} +// Move this validation earlier in the pipeline (e.g., in AstBuilder.visitMakeresultsCommand) +// before passing to MakeResultsDataParser.parse() Suggestion importance[1-10]: 7 __ Why: Moving the character limit validation to AstBuilder.visitMakeresultsCommand (where it's already present in the PR) before passing to MakeResultsDataParser.parse() is a good practice to fail fast and avoid unnecessary processing. The PR already implements this check at line 321-323, making the suggestion's intent already addressed.	Medium
	Validate row count during parsing The row count validation occurs after parsing the entire data string, which could consume significant memory for large inputs. Consider validating the row count during parsing (incrementally) to fail fast and avoid unnecessary memory allocation when the limit is exceeded early in the data. ppl/src/main/java/org/opensearch/sql/ppl/utils/MakeResultsDataParser.java [62-64] -if (result.getValues() != null && result.getValues().size() > 5000) { - throw new SyntaxCheckException("makeresults data must not exceed 5000 rows"); -} +// Move this validation into parseJson/parseCsv methods to check incrementally +// during parsing, before accumulating all rows in memory Suggestion importance[1-10]: 6 __ Why: The suggestion to validate row count incrementally during parsing is valid and would improve memory efficiency by failing fast. However, the impact is moderate since the 5000-row limit and 29999-character limit already constrain memory usage to reasonable bounds.	Low
	Optimize memory for large counts When count is large (up to 5000), this allocates an array and populates it with sequential integers. For large counts, this could be memory-intensive. Consider using a more memory-efficient approach, such as generating a single-row relation and using a LogicalRepeat or similar construct to multiply it, if Calcite supports such optimization. core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java [4605-4608] -Object[] dummy = new Object[count]; -for (int i = 0; i < count; i++) { - dummy[i] = i; -} +// Consider using LogicalRepeat or a more memory-efficient approach +// to avoid materializing large arrays for high count values Suggestion importance[1-10]: 5 __ Why: The suggestion to use a more memory-efficient approach for large count values is reasonable, but the impact is limited. The array allocation for up to 5000 integers is relatively small (20KB), and the code already has a 5000-row cap enforced elsewhere, making this optimization minor.	Low

---

Previous suggestions

Suggestions up to commit 58ab1c4

Category	Suggestion	Impact
General	Handle empty first row edge case When rows is not empty but contains an empty first row, accessing rows.get(0).size() could return 0 unexpectedly. Add validation to ensure the first row is not empty when inferring column count from rows, or handle this edge case explicitly. core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java [4495-4502] if (explicitTypes != null) { nc = explicitTypes.size(); } else if (explicitNames != null) { nc = explicitNames.size(); +} else if (!rows.isEmpty() && !rows.get(0).isEmpty()) { + nc = rows.get(0).size(); } else { - nc = rows.isEmpty() ? 0 : rows.get(0).size(); + nc = 0; } Suggestion importance[1-10]: 7 __ Why: The suggestion addresses a potential edge case where rows is not empty but contains an empty first row. This could prevent unexpected behavior when rows.get(0).size() returns 0, improving robustness.	Medium
	Improve error message clarity The error message should be more descriptive and consistent with similar nodes. Consider including the node type in the message to aid debugging when this exception is encountered. core/src/main/java/org/opensearch/sql/ast/tree/MakeResults.java [34-36] @Override public UnresolvedPlan attach(UnresolvedPlan child) { - throw new UnsupportedOperationException("MakeResults node is supposed to have no child node"); + throw new UnsupportedOperationException("MakeResults is a leading command and cannot have child nodes"); } Suggestion importance[1-10]: 5 __ Why: The suggestion improves error message clarity by making it more descriptive and consistent with the node's purpose as a leading command. However, the impact is minor since it only affects error messaging.	Low

Suggestions up to commit 1257c59

Category	Suggestion	Impact
General	Detect unterminated CSV quoted fields The CSV parser doesn't handle the case where a quoted field is not properly closed (unterminated quote). If inQuotes is still true after processing the entire line, it indicates malformed CSV data that should be rejected rather than silently accepted. ppl/src/main/java/org/opensearch/sql/ppl/utils/MakeResultsDataParser.java [162-190] private static List<String> splitCsvLine(String line) { List<String> out = new ArrayList<>(); StringBuilder cur = new StringBuilder(); boolean inQuotes = false; for (int i = 0; i < line.length(); i++) { char c = line.charAt(i); if (inQuotes) { if (c == '"') { if (i + 1 < line.length() && line.charAt(i + 1) == '"') { cur.append('"'); i++; } else { inQuotes = false; } } else { cur.append(c); } } else if (c == '"') { inQuotes = true; } else if (c == ',') { out.add(cur.toString()); cur.setLength(0); } else { cur.append(c); } } + if (inQuotes) { + throw new SyntaxCheckException("makeresults CSV has unterminated quoted field"); + } out.add(cur.toString()); return out; } Suggestion importance[1-10]: 8 __ Why: Valid bug fix for malformed CSV input. The current implementation silently accepts unterminated quoted fields, which could lead to incorrect data parsing. Adding validation for inQuotes after the loop ensures proper CSV format and provides clear error messages for malformed input.	Medium
Security	Validate row count during parsing The row count validation occurs after parsing the entire data string, which could consume significant memory for large inputs. Consider validating the row count during parsing to fail fast and prevent potential memory exhaustion from malicious or malformed inputs. ppl/src/main/java/org/opensearch/sql/ppl/utils/MakeResultsDataParser.java [57-59] -if (result.getRows() != null && result.getRows().size() > 5000) { - throw new SyntaxCheckException("makeresults data must not exceed 5000 rows"); -} +// Validate row count during parsing in parseJson/parseCsv methods +// to fail fast before materializing all rows in memory Suggestion importance[1-10]: 7 __ Why: Valid security improvement to prevent memory exhaustion by validating row count during parsing rather than after. However, the implementation would require significant refactoring of the parsing methods, and the current approach with a 29999 character limit already provides some protection.	Medium

Suggestions up to commit 3a8654b

Category	Suggestion	Impact
Possible issue	Reject oversized integers explicitly The fallback to STRING for integers exceeding long range may cause unexpected type inconsistencies. Consider throwing a SyntaxCheckException instead to alert users that their integer value is too large, ensuring data integrity and preventing silent type changes. ppl/src/main/java/org/opensearch/sql/ppl/utils/MakeResultsDataParser.java [193-196] if (v.isIntegralNumber()) { - // A JSON integer wider than long keeps full precision as a string rather than overflowing. - return v.canConvertToLong() ? ExprCoreType.LONG : ExprCoreType.STRING; + if (!v.canConvertToLong()) { + throw new SyntaxCheckException( + "makeresults JSON integer value exceeds long range: " + v.asText()); + } + return ExprCoreType.LONG; } Suggestion importance[1-10]: 5 __ Why: The suggestion proposes throwing an exception for integers exceeding long range instead of falling back to STRING. While this would prevent silent type changes, the current behavior of preserving precision as a string is a reasonable design choice documented in the code comment. The suggestion improves consistency but isn't critical.	Low
General	Validate empty CSV lines properly Empty lines are silently skipped, which may hide malformed CSV data. Consider validating that skipped lines contain only whitespace, or throw an exception for truly empty lines to ensure data quality and prevent accidental data loss. ppl/src/main/java/org/opensearch/sql/ppl/utils/MakeResultsDataParser.java [132-135] for (int li = 1; li < lines.length; li++) { - if (lines[li].isEmpty()) { + if (lines[li].trim().isEmpty()) { continue; } List<String> cells = splitCsvLine(lines[li]); if (cells.size() > names.size()) { throw new SyntaxCheckException( "makeresults CSV row has more columns than the header: " + lines[li]); } Suggestion importance[1-10]: 3 __ Why: The suggestion to use trim().isEmpty() instead of isEmpty() is a minor improvement that handles lines with only whitespace more consistently. However, the current behavior is acceptable for typical CSV parsing scenarios and the impact is minimal.	Low

Suggestions up to commit 8e6ecc8

Category	Suggestion	Impact
Possible issue	Fix unreachable empty array condition The condition lines.length == 0 can never be true because split() with a limit of -1 always returns at least one element (even for an empty string). This makes the first part of the condition unreachable. Consider checking if the array is empty or if the first element is empty after splitting. ppl/src/main/java/org/opensearch/sql/ppl/utils/MakeResultsDataParser.java [96-99] String[] lines = data.split("\r?\n", -1); -if (lines.length == 0 || lines[0].trim().isEmpty()) { +if (lines.length == 0 || lines[0].isEmpty()) { throw new SyntaxCheckException("makeresults CSV data must start with a header line"); } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that split() with limit -1 always returns at least one element, making lines.length == 0 unreachable. However, the improved code removes .trim() from the second condition, which changes the behavior from checking if the trimmed first line is empty to checking if the raw first line is empty. This could introduce inconsistency with the intended validation logic.	Medium
Security	Add upper bound for count parameter When count is zero, an empty array is created and the loop doesn't execute, which is correct. However, when count is very large (e.g., Integer.MAX_VALUE), this could cause an OutOfMemoryError. Consider adding a reasonable upper bound check to prevent memory exhaustion attacks. core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java [4506-4512] int count = node.getCount(); +if (count > 10000) { + throw new IllegalArgumentException("makeresults count exceeds maximum allowed value of 10000"); +} // Build `count` one-column dummy rows, then project them away with a single _time column set // to query time. The dummy column only carries row multiplicity; count=0 yields zero rows. Object[] dummy = new Object[count]; for (int i = 0; i < count; i++) { dummy[i] = i; } Suggestion importance[1-10]: 6 __ Why: The suggestion raises a valid concern about potential memory exhaustion with very large count values. However, the specific limit of 10000 is arbitrary and not justified by the PR context. A more appropriate approach would involve configuration-based limits or resource-aware validation rather than a hardcoded constant.	Low
General	Handle whitespace-only lines consistently Empty lines are skipped but trimmed empty lines are not, which could lead to inconsistent behavior. A line containing only whitespace will be processed and may cause unexpected results. Consider using trim().isEmpty() for consistency with the header check. ppl/src/main/java/org/opensearch/sql/ppl/utils/MakeResultsDataParser.java [119-127] for (int li = 1; li < lines.length; li++) { - if (lines[li].isEmpty()) { + if (lines[li].trim().isEmpty()) { continue; } String[] cells = lines[li].split(",", -1); if (cells.length > names.size()) { throw new SyntaxCheckException( "makeresults CSV row has more columns than the header: " + lines[li]); } Suggestion importance[1-10]: 5 __ Why: The suggestion identifies a minor inconsistency where whitespace-only lines are not skipped like empty lines. While adding .trim() would improve consistency, this is a relatively minor issue that doesn't affect correctness significantly, as whitespace-only lines would still be parsed (potentially as rows with empty cells).	Low

[github-actions]

PR Code Analyzer ❗

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

Path	Line	Severity	Description
ppl/src/main/java/org/opensearch/sql/ppl/parser/AstBuilder.java	279	low	The data length check uses Java String.length() (UTF-16 code units) rather than byte length. For multi-byte UTF-8 input (e.g., CJK characters), the actual byte payload passed to Jackson's ObjectMapper can be up to 3–4x larger than the 29,999-character cap implies, allowing a user to feed more data into the JSON/CSV parser than the limit suggests. Not clearly intentional but worth noting as an edge case in the bounds enforcement.

The table above displays the top 10 most important findings.

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

---

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]

Persistent review updated to latest commit 3a8654b

[github-actions]

Persistent review updated to latest commit 1257c59

[github-actions]

Persistent review updated to latest commit 58ab1c4

[songkant-aws]

Do we expect to reject those nested field types? Or just fallback to serialized JSON strings? I think spath command could resolve the json string later.

[songkant-aws]

We should generate @timestamp default time fields as well for specified json data.

[songkant-aws]

Same as above

[songkant-aws]

Same as above

[songkant-aws]

Nit: Claim this constraint in the md doc.

[songkant-aws]

Boolean.parseBoolean() will treat any invalid boolean value string to false. For example, Boolean.parseBoolean("not true") -> false.

[songkant-aws]

PR Code Suggestions ✨

Latest suggestions up to 58ab1c4

Explore these optional code suggestions:

Category **Suggestion                                                                                                                                    ** Impact
General
Handle empty first row edge case
When rows is not empty but contains an empty first row, accessing rows.get(0).size() could return 0 unexpectedly. Add validation to ensure the first row is not empty when inferring column count from rows, or handle this edge case explicitly.

core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java [4495-4502]

 if (explicitTypes != null) {
   nc = explicitTypes.size();
 } else if (explicitNames != null) {
   nc = explicitNames.size();
+} else if (!rows.isEmpty() && !rows.get(0).isEmpty()) {
+  nc = rows.get(0).size();
 } else {
-  nc = rows.isEmpty() ? 0 : rows.get(0).size();
+  nc = 0;
 }

Suggestion importance[1-10]: 7
__

Why: The suggestion addresses a potential edge case where rows is not empty but contains an empty first row. This could prevent unexpected behavior when rows.get(0).size() returns 0, improving robustness.

Low

Previous suggestions

Suggestions up to commit 1257c59

Category **Suggestion                                                                                                                                    ** Impact
General
Detect unterminated CSV quoted fields
The CSV parser doesn't handle the case where a quoted field is not properly closed (unterminated quote). If inQuotes is still true after processing the entire line, it indicates malformed CSV data that should be rejected rather than silently accepted.

ppl/src/main/java/org/opensearch/sql/ppl/utils/MakeResultsDataParser.java [162-190]

 private static List<String> splitCsvLine(String line) {
   List<String> out = new ArrayList<>();
   StringBuilder cur = new StringBuilder();
   boolean inQuotes = false;
   for (int i = 0; i < line.length(); i++) {
     char c = line.charAt(i);
     if (inQuotes) {
       if (c == '"') {
         if (i + 1 < line.length() && line.charAt(i + 1) == '"') {
           cur.append('"');
           i++;
         } else {
           inQuotes = false;
         }
       } else {
         cur.append(c);
       }
     } else if (c == '"') {
       inQuotes = true;
     } else if (c == ',') {
       out.add(cur.toString());
       cur.setLength(0);
     } else {
       cur.append(c);
     }
   }
+  if (inQuotes) {
+    throw new SyntaxCheckException("makeresults CSV has unterminated quoted field");
+  }
   out.add(cur.toString());
   return out;
 }

Suggestion importance[1-10]: 8
__

Why: Valid bug fix for malformed CSV input. The current implementation silently accepts unterminated quoted fields, which could lead to incorrect data parsing. Adding validation for inQuotes after the loop ensures proper CSV format and provides clear error messages for malformed input.

Medium
Security
Validate row count during parsing
The row count validation occurs after parsing the entire data string, which could consume significant memory for large inputs. Consider validating the row count during parsing to fail fast and prevent potential memory exhaustion from malicious or malformed inputs.

ppl/src/main/java/org/opensearch/sql/ppl/utils/MakeResultsDataParser.java [57-59]

-if (result.getRows() != null && result.getRows().size() > 5000) {
-  throw new SyntaxCheckException("makeresults data must not exceed 5000 rows");
-}
+// Validate row count during parsing in parseJson/parseCsv methods
+// to fail fast before materializing all rows in memory

Suggestion importance[1-10]: 7
__

Why: Valid security improvement to prevent memory exhaustion by validating row count during parsing rather than after. However, the implementation would require significant refactoring of the parsing methods, and the current approach with a 29999 character limit already provides some protection.

Medium
Suggestions up to commit 3a8654b

Category **Suggestion                                                                                                                                    ** Impact
Possible issue
Reject oversized integers explicitly
The fallback to STRING for integers exceeding long range may cause unexpected type inconsistencies. Consider throwing a SyntaxCheckException instead to alert users that their integer value is too large, ensuring data integrity and preventing silent type changes.

ppl/src/main/java/org/opensearch/sql/ppl/utils/MakeResultsDataParser.java [193-196]

 if (v.isIntegralNumber()) {
-  // A JSON integer wider than long keeps full precision as a string rather than overflowing.
-  return v.canConvertToLong() ? ExprCoreType.LONG : ExprCoreType.STRING;
+  if (!v.canConvertToLong()) {
+    throw new SyntaxCheckException(
+        "makeresults JSON integer value exceeds long range: " + v.asText());
+  }
+  return ExprCoreType.LONG;
 }

Suggestion importance[1-10]: 5
__

Why: The suggestion proposes throwing an exception for integers exceeding long range instead of falling back to STRING. While this would prevent silent type changes, the current behavior of preserving precision as a string is a reasonable design choice documented in the code comment. The suggestion improves consistency but isn't critical.

Low
General
Validate empty CSV lines properly
Empty lines are silently skipped, which may hide malformed CSV data. Consider validating that skipped lines contain only whitespace, or throw an exception for truly empty lines to ensure data quality and prevent accidental data loss.

ppl/src/main/java/org/opensearch/sql/ppl/utils/MakeResultsDataParser.java [132-135]

 for (int li = 1; li < lines.length; li++) {
-  if (lines[li].isEmpty()) {
+  if (lines[li].trim().isEmpty()) {
     continue;
   }
   List<String> cells = splitCsvLine(lines[li]);
   if (cells.size() > names.size()) {
     throw new SyntaxCheckException(
         "makeresults CSV row has more columns than the header: " + lines[li]);
   }

Suggestion importance[1-10]: 3
__

Why: The suggestion to use trim().isEmpty() instead of isEmpty() is a minor improvement that handles lines with only whitespace more consistently. However, the current behavior is acceptable for typical CSV parsing scenarios and the impact is minimal.

Low
Suggestions up to commit 8e6ecc8

Category **Suggestion                                                                                                                                    ** Impact
Possible issue
Fix unreachable empty array condition
The condition lines.length == 0 can never be true because split() with a limit of -1 always returns at least one element (even for an empty string). This makes the first part of the condition unreachable. Consider checking if the array is empty or if the first element is empty after splitting.

ppl/src/main/java/org/opensearch/sql/ppl/utils/MakeResultsDataParser.java [96-99]

 String[] lines = data.split("\r?\n", -1);
-if (lines.length == 0 || lines[0].trim().isEmpty()) {
+if (lines.length == 0 || lines[0].isEmpty()) {
   throw new SyntaxCheckException("makeresults CSV data must start with a header line");
 }

Suggestion importance[1-10]: 7
__

Why: The suggestion correctly identifies that split() with limit -1 always returns at least one element, making lines.length == 0 unreachable. However, the improved code removes .trim() from the second condition, which changes the behavior from checking if the trimmed first line is empty to checking if the raw first line is empty. This could introduce inconsistency with the intended validation logic.

Medium
Security
Add upper bound for count parameter
When count is zero, an empty array is created and the loop doesn't execute, which is correct. However, when count is very large (e.g., Integer.MAX_VALUE), this could cause an OutOfMemoryError. Consider adding a reasonable upper bound check to prevent memory exhaustion attacks.

core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java [4506-4512]

 int count = node.getCount();
+if (count > 10000) {
+  throw new IllegalArgumentException("makeresults count exceeds maximum allowed value of 10000");
+}
 // Build `count` one-column dummy rows, then project them away with a single _time column set
 // to query time. The dummy column only carries row multiplicity; count=0 yields zero rows.
 Object[] dummy = new Object[count];
 for (int i = 0; i < count; i++) {
   dummy[i] = i;
 }

Suggestion importance[1-10]: 6
__

Why: The suggestion raises a valid concern about potential memory exhaustion with very large count values. However, the specific limit of 10000 is arbitrary and not justified by the PR context. A more appropriate approach would involve configuration-based limits or resource-aware validation rather than a hardcoded constant.

Low
General
Handle whitespace-only lines consistently
Empty lines are skipped but trimmed empty lines are not, which could lead to inconsistent behavior. A line containing only whitespace will be processed and may cause unexpected results. Consider using trim().isEmpty() for consistency with the header check.

ppl/src/main/java/org/opensearch/sql/ppl/utils/MakeResultsDataParser.java [119-127]

 for (int li = 1; li < lines.length; li++) {
-  if (lines[li].isEmpty()) {
+  if (lines[li].trim().isEmpty()) {
     continue;
   }
   String[] cells = lines[li].split(",", -1);
   if (cells.length > names.size()) {
     throw new SyntaxCheckException(
         "makeresults CSV row has more columns than the header: " + lines[li]);
   }

Suggestion importance[1-10]: 5
__

Why: The suggestion identifies a minor inconsistency where whitespace-only lines are not skipped like empty lines. While adding .trim() would improve consistency, this is a relatively minor issue that doesn't affect correctness significantly, as whitespace-only lines would still be parsed (potentially as rows with empty cells).

Low

Also, these edge cases have some point as well.

[songkant-aws]

How about dedupe the field names while processing csv headers? Or we can check if Project can handle them. I remember SqlValidatorUtil.uniquify could append identity numbers to avoid them. For example, 'field, field' names will be uniquified as 'field, field0'

[songkant-aws]

And how about the blank field handling? Like ',field'?

[github-actions]

Persistent review updated to latest commit 9dc6277