Merge remote-tracking branch 'upstream/main' into integ-cleanupcode

Signed-off-by: Yury-Fridlyand <[email protected]>
opensearch-project · Feb 10, 2023 · 5b7e30b · 5b7e30b
2 parents 1b65b8f + 4b3cdbb
commit 5b7e30b
Show file tree

Hide file tree

Showing 12 changed files with 115 additions and 49 deletions.
diff --git a/common/src/main/java/org/opensearch/sql/common/utils/StringUtils.java b/common/src/main/java/org/opensearch/sql/common/utils/StringUtils.java
@@ -12,52 +12,41 @@
 
 public class StringUtils {
   /**
-   * Unquote any string with mark specified.
-   * @param text string
-   * @param mark quotation mark
-   * @return An unquoted string whose outer pair of (single/double/back-tick) quotes have been
-   *     removed
-   */
-  public static String unquote(String text, String mark) {
-    if (isQuoted(text, mark)) {
-      return text.substring(mark.length(), text.length() - mark.length());
-    }
-    return text;
-  }
-
-  /**
-   * Unquote Identifier which has " or ' or ` as mark.
+   * Unquote Identifier which has " or ' as mark.
    * Strings quoted by ' or " with two of these quotes appearing next to each other in the quote
    * acts as an escape
    * Example: 'Test''s' will result in 'Test's', similar with those single quotes being replaced
-   * with double.
+   * with double quote.
+   * Supports escaping quotes (single/double) and escape characters using the `\` characters.
    * @param text string
-   * @return An unquoted string whose outer pair of (single/double/back-tick) quotes have been
+   * @return An unquoted string whose outer pair of (single/double) quotes have been
    *     removed
    */
   public static String unquoteText(String text) {
-
     if (text.length() < 2) {
       return text;
     }
 
-    char enclosingQuote;
+    char enclosingQuote = 0;
     char firstChar = text.charAt(0);
     char lastChar = text.charAt(text.length() - 1);
 
+    if (firstChar != lastChar) {
+      return text;
+    }
+
+    if (firstChar == '`') {
+      return text.substring(1, text.length() - 1);
+    }
+
     if (firstChar == lastChar
         && (firstChar == '\''
-        || firstChar == '"'
-        || firstChar == '`')) {
+        || firstChar == '"')) {
       enclosingQuote = firstChar;
     } else {
       return text;
     }
 
-    if (enclosingQuote == '`') {
-      return text.substring(1, text.length() - 1);
-    }
-
     char currentChar;
     char nextChar;
 
@@ -67,13 +56,18 @@ public static String unquoteText(String text) {
     for (int chIndex = 1; chIndex < text.length() - 1; chIndex++) {
       currentChar = text.charAt(chIndex);
       nextChar = text.charAt(chIndex + 1);
-      if (currentChar == enclosingQuote
-          && nextChar == currentChar) {
+
+      if ((currentChar == '\\'
+          && (nextChar == '"'
+          || nextChar == '\\'
+          || nextChar == '\''))
+          || (currentChar == nextChar
+          && currentChar == enclosingQuote)) {
         chIndex++;
+        currentChar = nextChar;
       }
       textSB.append(currentChar);
     }
-
     return textSB.toString();
   }
 

diff --git a/core/src/test/java/org/opensearch/sql/common/utils/StringUtilsTest.java b/core/src/test/java/org/opensearch/sql/common/utils/StringUtilsTest.java
@@ -10,39 +10,35 @@ class StringUtilsTest {
   void unquoteTest() {
     assertEquals("test", unquoteText("test"));
     assertEquals("test", unquoteText("'test'"));
-    assertEquals("test", unquoteText("`test`"));
 
     assertEquals("test'", unquoteText("'test'''"));
     assertEquals("test\"", unquoteText("\"test\"\"\""));
 
     assertEquals("te``st", unquoteText("'te``st'"));
     assertEquals("te``st", unquoteText("\"te``st\""));
-    assertEquals("te``st", unquoteText("`te``st`"));
 
     assertEquals("te'st", unquoteText("'te''st'"));
     assertEquals("te''st", unquoteText("\"te''st\""));
-    assertEquals("te''st", unquoteText("`te''st`"));
 
     assertEquals("te\"\"st", unquoteText("'te\"\"st'"));
     assertEquals("te\"st", unquoteText("\"te\"\"st\""));
-    assertEquals("te\"\"st", unquoteText("`te\"\"st`"));
 
     assertEquals("''", unquoteText("''''''"));
     assertEquals("\"\"", unquoteText("\"\"\"\"\"\""));
-    assertEquals("````", unquoteText("``````"));
 
     assertEquals("test'", unquoteText("'test''"));
 
     assertEquals("", unquoteText(""));
     assertEquals("'", unquoteText("'"));
-    assertEquals("`", unquoteText("`"));
     assertEquals("\"", unquoteText("\""));
 
     assertEquals("hello'", unquoteText("'hello''"));
     assertEquals("don't", unquoteText("'don't'"));
-    assertEquals("hello`", unquoteText("`hello``"));
     assertEquals("don\"t", unquoteText("\"don\"t\""));
 
+    assertEquals("hel\\lo'", unquoteText("'hel\\lo''"));
+    assertEquals("hel'lo", unquoteText("'hel'lo'"));
+    assertEquals("hel\"lo", unquoteText("\"hel\"lo\""));
+    assertEquals("hel\\'\\lo", unquoteText("'hel\\\\''\\\\lo'"));
   }
-
 }
diff --git a/docs/user/dql/expressions.rst b/docs/user/dql/expressions.rst
@@ -40,6 +40,15 @@ Here is an example for different type of literals::
     | 123   | hello     | False   | -4.567   | 2020-07-07          | 01:01:01          | 2020-07-07 01:01:01               |
     +-------+-----------+---------+----------+---------------------+-------------------+-----------------------------------+
 
+
+    os> SELECT "Hello", 'Hello', "It""s", 'It''s', "It's", '"Its"', 'It\'s', 'It\\\'s', "\I\t\s"
+    fetched rows / total rows = 1/1
+    +-----------+-----------+-----------+-----------+----------+-----------+-----------+-------------+------------+
+    | "Hello"   | 'Hello'   | "It""s"   | 'It''s'   | "It's"   | '"Its"'   | 'It\'s'   | 'It\\\'s'   | "\I\t\s"   |
+    |-----------+-----------+-----------+-----------+----------+-----------+-----------+-------------+------------|
+    | Hello     | Hello     | It"s      | It's      | It's     | "Its"     | It's      | It\'s       | \I\t\s     |
+    +-----------+-----------+-----------+-----------+----------+-----------+-----------+-------------+------------+
+
 Limitations
 -----------
 

diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/PluginIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/PluginIT.java
@@ -418,7 +418,7 @@ public void nonRegisteredSQLSettingsThrowException() throws IOException {
 
     actual = new JSONObject(TestUtils.getResponseBody(response));
     assertThat(actual.getInt("status"), equalTo(400));
-    assertThat(actual.query("/error/type"), equalTo("illegal_argument_exception"));
+    assertThat(actual.query("/error/type"), equalTo("settings_exception"));
     assertThat(
         actual.query("/error/reason"),
         equalTo("transient setting [plugins.sql.query.state.city], not recognized")

diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/MultiMatchIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/MultiMatchIT.java
@@ -23,7 +23,7 @@ public void init() throws IOException {
   @Test
   public void test_multi_match() throws IOException {
     String query = "SOURCE=" + TEST_INDEX_BEER
-        + " | WHERE multi_match([\\\"Tags\\\" ^ 1.5, Title, `Body` 4.2], 'taste') | fields Id";
+        + " | WHERE multi_match([\\\"Tags\\\" ^ 1.5, Title, 'Body' 4.2], 'taste') | fields Id";
     var result = executeQuery(query);
     assertEquals(16, result.getInt("total"));
   }

diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/QueryStringIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/QueryStringIT.java
@@ -20,14 +20,14 @@ public void init() throws IOException {
 
   @Test
   public void all_fields_test() throws IOException {
-    String query = "source=" + TEST_INDEX_BEER + " | where query_string([`*`], 'taste')";
+    String query = "source=" + TEST_INDEX_BEER + " | where query_string(['*'], 'taste')";
     JSONObject result = executeQuery(query);
     assertEquals(16, result.getInt("total"));
   }
 
   @Test
   public void mandatory_params_test() throws IOException {
-    String query = "source=" + TEST_INDEX_BEER + " | where query_string([\\\"Tags\\\" ^ 1.5, Title, `Body` 4.2], 'taste')";
+    String query = "source=" + TEST_INDEX_BEER + " | where query_string([\\\"Tags\\\" ^ 1.5, Title, 'Body' 4.2], 'taste')";
     JSONObject result = executeQuery(query);
     assertEquals(16, result.getInt("total"));
   }

diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/SimpleQueryStringIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/SimpleQueryStringIT.java
@@ -26,7 +26,7 @@ public void init() throws IOException {
   @Test
   public void test_simple_query_string() throws IOException {
     String query = "SOURCE=" + TEST_INDEX_BEER
-        + " | WHERE simple_query_string([\\\"Tags\\\" ^ 1.5, Title, `Body` 4.2], 'taste') | fields Id";
+        + " | WHERE simple_query_string([\\\"Tags\\\" ^ 1.5, Title, 'Body' 4.2], 'taste') | fields Id";
     var result = executeQuery(query);
     assertEquals(16, result.getInt("total"));
   }

diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/MultiMatchIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/MultiMatchIT.java
@@ -29,9 +29,9 @@ public void init() throws IOException {
   */
 
   @Test
-  public void test_mandatory_params() {
+  public void test_mandatory_params() throws IOException {
     String query = "SELECT Id FROM " + TEST_INDEX_BEER
-        + " WHERE multi_match([\\\"Tags\\\" ^ 1.5, Title, `Body` 4.2], 'taste')";
+        + " WHERE multi_match([\\\"Tags\\\" ^ 1.5, Title, 'Body' 4.2], 'taste')";
     JSONObject result = executeJdbcRequest(query);
     assertEquals(16, result.getInt("total"));
   }

diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/QueryStringIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/QueryStringIT.java
@@ -21,15 +21,15 @@ public void init() throws IOException {
   @Test
   public void all_fields_test() throws IOException {
     String query = "SELECT * FROM "
-        + TEST_INDEX_BEER + " WHERE query_string([`*`], 'taste')";
+        + TEST_INDEX_BEER + " WHERE query_string(['*'], 'taste')";
     JSONObject result = executeJdbcRequest(query);
     assertEquals(16, result.getInt("total"));
   }
 
   @Test
   public void mandatory_params_test() throws IOException {
     String query = "SELECT Id FROM "
-        + TEST_INDEX_BEER + " WHERE query_string([\\\"Tags\\\" ^ 1.5, Title, `Body` 4.2], 'taste')";
+        + TEST_INDEX_BEER + " WHERE query_string([\\\"Tags\\\" ^ 1.5, Title, 'Body' 4.2], 'taste')";
     JSONObject result = executeJdbcRequest(query);
     assertEquals(16, result.getInt("total"));
   }

diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/SimpleQueryStringIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/SimpleQueryStringIT.java
@@ -22,14 +22,14 @@ public void init() throws IOException {
   The 'beer.stackexchange' index is a dump of beer.stackexchange.com converted to the format which might be ingested by OpenSearch.
   This is a forum like StackOverflow with questions about beer brewing. The dump contains both questions, answers and comments.
   The reference query is:
-    select count(Id) from beer.stackexchange where simple_query_string(["Tags" ^ 1.5, Title, `Body` 4.2], 'taste') and Tags like '% % %' and Title like '%';
-  It filters out empty `Tags` and `Title`.
+    select count(Id) from beer.stackexchange where simple_query_string(["Tags" ^ 1.5, Title, 'Body' 4.2], 'taste') and Tags like '% % %' and Title like '%';
+  It filters out empty 'Tags' and 'Title'.
   */
 
   @Test
   public void test_mandatory_params() throws IOException {
     String query = "SELECT Id FROM "
-        + TEST_INDEX_BEER + " WHERE simple_query_string([\\\"Tags\\\" ^ 1.5, Title, `Body` 4.2], 'taste')";
+        + TEST_INDEX_BEER + " WHERE simple_query_string([\\\"Tags\\\" ^ 1.5, Title, 'Body' 4.2], 'taste')";
     var result = new JSONObject(executeQuery(query, "jdbc"));
     assertEquals(16, result.getInt("total"));
   }

diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/StringLiteralIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/StringLiteralIT.java
@@ -0,0 +1,67 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+
+package org.opensearch.sql.sql;
+
+import org.json.JSONObject;
+import org.junit.Test;
+import org.opensearch.sql.legacy.SQLIntegTestCase;
+
+import java.io.IOException;
+
+import static org.opensearch.sql.util.MatcherUtils.rows;
+import static org.opensearch.sql.util.MatcherUtils.schema;
+import static org.opensearch.sql.util.MatcherUtils.verifyDataRows;
+import static org.opensearch.sql.util.MatcherUtils.verifySchema;
+
+
+
+public class StringLiteralIT extends SQLIntegTestCase {
+  @Test
+  public void testStringHelloSingleQuote() throws IOException {
+    JSONObject result =
+        executeJdbcRequest("select 'Hello'");
+    verifySchema(result,
+        schema("'Hello'", null, "keyword"));
+    verifyDataRows(result, rows("Hello"));
+  }
+
+  @Test
+  public void testStringHelloDoubleQuote() throws IOException {
+    JSONObject result =
+        executeJdbcRequest("select \\\"Hello\\\"");
+    verifySchema(result,
+        schema("\"Hello\"", null, "keyword"));
+    verifyDataRows(result, rows("Hello"));
+  }
+
+  @Test
+  public void testImStringDoubleDoubleQuoteEscape() throws IOException {
+    JSONObject result =
+        executeJdbcRequest("select \\\"I\\\"\\\"m\\\"");
+    verifySchema(result,
+        schema("\"I\"\"m\"", null, "keyword"));
+    verifyDataRows(result, rows("I\"m"));
+  }
+
+  @Test
+  public void testImStringDoubleSingleQuoteEscape() throws IOException {
+    JSONObject result =
+        executeJdbcRequest("select 'I''m'");
+    verifySchema(result,
+        schema("'I''m'", null, "keyword"));
+    verifyDataRows(result, rows("I'm"));
+  }
+
+  @Test
+  public void testImStringEscapedSingleQuote() throws IOException {
+    JSONObject result =
+        executeJdbcRequest("select 'I\\\\'m'");
+    verifySchema(result,
+        schema("'I\\'m'", null, "keyword"));
+    verifyDataRows(result, rows("I'm"));
+  }
+}
diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/WildcardQueryIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/WildcardQueryIT.java
@@ -139,7 +139,7 @@ public void test_double_escaped_wildcard_underscore() throws IOException {
 
   @Test
   public void test_backslash_wildcard() throws IOException {
-    String query = "SELECT KeywordBody FROM " + TEST_INDEX_WILDCARD + " WHERE wildcard_query(KeywordBody, '*\\\\\\\\\\\\_')";
+    String query = "SELECT KeywordBody FROM " + TEST_INDEX_WILDCARD + " WHERE wildcard_query(KeywordBody, '*\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\_')";
     JSONObject result = executeJdbcRequest(query);
     verifyDataRows(result, rows("test backslash wildcard \\_"));
   }