fix: PostgreSQL supports newline in quoted literals and identifiers (#1731)

olavloite · web-flow · commit f403d99acd21 · 2022-03-08T08:23:47.000+01:00
* fix: PostgreSQL supports newline in quoted literals and identifiers PostgreSQL supports newline characters in string literals and quoted identifiers. Trying to execute a statement with a string literal or quoted identifier that contained a newline character would cause an 'Unclosed string literal' error. Fixes #1730 * fix: typo
diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/AbstractStatementParser.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/AbstractStatementParser.java
@@ -460,7 +460,7 @@ private boolean statementStartsWith(String sql, Iterable<String> checkStatements
   static final char HYPHEN = '-';
   static final char DASH = '#';
   static final char SLASH = '/';
-  static final char ASTERIKS = '*';
+  static final char ASTERISK = '*';
   static final char DOLLAR = '$';
 
   /**
diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/PostgreSQLStatementParser.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/PostgreSQLStatementParser.java
@@ -56,95 +56,44 @@ protected boolean supportsExplain() {
   @Override
   String removeCommentsAndTrimInternal(String sql) {
     Preconditions.checkNotNull(sql);
-    String currentTag = null;
-    boolean isInQuoted = false;
     boolean isInSingleLineComment = false;
     int multiLineCommentLevel = 0;
-    char startQuote = 0;
-    boolean lastCharWasEscapeChar = false;
     StringBuilder res = new StringBuilder(sql.length());
     int index = 0;
     while (index < sql.length()) {
       char c = sql.charAt(index);
-      if (isInQuoted) {
-        if ((c == '\n' || c == '\r') && startQuote != DOLLAR) {
-          throw SpannerExceptionFactory.newSpannerException(
-              ErrorCode.INVALID_ARGUMENT, "SQL statement contains an unclosed literal: " + sql);
-        } else if (c == startQuote) {
-          if (c == DOLLAR) {
-            // Check if this is the end of the current dollar quoted string.
-            String tag = parseDollarQuotedString(sql, index + 1);
-            if (tag != null && tag.equals(currentTag)) {
-              index += tag.length() + 1;
-              res.append(c);
-              res.append(tag);
-              isInQuoted = false;
-              startQuote = 0;
-            }
-          } else if (lastCharWasEscapeChar) {
-            lastCharWasEscapeChar = false;
-          } else if (sql.length() > index + 1 && sql.charAt(index + 1) == startQuote) {
-            // This is an escaped quote (e.g. 'foo''bar')
-            res.append(c);
-            index++;
-          } else {
-            isInQuoted = false;
-            startQuote = 0;
-          }
-        } else if (c == '\\') {
-          lastCharWasEscapeChar = true;
-        } else {
-          lastCharWasEscapeChar = false;
+      if (isInSingleLineComment) {
+        if (c == '\n') {
+          isInSingleLineComment = false;
+          // Include the line feed in the result.
+          res.append(c);
+        }
+      } else if (multiLineCommentLevel > 0) {
+        if (sql.length() > index + 1 && c == ASTERISK && sql.charAt(index + 1) == SLASH) {
+          multiLineCommentLevel--;
+          index++;
+        } else if (sql.length() > index + 1 && c == SLASH && sql.charAt(index + 1) == ASTERISK) {
+          multiLineCommentLevel++;
+          index++;
         }
-        res.append(c);
       } else {
-        // We are not in a quoted string.
-        if (isInSingleLineComment) {
-          if (c == '\n') {
-            isInSingleLineComment = false;
-            // Include the line feed in the result.
-            res.append(c);
-          }
-        } else if (multiLineCommentLevel > 0) {
-          if (sql.length() > index + 1 && c == ASTERIKS && sql.charAt(index + 1) == SLASH) {
-            multiLineCommentLevel--;
-            index++;
-          } else if (sql.length() > index + 1 && c == SLASH && sql.charAt(index + 1) == ASTERIKS) {
-            multiLineCommentLevel++;
-            index++;
-          }
+        // Check for -- which indicates the start of a single-line comment.
+        if (sql.length() > index + 1 && c == HYPHEN && sql.charAt(index + 1) == HYPHEN) {
+          // This is a single line comment.
+          isInSingleLineComment = true;
+          index += 2;
+          continue;
+        } else if (sql.length() > index + 1 && c == SLASH && sql.charAt(index + 1) == ASTERISK) {
+          multiLineCommentLevel++;
+          index += 2;
+          continue;
         } else {
-          // Check for -- which indicates the start of a single-line comment.
-          if (sql.length() > index + 1 && c == HYPHEN && sql.charAt(index + 1) == HYPHEN) {
-            // This is a single line comment.
-            isInSingleLineComment = true;
-          } else if (sql.length() > index + 1 && c == SLASH && sql.charAt(index + 1) == ASTERIKS) {
-            multiLineCommentLevel++;
-            index++;
-          } else {
-            if (c == SINGLE_QUOTE || c == DOUBLE_QUOTE) {
-              isInQuoted = true;
-              startQuote = c;
-            } else if (c == DOLLAR) {
-              currentTag = parseDollarQuotedString(sql, index + 1);
-              if (currentTag != null) {
-                isInQuoted = true;
-                startQuote = DOLLAR;
-                index += currentTag.length() + 1;
-                res.append(c);
-                res.append(currentTag);
-              }
-            }
-            res.append(c);
-          }
+          index = skip(sql, index, res);
+          continue;
         }
       }
       index++;
     }
-    if (isInQuoted) {
-      throw SpannerExceptionFactory.newSpannerException(
-          ErrorCode.INVALID_ARGUMENT, "SQL statement contains an unclosed literal: " + sql);
-    }
     if (multiLineCommentLevel > 0) {
       throw SpannerExceptionFactory.newSpannerException(
           ErrorCode.INVALID_ARGUMENT,
@@ -184,73 +133,78 @@ String removeStatementHint(String sql) {
   ParametersInfo convertPositionalParametersToNamedParametersInternal(char paramChar, String sql) {
     Preconditions.checkNotNull(sql);
     final String namedParamPrefix = "$";
-    String currentTag = null;
-    boolean isInQuoted = false;
-    char startQuote = 0;
-    boolean lastCharWasEscapeChar = false;
     StringBuilder named = new StringBuilder(sql.length() + countOccurrencesOf(paramChar, sql));
     int index = 0;
     int paramIndex = 1;
     while (index < sql.length()) {
       char c = sql.charAt(index);
-      if (isInQuoted) {
-        if ((c == '\n' || c == '\r') && startQuote != DOLLAR) {
-          throw SpannerExceptionFactory.newSpannerException(
-              ErrorCode.INVALID_ARGUMENT, "SQL statement contains an unclosed literal: " + sql);
-        } else if (c == startQuote) {
-          if (c == DOLLAR) {
-            // Check if this is the end of the current dollar quoted string.
-            String tag = parseDollarQuotedString(sql, index + 1);
-            if (tag != null && tag.equals(currentTag)) {
-              index += tag.length() + 1;
-              named.append(c);
-              named.append(tag);
-              isInQuoted = false;
-              startQuote = 0;
-            }
-          } else if (lastCharWasEscapeChar) {
-            lastCharWasEscapeChar = false;
-          } else if (sql.length() > index + 1 && sql.charAt(index + 1) == startQuote) {
-            // This is an escaped quote (e.g. 'foo''bar')
-            named.append(c);
-            index++;
-          } else {
-            isInQuoted = false;
-            startQuote = 0;
+      if (c == paramChar) {
+        named.append(namedParamPrefix).append(paramIndex);
+        paramIndex++;
+        index++;
+      } else {
+        index = skip(sql, index, named);
+      }
+    }
+    return new ParametersInfo(paramIndex - 1, named.toString());
+  }
+
+  private int skip(String sql, int currentIndex, StringBuilder result) {
+    char currentChar = sql.charAt(currentIndex);
+    if (currentChar == SINGLE_QUOTE || currentChar == DOUBLE_QUOTE) {
+      result.append(currentChar);
+      return skipQuoted(sql, currentIndex, currentChar, result);
+    } else if (currentChar == DOLLAR) {
+      String dollarTag = parseDollarQuotedString(sql, currentIndex + 1);
+      if (dollarTag != null) {
+        result.append(currentChar).append(dollarTag).append(currentChar);
+        return skipQuoted(
+            sql, currentIndex + dollarTag.length() + 1, currentChar, dollarTag, result);
+      }
+    }
+
+    result.append(currentChar);
+    return currentIndex + 1;
+  }
+
+  private int skipQuoted(String sql, int startIndex, char startQuote, StringBuilder result) {
+    return skipQuoted(sql, startIndex, startQuote, null, result);
+  }
+
+  private int skipQuoted(
+      String sql, int startIndex, char startQuote, String dollarTag, StringBuilder result) {
+    boolean lastCharWasEscapeChar = false;
+    int currentIndex = startIndex + 1;
+    while (currentIndex < sql.length()) {
+      char currentChar = sql.charAt(currentIndex);
+      if (currentChar == startQuote) {
+        if (currentChar == DOLLAR) {
+          // Check if this is the end of the current dollar quoted string.
+          String tag = parseDollarQuotedString(sql, currentIndex + 1);
+          if (tag != null && tag.equals(dollarTag)) {
+            result.append(currentChar).append(tag).append(currentChar);
+            return currentIndex + tag.length() + 2;
           }
-        } else if (c == '\\') {
-          lastCharWasEscapeChar = true;
-        } else {
+        } else if (lastCharWasEscapeChar) {
           lastCharWasEscapeChar = false;
-        }
-        named.append(c);
-      } else {
-        if (c == paramChar) {
-          named.append(namedParamPrefix + paramIndex);
-          paramIndex++;
+        } else if (sql.length() > currentIndex + 1 && sql.charAt(currentIndex + 1) == startQuote) {
+          // This is an escaped quote (e.g. 'foo''bar')
+          result.append(currentChar).append(currentChar);
+          currentIndex += 2;
+          continue;
         } else {
-          if (c == SINGLE_QUOTE || c == DOUBLE_QUOTE) {
-            isInQuoted = true;
-            startQuote = c;
-          } else if (c == DOLLAR) {
-            currentTag = parseDollarQuotedString(sql, index + 1);
-            if (currentTag != null) {
-              isInQuoted = true;
-              startQuote = DOLLAR;
-              index += currentTag.length() + 1;
-              named.append(c);
-              named.append(currentTag);
-            }
-          }
-          named.append(c);
+          result.append(currentChar);
+          return currentIndex + 1;
         }
+      } else if (currentChar == '\\') {
+        lastCharWasEscapeChar = true;
+      } else {
+        lastCharWasEscapeChar = false;
       }
-      index++;
-    }
-    if (isInQuoted) {
-      throw SpannerExceptionFactory.newSpannerException(
-          ErrorCode.INVALID_ARGUMENT, "SQL statement contains an unclosed literal: " + sql);
+      currentIndex++;
+      result.append(currentChar);
     }
-    return new ParametersInfo(paramIndex - 1, named.toString());
+    throw SpannerExceptionFactory.newSpannerException(
+        ErrorCode.INVALID_ARGUMENT, "SQL statement contains an unclosed literal: " + sql);
   }
 }
diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/SpannerStatementParser.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/SpannerStatementParser.java
@@ -106,7 +106,7 @@ String removeCommentsAndTrimInternal(String sql) {
             res.append(c);
           }
         } else if (isInMultiLineComment) {
-          if (sql.length() > index + 1 && c == ASTERIKS && sql.charAt(index + 1) == SLASH) {
+          if (sql.length() > index + 1 && c == ASTERISK && sql.charAt(index + 1) == SLASH) {
             isInMultiLineComment = false;
             index++;
           }
@@ -115,7 +115,7 @@ String removeCommentsAndTrimInternal(String sql) {
               || (sql.length() > index + 1 && c == HYPHEN && sql.charAt(index + 1) == HYPHEN)) {
             // This is a single line comment.
             isInSingleLineComment = true;
-          } else if (sql.length() > index + 1 && c == SLASH && sql.charAt(index + 1) == ASTERIKS) {
+          } else if (sql.length() > index + 1 && c == SLASH && sql.charAt(index + 1) == ASTERISK) {
             isInMultiLineComment = true;
             index++;
           } else {
diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/StatementParserTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/StatementParserTest.java
@@ -127,6 +127,33 @@ public void testRemoveComments() {
             parser.removeCommentsAndTrim(
                 "/* This\nis\na\nmulti\nline\ncomment */\nSELECT * FROM FOO"))
         .isEqualTo("SELECT * FROM FOO");
+
+    assertEquals(
+        "SELECT \"FOO\" FROM \"BAR\" WHERE name='test'",
+        parser.removeCommentsAndTrim(
+            "-- Single line comment\nSELECT \"FOO\" FROM \"BAR\" WHERE name='test'"));
+    assertEquals(
+        "SELECT \"FOO\" FROM \"BAR\" WHERE name='test' and id=1",
+        parser.removeCommentsAndTrim(
+            "/* Multi\n"
+                + "line\n"
+                + "comment\n"
+                + "*/SELECT \"FOO\" FROM \"BAR\" WHERE name='test' and id=1"));
+
+    if (dialect == Dialect.POSTGRESQL) {
+      // PostgreSQL allows string literals and quoted identifiers to contain newline characters.
+      assertEquals(
+          "SELECT \"FOO\nBAR\" FROM \"BAR\" WHERE name='test\ntest'",
+          parser.removeCommentsAndTrim(
+              "-- Single line comment\nSELECT \"FOO\nBAR\" FROM \"BAR\" WHERE name='test\ntest'"));
+      assertEquals(
+          "SELECT \"FOO\nBAR\" FROM \"BAR\" WHERE name='test\ntest' and id=1",
+          parser.removeCommentsAndTrim(
+              "/* Multi\n"
+                  + "line\n"
+                  + "comment\n"
+                  + "*/SELECT \"FOO\nBAR\" FROM \"BAR\" WHERE name='test\ntest' and id=1"));
+    }
   }
 
   @Test
@@ -1221,9 +1248,16 @@ public void testPostgreSQLDialectDialectConvertPositionalParametersToNamedParame
                 .sqlWithNamedParameters)
         .isEqualTo("$1$$?it\\'?s \n ?it\\'?s$$$2");
 
-    assertUnclosedLiteral("?'?it\\'?s \n ?it\\'?s'?");
+    // Note: PostgreSQL allows a single-quoted string literal to contain line feeds.
+    assertEquals(
+        "$1'?it\\'?s \n ?it\\'?s'$2",
+        parser.convertPositionalParametersToNamedParameters('?', "?'?it\\'?s \n ?it\\'?s'?")
+            .sqlWithNamedParameters);
     assertUnclosedLiteral("?'?it\\'?s \n ?it\\'?s?");
-    assertUnclosedLiteral("?'''?it\\'?s \n ?it\\'?s'?");
+    assertEquals(
+        "$1'''?it\\'?s \n ?it\\'?s'$2",
+        parser.convertPositionalParametersToNamedParameters('?', "?'''?it\\'?s \n ?it\\'?s'?")
+            .sqlWithNamedParameters);
 
     assertThat(
         parser.convertPositionalParametersToNamedParameters(
