[Manual backport] Fix: CSV and Raw output, escape quotes (#3148)

* Improve error handling for some more edge cases (#3080) * Add failing tests Signed-off-by: Simeon Widdis <[email protected]> * Fix the first test Signed-off-by: Simeon Widdis <[email protected]> * Revise the tests Signed-off-by: Simeon Widdis <[email protected]> * Fix wildcard tests Signed-off-by: Simeon Widdis <[email protected]> * Add license header Signed-off-by: Simeon Widdis <[email protected]> * Fix rerunning SQL parsing Signed-off-by: Simeon Widdis <[email protected]> --------- Signed-off-by: Simeon Widdis <[email protected]> * Fix: CSV and Raw output, escape quotes (#3063) Fixes #3050 Signed-off-by: Mike Swierczek <[email protected]> (cherry picked from commit cfe38d7) Signed-off-by: Simeon Widdis <[email protected]> * Fix merge conflict Signed-off-by: Simeon Widdis <[email protected]> --------- Signed-off-by: Simeon Widdis <[email protected]> Signed-off-by: Mike Swierczek <[email protected]> Signed-off-by: Simeon Widdis <[email protected]> Co-authored-by: Mike Swierczek <[email protected]>
opensearch-project · Nov 14, 2024 · ce6086b · ce6086b
1 parent 1be7b30
commit ce6086b
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 7 deletions.
diff --git a/protocol/src/main/java/org/opensearch/sql/protocol/response/format/FlatResponseBase.java b/protocol/src/main/java/org/opensearch/sql/protocol/response/format/FlatResponseBase.java
@@ -84,6 +84,10 @@ protected List<List<String>> formatData(List<List<String>> lines) {
 
   protected String quoteIfRequired(String separator, String cell) {
     final String quote = "\"";
-    return cell.contains(separator) ? quote + cell.replaceAll("\"", "\"\"") + quote : cell;
+    if (cell != null && (cell.contains(separator) || cell.contains(quote))) {
+      return quote + cell.replaceAll(quote, quote + quote) + quote;
+    } else {
+      return cell;
+    }
   }
 }
diff --git a/...l/src/test/java/org/opensearch/sql/protocol/response/format/CsvResponseFormatterTest.java b/...l/src/test/java/org/opensearch/sql/protocol/response/format/CsvResponseFormatterTest.java
@@ -108,9 +108,12 @@ void quoteIfRequired() {
     QueryResult response =
         new QueryResult(
             schema,
-            Arrays.asList(tupleValue(ImmutableMap.of("na,me", "John,Smith", ",,age", "30,,,"))));
-    String expected = "\"na,me\",\",,age\"%n" + "\"John,Smith\",\"30,,,\"";
-    assertEquals(format(expected), formatter.format(response));
+            Arrays.asList(
+                tupleValue(ImmutableMap.of("na,me", "John,Smith", ",,age", "30,,,")),
+                tupleValue(ImmutableMap.of("na,me", "\"Janice Jones", ",,age", "26\""))));
+    String expected =
+        "\"na,me\",\",,age\"%n\"John,Smith\",\"30,,,\"%n\"\"\"Janice Jones\",\"26\"\"\"";
+   assertEquals(format(expected), formatter.format(response));
   }
 
   @Test

diff --git a/...l/src/test/java/org/opensearch/sql/protocol/response/format/RawResponseFormatterTest.java b/...l/src/test/java/org/opensearch/sql/protocol/response/format/RawResponseFormatterTest.java
@@ -120,10 +120,18 @@ void quoteIfRequired() {
     QueryResult response =
         new QueryResult(
             schema,
-            Arrays.asList(tupleValue(ImmutableMap.of("na|me", "John|Smith", "||age", "30|||"))));
-    String expected = "\"na|me\"|\"||age\"%n" + "\"John|Smith\"|\"30|||\"";
+            Arrays.asList(
+                tupleValue(ImmutableMap.of("na|me", "John|Smith", "||age", "30|||")),
+                tupleValue(ImmutableMap.of("na|me", "Ja\"ne J\"ones", "||age", "\"40\""))));
+    String expected =
+        "\"na|me\"|\"||age\"%n"
+            + "\"John|Smith\"|\"30|||\"%n"
+            + "\"Ja\"\"ne J\"\"ones\"|\"\"\"40\"\"\"";
     assertEquals(format(expected), getRawFormatter().format(response));
-    String expectedPretty = "\"na|me\"     |\"||age\"%n" + "\"John|Smith\"|\"30|||\"";
+    String expectedPretty =
+        "\"na|me\"         |\"||age\" %n"
+            + "\"John|Smith\"    |\"30|||\" %n"
+            + "\"Ja\"\"ne J\"\"ones\"|\"\"\"40\"\"\"";
     assertEquals(format(expectedPretty), getRawFormatterPretty().format(response));
   }