From 50e288791bf94903cc76e0c5a703a936cc9a5d3e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= <koloite@gmail.com>
Date: Tue, 19 Mar 2024 11:42:17 +0100
Subject: [PATCH] perf: keep comments when searching for params (#2951)

Keep all comments in the SQL string in place when converting positional parameters to named parameters. This reduces the amount of string operations that are needed for each query that is executed, and also enables actually sending comments from the client to Spanner when using positional parameters (e.g. in JDBC).

This is step 3 in the refactoring to share more code between the SpannerStatementParser and PostgreSQLStatementParser.
---
 .../connection/AbstractStatementParser.java   |  49 +--
 .../connection/PostgreSQLStatementParser.java |  26 +-
 .../connection/SpannerStatementParser.java    |  67 +---
 .../SpannerStatementParserTest.java           | 156 ++++++++
 .../connection/StatementParserTest.java       | 362 ++++++++----------
 5 files changed, 361 insertions(+), 299 deletions(-)

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
index 13301181452..9793a50c636 100644
--- 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
@@ -634,36 +634,33 @@ public static class ParametersInfo {
 
   /**
    * Converts all positional parameters (?) in the given sql string into named parameters. The
-   * parameters are named @p1, @p2, etc. This method is used when converting a JDBC statement that
-   * uses positional parameters to a Cloud Spanner {@link Statement} instance that requires named
-   * parameters. The input SQL string may not contain any comments, except for PostgreSQL-dialect
-   * SQL strings.
+   * parameters are named @p1, @p2, etc. for GoogleSQL, and $1, $2, etc. for PostgreSQL. This method
+   * is used when converting a JDBC statement that uses positional parameters to a Cloud Spanner
+   * {@link Statement} instance that requires named parameters.
    *
-   * @param sql The sql string that should be converted
-   * @return A {@link ParametersInfo} object containing a string with named parameters instead of
-   *     positional parameters and the number of parameters.
-   * @throws SpannerException If the input sql string contains an unclosed string/byte literal.
-   */
-  @InternalApi
-  abstract ParametersInfo convertPositionalParametersToNamedParametersInternal(
-      char paramChar, String sql);
-
-  /**
-   * Converts all positional parameters (?) in the given sql string into named parameters. The
-   * parameters are named @p1, @p2, etc. This method is used when converting a JDBC statement that
-   * uses positional parameters to a Cloud Spanner {@link Statement} instance that requires named
-   * parameters. The input SQL string may not contain any comments. There is an exception case if
-   * the statement starts with a GSQL comment which forces it to be interpreted as a GoogleSql
-   * statement.
-   *
-   * @param sql The sql string without comments that should be converted
+   * @param sql The sql string that should be converted to use named parameters
    * @return A {@link ParametersInfo} object containing a string with named parameters instead of
    *     positional parameters and the number of parameters.
    * @throws SpannerException If the input sql string contains an unclosed string/byte literal.
    */
   @InternalApi
   public ParametersInfo convertPositionalParametersToNamedParameters(char paramChar, String sql) {
-    return convertPositionalParametersToNamedParametersInternal(paramChar, sql);
+    Preconditions.checkNotNull(sql);
+    final String namedParamPrefix = getQueryParameterPrefix();
+    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 (c == paramChar) {
+        named.append(namedParamPrefix).append(paramIndex);
+        paramIndex++;
+        index++;
+      } else {
+        index = skip(sql, index, named);
+      }
+    }
+    return new ParametersInfo(paramIndex - 1, named.toString());
   }
 
   /** Convenience method that is used to estimate the number of parameters in a SQL statement. */
@@ -700,7 +697,8 @@ public boolean checkReturningClause(String sql) {
   }
 
   /**
-   * <<<<<<< HEAD Returns true if this dialect supports nested comments.
+   * <<<<<<< HEAD Returns true if this dialect supports nested comments. ======= <<<<<<< HEAD
+   * Returns true if this dialect supports nested comments. >>>>>>> main
    *
    * <ul>
    *   <li>This method should return false for dialects that consider this to be a valid comment:
@@ -755,6 +753,9 @@ public boolean checkReturningClause(String sql) {
    */
   abstract boolean supportsLineFeedInQuotedString();
 
+  /** Returns the query parameter prefix that should be used for this dialect. */
+  abstract String getQueryParameterPrefix();
+
   /**
    * Returns true for characters that can be used as the first character in unquoted identifiers.
    */
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
index 6b0c69d40a9..be4aa9d7f46 100644
--- 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
@@ -88,6 +88,11 @@ boolean supportsLineFeedInQuotedString() {
     return true;
   }
 
+  @Override
+  String getQueryParameterPrefix() {
+    return "$";
+  }
+
   /**
    * Removes comments from and trims the given sql statement. PostgreSQL supports two types of
    * comments:
@@ -181,27 +186,6 @@ String removeStatementHint(String sql) {
     return sql;
   }
 
-  @InternalApi
-  @Override
-  ParametersInfo convertPositionalParametersToNamedParametersInternal(char paramChar, String sql) {
-    Preconditions.checkNotNull(sql);
-    final String namedParamPrefix = "$";
-    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 (c == paramChar) {
-        named.append(namedParamPrefix).append(paramIndex);
-        paramIndex++;
-        index++;
-      } else {
-        index = skip(sql, index, named);
-      }
-    }
-    return new ParametersInfo(paramIndex - 1, named.toString());
-  }
-
   /**
    * Note: This is an internal API and breaking changes can be made without prior notice.
    *
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
index 1c5cdda7b01..892672ad0df 100644
--- 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
@@ -90,6 +90,11 @@ boolean supportsLineFeedInQuotedString() {
     return false;
   }
 
+  @Override
+  String getQueryParameterPrefix() {
+    return "@p";
+  }
+
   /**
    * Removes comments from and trims the given sql statement. Spanner supports three types of
    * comments:
@@ -250,68 +255,6 @@ String removeStatementHint(String sql) {
     return sql;
   }
 
-  @InternalApi
-  @Override
-  ParametersInfo convertPositionalParametersToNamedParametersInternal(char paramChar, String sql) {
-    boolean isInQuoted = false;
-    char startQuote = 0;
-    boolean lastCharWasEscapeChar = false;
-    boolean isTripleQuoted = false;
-    int paramIndex = 1;
-    StringBuilder named = new StringBuilder(sql.length() + countOccurrencesOf(paramChar, sql));
-    for (int index = 0; index < sql.length(); index++) {
-      char c = sql.charAt(index);
-      if (isInQuoted) {
-        if ((c == '\n' || c == '\r') && !isTripleQuoted) {
-          throw SpannerExceptionFactory.newSpannerException(
-              ErrorCode.INVALID_ARGUMENT, "SQL statement contains an unclosed literal: " + sql);
-        } else if (c == startQuote) {
-          if (lastCharWasEscapeChar) {
-            lastCharWasEscapeChar = false;
-          } else if (isTripleQuoted) {
-            if (sql.length() > index + 2
-                && sql.charAt(index + 1) == startQuote
-                && sql.charAt(index + 2) == startQuote) {
-              isInQuoted = false;
-              startQuote = 0;
-              isTripleQuoted = false;
-            }
-          } else {
-            isInQuoted = false;
-            startQuote = 0;
-          }
-        } else if (c == '\\') {
-          lastCharWasEscapeChar = true;
-        } else {
-          lastCharWasEscapeChar = false;
-        }
-        named.append(c);
-      } else {
-        if (c == paramChar) {
-          named.append("@p" + paramIndex);
-          paramIndex++;
-        } else {
-          if (c == SINGLE_QUOTE || c == DOUBLE_QUOTE || c == BACKTICK_QUOTE) {
-            isInQuoted = true;
-            startQuote = c;
-            // check whether it is a triple-quote
-            if (sql.length() > index + 2
-                && sql.charAt(index + 1) == startQuote
-                && sql.charAt(index + 2) == startQuote) {
-              isTripleQuoted = true;
-            }
-          }
-          named.append(c);
-        }
-      }
-    }
-    if (isInQuoted) {
-      throw SpannerExceptionFactory.newSpannerException(
-          ErrorCode.INVALID_ARGUMENT, "SQL statement contains an unclosed literal: " + sql);
-    }
-    return new ParametersInfo(paramIndex - 1, named.toString());
-  }
-
   private boolean isReturning(String sql, int index) {
     return (index >= 1)
         && (index + 12 <= sql.length())
diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/SpannerStatementParserTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/SpannerStatementParserTest.java
index d4dc76d48bb..5cec5d838d1 100644
--- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/SpannerStatementParserTest.java
+++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/SpannerStatementParserTest.java
@@ -16,9 +16,11 @@
 
 package com.google.cloud.spanner.connection;
 
+import static com.google.cloud.spanner.connection.StatementParserTest.assertUnclosedLiteral;
 import static org.junit.Assert.assertEquals;
 
 import com.google.cloud.spanner.Dialect;
+import com.google.cloud.spanner.connection.StatementParserTest.CommentInjector;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -80,4 +82,158 @@ public void testSkip() {
     assertEquals("'foo\\''", skip("r'foo\\''  ", 1));
     assertEquals("'''foo\\'\\'\\'bar'''", skip("'''foo\\'\\'\\'bar'''  ", 0));
   }
+
+  @Test
+  public void testConvertPositionalParametersToNamedParameters() {
+    AbstractStatementParser parser =
+        AbstractStatementParser.getInstance(Dialect.GOOGLE_STANDARD_SQL);
+
+    for (String comment :
+        new String[] {
+          "-- test comment\n",
+          "/* another test comment */",
+          "/* comment\nwith\nmultiple\nlines\n */",
+          "/* comment /* with nested */ comment */"
+        }) {
+      for (CommentInjector injector : CommentInjector.values()) {
+        assertEquals(
+            injector.inject("select * %sfrom foo where name=@p1", comment),
+            parser.convertPositionalParametersToNamedParameters(
+                    '?', injector.inject("select * %sfrom foo where name=?", comment))
+                .sqlWithNamedParameters);
+        assertEquals(
+            injector.inject("@p1%s'?test?\"?test?\"?'@p2", comment),
+            parser.convertPositionalParametersToNamedParameters(
+                    '?', injector.inject("?%s'?test?\"?test?\"?'?", comment))
+                .sqlWithNamedParameters);
+        assertEquals(
+            injector.inject("@p1'?it\\'?s'%s@p2", comment),
+            parser.convertPositionalParametersToNamedParameters(
+                    '?', injector.inject("?'?it\\'?s'%s?", comment))
+                .sqlWithNamedParameters);
+        assertEquals(
+            injector.inject("@p1'?it\\\"?s'%s@p2", comment),
+            parser.convertPositionalParametersToNamedParameters(
+                    '?', injector.inject("?'?it\\\"?s'%s?", comment))
+                .sqlWithNamedParameters);
+        assertEquals(
+            injector.inject("@p1\"?it\\\"?s\"%s@p2", comment),
+            parser.convertPositionalParametersToNamedParameters(
+                    '?', injector.inject("?\"?it\\\"?s\"%s?", comment))
+                .sqlWithNamedParameters);
+        assertEquals(
+            injector.inject("@p1%s'''?it\\''?s'''@p2", comment),
+            parser.convertPositionalParametersToNamedParameters(
+                    '?', injector.inject("?%s'''?it\\''?s'''?", comment))
+                .sqlWithNamedParameters);
+        assertEquals(
+            injector.inject("@p1\"\"\"?it\\\"\"?s\"\"\"%s@p2", comment),
+            parser.convertPositionalParametersToNamedParameters(
+                    '?', injector.inject("?\"\"\"?it\\\"\"?s\"\"\"%s?", comment))
+                .sqlWithNamedParameters);
+
+        // GoogleSQL does not support dollar-quoted strings, so these are all ignored.
+        assertEquals(
+            injector.inject("@p1$$@p2it$@p3s$$%s@p4", comment),
+            parser.convertPositionalParametersToNamedParameters(
+                    '?', injector.inject("?$$?it$?s$$%s?", comment))
+                .sqlWithNamedParameters);
+        assertEquals(
+            injector.inject("@p1$tag$@p2it$$@p3s$tag$%s@p4", comment),
+            parser.convertPositionalParametersToNamedParameters(
+                    '?', injector.inject("?$tag$?it$$?s$tag$%s?", comment))
+                .sqlWithNamedParameters);
+        assertEquals(
+            injector.inject("@p1%s$$@p2it\\'?s \t ?it\\'?s'$$@p3", comment),
+            parser.convertPositionalParametersToNamedParameters(
+                    '?', injector.inject("?%s$$?it\\'?s \t ?it\\'?s'$$?", comment))
+                .sqlWithNamedParameters);
+
+        // Note: GoogleSQL does not allowa a single-quoted string literal to contain line feeds.
+        assertUnclosedLiteral(parser, injector.inject("?'?it\\''?s \n ?it\\''?s'%s?", comment));
+        assertEquals(
+            "@p1'?it\\''@p2s \n @p3it\\''@p4s@p5",
+            parser.convertPositionalParametersToNamedParameters('?', "?'?it\\''?s \n ?it\\''?s?")
+                .sqlWithNamedParameters);
+        assertEquals(
+            injector.inject("@p1%s'''?it\\''?s \n ?it\\''?s'''@p2", comment),
+            parser.convertPositionalParametersToNamedParameters(
+                    '?', injector.inject("?%s'''?it\\''?s \n ?it\\''?s'''?", comment))
+                .sqlWithNamedParameters);
+
+        assertEquals(
+            injector.inject(
+                "select 1, @p1, 'test?test', \"test?test\", %sfoo.* from `foo` where col1=@p2 and col2='test' and col3=@p3 and col4='?' and col5=\"?\" and col6='?''?''?'",
+                comment),
+            parser.convertPositionalParametersToNamedParameters(
+                    '?',
+                    injector.inject(
+                        "select 1, ?, 'test?test', \"test?test\", %sfoo.* from `foo` where col1=? and col2='test' and col3=? and col4='?' and col5=\"?\" and col6='?''?''?'",
+                        comment))
+                .sqlWithNamedParameters);
+
+        assertEquals(
+            injector.inject(
+                "select * "
+                    + "%sfrom foo "
+                    + "where name=@p1 "
+                    + "and col2 like @p2 "
+                    + "and col3 > @p3",
+                comment),
+            parser.convertPositionalParametersToNamedParameters(
+                    '?',
+                    injector.inject(
+                        "select * "
+                            + "%sfrom foo "
+                            + "where name=? "
+                            + "and col2 like ? "
+                            + "and col3 > ?",
+                        comment))
+                .sqlWithNamedParameters);
+        assertEquals(
+            injector.inject("select * " + "from foo " + "where id between @p1%s and @p2", comment),
+            parser.convertPositionalParametersToNamedParameters(
+                    '?',
+                    injector.inject(
+                        "select * " + "from foo " + "where id between ?%s and ?", comment))
+                .sqlWithNamedParameters);
+        assertEquals(
+            injector.inject("select * " + "from foo " + "limit @p1 %s offset @p2", comment),
+            parser.convertPositionalParametersToNamedParameters(
+                    '?',
+                    injector.inject("select * " + "from foo " + "limit ? %s offset ?", comment))
+                .sqlWithNamedParameters);
+        assertEquals(
+            injector.inject(
+                "select * "
+                    + "from foo "
+                    + "where col1=@p1 "
+                    + "and col2 like @p2 "
+                    + " %s "
+                    + "and col3 > @p3 "
+                    + "and col4 < @p4 "
+                    + "and col5 != @p5 "
+                    + "and col6 not in (@p6, @p7, @p8) "
+                    + "and col7 in (@p9, @p10, @p11) "
+                    + "and col8 between @p12 and @p13",
+                comment),
+            parser.convertPositionalParametersToNamedParameters(
+                    '?',
+                    injector.inject(
+                        "select * "
+                            + "from foo "
+                            + "where col1=? "
+                            + "and col2 like ? "
+                            + " %s "
+                            + "and col3 > ? "
+                            + "and col4 < ? "
+                            + "and col5 != ? "
+                            + "and col6 not in (?, ?, ?) "
+                            + "and col7 in (?, ?, ?) "
+                            + "and col8 between ? and ?",
+                        comment))
+                .sqlWithNamedParameters);
+      }
+    }
+  }
 }
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
index c60550c3ba6..3739aa11064 100644
--- 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
@@ -17,12 +17,11 @@
 package com.google.cloud.spanner.connection;
 
 import static com.google.common.truth.Truth.assertThat;
-import static org.hamcrest.CoreMatchers.equalTo;
-import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotSame;
+import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 import static org.junit.Assume.assumeTrue;
@@ -1066,85 +1065,86 @@ private <T extends ClientSideStatementImpl> void testParseStatementWithOneParame
   public void testGoogleStandardSQLDialectConvertPositionalParametersToNamedParameters() {
     assumeTrue(dialect == Dialect.GOOGLE_STANDARD_SQL);
 
-    assertThat(
-            parser.convertPositionalParametersToNamedParameters(
-                    '?', "select * from foo where name=?")
-                .sqlWithNamedParameters)
-        .isEqualTo("select * from foo where name=@p1");
-    assertThat(
-            parser.convertPositionalParametersToNamedParameters('?', "?'?test?\"?test?\"?'?")
-                .sqlWithNamedParameters)
-        .isEqualTo("@p1'?test?\"?test?\"?'@p2");
-    assertThat(
-            parser.convertPositionalParametersToNamedParameters('?', "?'?it\\'?s'?")
-                .sqlWithNamedParameters)
-        .isEqualTo("@p1'?it\\'?s'@p2");
-    assertThat(
-            parser.convertPositionalParametersToNamedParameters('?', "?'?it\\\"?s'?")
-                .sqlWithNamedParameters)
-        .isEqualTo("@p1'?it\\\"?s'@p2");
-    assertThat(
-            parser.convertPositionalParametersToNamedParameters('?', "?\"?it\\\"?s\"?")
-                .sqlWithNamedParameters)
-        .isEqualTo("@p1\"?it\\\"?s\"@p2");
-    assertThat(
-            parser.convertPositionalParametersToNamedParameters('?', "?'''?it\\'?s'''?")
-                .sqlWithNamedParameters)
-        .isEqualTo("@p1'''?it\\'?s'''@p2");
-    assertThat(
-            parser.convertPositionalParametersToNamedParameters('?', "?\"\"\"?it\\\"?s\"\"\"?")
-                .sqlWithNamedParameters)
-        .isEqualTo("@p1\"\"\"?it\\\"?s\"\"\"@p2");
+    assertEquals(
+        "select * from foo where name=@p1",
+        parser.convertPositionalParametersToNamedParameters('?', "select * from foo where name=?")
+            .sqlWithNamedParameters);
+    assertEquals(
+        "@p1'?test?\"?test?\"?'@p2",
+        parser.convertPositionalParametersToNamedParameters('?', "?'?test?\"?test?\"?'?")
+            .sqlWithNamedParameters);
+    assertEquals(
+        "@p1'?it\\'?s'@p2",
+        parser.convertPositionalParametersToNamedParameters('?', "?'?it\\'?s'?")
+            .sqlWithNamedParameters);
+    assertEquals(
+        "@p1'?it\\\"?s'@p2",
+        parser.convertPositionalParametersToNamedParameters('?', "?'?it\\\"?s'?")
+            .sqlWithNamedParameters);
+    assertEquals(
+        "@p1\"?it\\\"?s\"@p2",
+        parser.convertPositionalParametersToNamedParameters('?', "?\"?it\\\"?s\"?")
+            .sqlWithNamedParameters);
+    assertEquals(
+        "@p1'''?it\\'?s'''@p2",
+        parser.convertPositionalParametersToNamedParameters('?', "?'''?it\\'?s'''?")
+            .sqlWithNamedParameters);
+    assertEquals(
+        "@p1\"\"\"?it\\\"?s\"\"\"@p2",
+        parser.convertPositionalParametersToNamedParameters('?', "?\"\"\"?it\\\"?s\"\"\"?")
+            .sqlWithNamedParameters);
 
-    assertThat(
-            parser.convertPositionalParametersToNamedParameters('?', "?`?it\\`?s`?")
-                .sqlWithNamedParameters)
-        .isEqualTo("@p1`?it\\`?s`@p2");
-    assertThat(
-            parser.convertPositionalParametersToNamedParameters('?', "?```?it\\`?s```?")
-                .sqlWithNamedParameters)
-        .isEqualTo("@p1```?it\\`?s```@p2");
-    assertThat(
-            parser.convertPositionalParametersToNamedParameters('?', "?'''?it\\'?s \n ?it\\'?s'''?")
-                .sqlWithNamedParameters)
-        .isEqualTo("@p1'''?it\\'?s \n ?it\\'?s'''@p2");
+    assertEquals(
+        "@p1`?it\\`?s`@p2",
+        parser.convertPositionalParametersToNamedParameters('?', "?`?it\\`?s`?")
+            .sqlWithNamedParameters);
+    assertEquals(
+        "@p1```?it\\`?s```@p2",
+        parser.convertPositionalParametersToNamedParameters('?', "?```?it\\`?s```?")
+            .sqlWithNamedParameters);
+    assertEquals(
+        "@p1'''?it\\'?s \n ?it\\'?s'''@p2",
+        parser.convertPositionalParametersToNamedParameters('?', "?'''?it\\'?s \n ?it\\'?s'''?")
+            .sqlWithNamedParameters);
 
-    assertUnclosedLiteral("?'?it\\'?s \n ?it\\'?s'?");
-    assertUnclosedLiteral("?'?it\\'?s \n ?it\\'?s?");
-    assertUnclosedLiteral("?'''?it\\'?s \n ?it\\'?s'?");
+    assertUnclosedLiteral(parser, "?'?it\\'?s \n ?it\\'?s'?");
+    assertUnclosedLiteral(parser, "?'?it\\'?s \n ?it\\'?s?");
+    assertUnclosedLiteral(parser, "?'''?it\\'?s \n ?it\\'?s'?");
 
-    assertThat(
+    assertEquals(
+        "select 1, @p1, 'test?test', \"test?test\", foo.* from `foo` where col1=@p2 and col2='test' and col3=@p3 and col4='?' and col5=\"?\" and col6='?''?''?'",
         parser.convertPositionalParametersToNamedParameters(
                 '?',
                 "select 1, ?, 'test?test', \"test?test\", foo.* from `foo` where col1=? and col2='test' and col3=? and col4='?' and col5=\"?\" and col6='?''?''?'")
-            .sqlWithNamedParameters,
-        is(
-            equalTo(
-                "select 1, @p1, 'test?test', \"test?test\", foo.* from `foo` where col1=@p2 and col2='test' and col3=@p3 and col4='?' and col5=\"?\" and col6='?''?''?'")));
+            .sqlWithNamedParameters);
 
-    assertThat(
+    assertEquals(
+        "select * " + "from foo " + "where name=@p1 " + "and col2 like @p2 " + "and col3 > @p3",
         parser.convertPositionalParametersToNamedParameters(
                 '?',
                 "select * " + "from foo " + "where name=? " + "and col2 like ? " + "and col3 > ?")
-            .sqlWithNamedParameters,
-        is(
-            equalTo(
-                "select * "
-                    + "from foo "
-                    + "where name=@p1 "
-                    + "and col2 like @p2 "
-                    + "and col3 > @p3")));
-    assertThat(
+            .sqlWithNamedParameters);
+    assertEquals(
+        "select * " + "from foo " + "where id between @p1 and @p2",
         parser.convertPositionalParametersToNamedParameters(
                 '?', "select * " + "from foo " + "where id between ? and ?")
-            .sqlWithNamedParameters,
-        is(equalTo("select * " + "from foo " + "where id between @p1 and @p2")));
-    assertThat(
+            .sqlWithNamedParameters);
+    assertEquals(
+        "select * " + "from foo " + "limit @p1 offset @p2",
         parser.convertPositionalParametersToNamedParameters(
                 '?', "select * " + "from foo " + "limit ? offset ?")
-            .sqlWithNamedParameters,
-        is(equalTo("select * " + "from foo " + "limit @p1 offset @p2")));
-    assertThat(
+            .sqlWithNamedParameters);
+    assertEquals(
+        "select * "
+            + "from foo "
+            + "where col1=@p1 "
+            + "and col2 like @p2 "
+            + "and col3 > @p3 "
+            + "and col4 < @p4 "
+            + "and col5 != @p5 "
+            + "and col6 not in (@p6, @p7, @p8) "
+            + "and col7 in (@p9, @p10, @p11) "
+            + "and col8 between @p12 and @p13",
         parser.convertPositionalParametersToNamedParameters(
                 '?',
                 "select * "
@@ -1157,22 +1157,10 @@ public void testGoogleStandardSQLDialectConvertPositionalParametersToNamedParame
                     + "and col6 not in (?, ?, ?) "
                     + "and col7 in (?, ?, ?) "
                     + "and col8 between ? and ?")
-            .sqlWithNamedParameters,
-        is(
-            equalTo(
-                "select * "
-                    + "from foo "
-                    + "where col1=@p1 "
-                    + "and col2 like @p2 "
-                    + "and col3 > @p3 "
-                    + "and col4 < @p4 "
-                    + "and col5 != @p5 "
-                    + "and col6 not in (@p6, @p7, @p8) "
-                    + "and col7 in (@p9, @p10, @p11) "
-                    + "and col8 between @p12 and @p13")));
+            .sqlWithNamedParameters);
   }
 
-  private enum CommentInjector {
+  enum CommentInjector {
     NONE {
       @Override
       String inject(String sql, String comment) {
@@ -1213,57 +1201,57 @@ public void testPostgreSQLDialectDialectConvertPositionalParametersToNamedParame
           "/* comment /* with nested */ comment */"
         }) {
       for (CommentInjector injector : CommentInjector.values()) {
-        assertThat(
-                parser.convertPositionalParametersToNamedParameters(
-                        '?', injector.inject("select * %sfrom foo where name=?", comment))
-                    .sqlWithNamedParameters)
-            .isEqualTo(injector.inject("select * %sfrom foo where name=$1", comment));
-        assertThat(
-                parser.convertPositionalParametersToNamedParameters(
-                        '?', injector.inject("?%s'?test?\"?test?\"?'?", comment))
-                    .sqlWithNamedParameters)
-            .isEqualTo(injector.inject("$1%s'?test?\"?test?\"?'$2", comment));
-        assertThat(
-                parser.convertPositionalParametersToNamedParameters(
-                        '?', injector.inject("?'?it\\''?s'%s?", comment))
-                    .sqlWithNamedParameters)
-            .isEqualTo(injector.inject("$1'?it\\''?s'%s$2", comment));
-        assertThat(
-                parser.convertPositionalParametersToNamedParameters(
-                        '?', injector.inject("?'?it\\\"?s'%s?", comment))
-                    .sqlWithNamedParameters)
-            .isEqualTo(injector.inject("$1'?it\\\"?s'%s$2", comment));
-        assertThat(
-                parser.convertPositionalParametersToNamedParameters(
-                        '?', injector.inject("?\"?it\\\"\"?s\"%s?", comment))
-                    .sqlWithNamedParameters)
-            .isEqualTo(injector.inject("$1\"?it\\\"\"?s\"%s$2", comment));
-        assertThat(
-                parser.convertPositionalParametersToNamedParameters(
-                        '?', injector.inject("?%s'''?it\\''?s'''?", comment))
-                    .sqlWithNamedParameters)
-            .isEqualTo(injector.inject("$1%s'''?it\\''?s'''$2", comment));
-        assertThat(
-                parser.convertPositionalParametersToNamedParameters(
-                        '?', injector.inject("?\"\"\"?it\\\"\"?s\"\"\"%s?", comment))
-                    .sqlWithNamedParameters)
-            .isEqualTo(injector.inject("$1\"\"\"?it\\\"\"?s\"\"\"%s$2", comment));
-
-        assertThat(
-                parser.convertPositionalParametersToNamedParameters(
-                        '?', injector.inject("?$$?it$?s$$%s?", comment))
-                    .sqlWithNamedParameters)
-            .isEqualTo(injector.inject("$1$$?it$?s$$%s$2", comment));
-        assertThat(
-                parser.convertPositionalParametersToNamedParameters(
-                        '?', injector.inject("?$tag$?it$$?s$tag$%s?", comment))
-                    .sqlWithNamedParameters)
-            .isEqualTo(injector.inject("$1$tag$?it$$?s$tag$%s$2", comment));
-        assertThat(
-                parser.convertPositionalParametersToNamedParameters(
-                        '?', injector.inject("?%s$$?it\\'?s \n ?it\\'?s$$?", comment))
-                    .sqlWithNamedParameters)
-            .isEqualTo(injector.inject("$1%s$$?it\\'?s \n ?it\\'?s$$$2", comment));
+        assertEquals(
+            injector.inject("select * %sfrom foo where name=$1", comment),
+            parser.convertPositionalParametersToNamedParameters(
+                    '?', injector.inject("select * %sfrom foo where name=?", comment))
+                .sqlWithNamedParameters);
+        assertEquals(
+            injector.inject("$1%s'?test?\"?test?\"?'$2", comment),
+            parser.convertPositionalParametersToNamedParameters(
+                    '?', injector.inject("?%s'?test?\"?test?\"?'?", comment))
+                .sqlWithNamedParameters);
+        assertEquals(
+            injector.inject("$1'?it\\''?s'%s$2", comment),
+            parser.convertPositionalParametersToNamedParameters(
+                    '?', injector.inject("?'?it\\''?s'%s?", comment))
+                .sqlWithNamedParameters);
+        assertEquals(
+            injector.inject("$1'?it\\\"?s'%s$2", comment),
+            parser.convertPositionalParametersToNamedParameters(
+                    '?', injector.inject("?'?it\\\"?s'%s?", comment))
+                .sqlWithNamedParameters);
+        assertEquals(
+            injector.inject("$1\"?it\\\"\"?s\"%s$2", comment),
+            parser.convertPositionalParametersToNamedParameters(
+                    '?', injector.inject("?\"?it\\\"\"?s\"%s?", comment))
+                .sqlWithNamedParameters);
+        assertEquals(
+            injector.inject("$1%s'''?it\\''?s'''$2", comment),
+            parser.convertPositionalParametersToNamedParameters(
+                    '?', injector.inject("?%s'''?it\\''?s'''?", comment))
+                .sqlWithNamedParameters);
+        assertEquals(
+            injector.inject("$1\"\"\"?it\\\"\"?s\"\"\"%s$2", comment),
+            parser.convertPositionalParametersToNamedParameters(
+                    '?', injector.inject("?\"\"\"?it\\\"\"?s\"\"\"%s?", comment))
+                .sqlWithNamedParameters);
+
+        assertEquals(
+            injector.inject("$1$$?it$?s$$%s$2", comment),
+            parser.convertPositionalParametersToNamedParameters(
+                    '?', injector.inject("?$$?it$?s$$%s?", comment))
+                .sqlWithNamedParameters);
+        assertEquals(
+            injector.inject("$1$tag$?it$$?s$tag$%s$2", comment),
+            parser.convertPositionalParametersToNamedParameters(
+                    '?', injector.inject("?$tag$?it$$?s$tag$%s?", comment))
+                .sqlWithNamedParameters);
+        assertEquals(
+            injector.inject("$1%s$$?it\\'?s \n ?it\\'?s$$$2", comment),
+            parser.convertPositionalParametersToNamedParameters(
+                    '?', injector.inject("?%s$$?it\\'?s \n ?it\\'?s$$?", comment))
+                .sqlWithNamedParameters);
 
         // Note: PostgreSQL allows a single-quoted string literal to contain line feeds.
         assertEquals(
@@ -1271,27 +1259,32 @@ public void testPostgreSQLDialectDialectConvertPositionalParametersToNamedParame
             parser.convertPositionalParametersToNamedParameters(
                     '?', injector.inject("?'?it\\''?s \n ?it\\''?s'%s?", comment))
                 .sqlWithNamedParameters);
-        assertUnclosedLiteral("?'?it\\''?s \n ?it\\''?s?");
+        assertUnclosedLiteral(parser, "?'?it\\''?s \n ?it\\''?s?");
         assertEquals(
             injector.inject("$1%s'''?it\\''?s \n ?it\\''?s'$2", comment),
             parser.convertPositionalParametersToNamedParameters(
                     '?', injector.inject("?%s'''?it\\''?s \n ?it\\''?s'?", comment))
                 .sqlWithNamedParameters);
 
-        assertThat(
+        assertEquals(
+            injector.inject(
+                "select 1, $1, 'test?test', \"test?test\", %sfoo.* from `foo` where col1=$2 and col2='test' and col3=$3 and col4='?' and col5=\"?\" and col6='?''?''?'",
+                comment),
             parser.convertPositionalParametersToNamedParameters(
                     '?',
                     injector.inject(
                         "select 1, ?, 'test?test', \"test?test\", %sfoo.* from `foo` where col1=? and col2='test' and col3=? and col4='?' and col5=\"?\" and col6='?''?''?'",
                         comment))
-                .sqlWithNamedParameters,
-            is(
-                equalTo(
-                    injector.inject(
-                        "select 1, $1, 'test?test', \"test?test\", %sfoo.* from `foo` where col1=$2 and col2='test' and col3=$3 and col4='?' and col5=\"?\" and col6='?''?''?'",
-                        comment))));
+                .sqlWithNamedParameters);
 
-        assertThat(
+        assertEquals(
+            injector.inject(
+                "select * "
+                    + "%sfrom foo "
+                    + "where name=$1 "
+                    + "and col2 like $2 "
+                    + "and col3 > $3",
+                comment),
             parser.convertPositionalParametersToNamedParameters(
                     '?',
                     injector.inject(
@@ -1301,36 +1294,34 @@ public void testPostgreSQLDialectDialectConvertPositionalParametersToNamedParame
                             + "and col2 like ? "
                             + "and col3 > ?",
                         comment))
-                .sqlWithNamedParameters,
-            is(
-                equalTo(
-                    injector.inject(
-                        "select * "
-                            + "%sfrom foo "
-                            + "where name=$1 "
-                            + "and col2 like $2 "
-                            + "and col3 > $3",
-                        comment))));
-        assertThat(
+                .sqlWithNamedParameters);
+        assertEquals(
+            injector.inject("select * " + "from foo " + "where id between $1%s and $2", comment),
             parser.convertPositionalParametersToNamedParameters(
                     '?',
                     injector.inject(
                         "select * " + "from foo " + "where id between ?%s and ?", comment))
-                .sqlWithNamedParameters,
-            is(
-                equalTo(
-                    injector.inject(
-                        "select * " + "from foo " + "where id between $1%s and $2", comment))));
-        assertThat(
+                .sqlWithNamedParameters);
+        assertEquals(
+            injector.inject("select * " + "from foo " + "limit $1 %s offset $2", comment),
             parser.convertPositionalParametersToNamedParameters(
                     '?',
                     injector.inject("select * " + "from foo " + "limit ? %s offset ?", comment))
-                .sqlWithNamedParameters,
-            is(
-                equalTo(
-                    injector.inject(
-                        "select * " + "from foo " + "limit $1 %s offset $2", comment))));
-        assertThat(
+                .sqlWithNamedParameters);
+        assertEquals(
+            injector.inject(
+                "select * "
+                    + "from foo "
+                    + "where col1=$1 "
+                    + "and col2 like $2 "
+                    + " %s "
+                    + "and col3 > $3 "
+                    + "and col4 < $4 "
+                    + "and col5 != $5 "
+                    + "and col6 not in ($6, $7, $8) "
+                    + "and col7 in ($9, $10, $11) "
+                    + "and col8 between $12 and $13",
+                comment),
             parser.convertPositionalParametersToNamedParameters(
                     '?',
                     injector.inject(
@@ -1346,22 +1337,7 @@ public void testPostgreSQLDialectDialectConvertPositionalParametersToNamedParame
                             + "and col7 in (?, ?, ?) "
                             + "and col8 between ? and ?",
                         comment))
-                .sqlWithNamedParameters,
-            is(
-                equalTo(
-                    injector.inject(
-                        "select * "
-                            + "from foo "
-                            + "where col1=$1 "
-                            + "and col2 like $2 "
-                            + " %s "
-                            + "and col3 > $3 "
-                            + "and col4 < $4 "
-                            + "and col5 != $5 "
-                            + "and col6 not in ($6, $7, $8) "
-                            + "and col7 in ($9, $10, $11) "
-                            + "and col8 between $12 and $13",
-                        comment))));
+                .sqlWithNamedParameters);
       }
     }
   }
@@ -1714,18 +1690,20 @@ public void testStatementCache_ParameterizedStatement() {
     assertEquals(1, stats.hitCount());
   }
 
-  private void assertUnclosedLiteral(String sql) {
-    try {
-      parser.convertPositionalParametersToNamedParameters('?', sql);
-      fail("missing expected exception");
-    } catch (SpannerException e) {
-      assertThat(e.getErrorCode()).isSameInstanceAs(ErrorCode.INVALID_ARGUMENT);
-      assertThat(e.getMessage())
-          .startsWith(
-              ErrorCode.INVALID_ARGUMENT.name()
-                  + ": SQL statement contains an unclosed literal: "
-                  + sql);
-    }
+  static void assertUnclosedLiteral(AbstractStatementParser parser, String sql) {
+    SpannerException exception =
+        assertThrows(
+            SpannerException.class,
+            () -> parser.convertPositionalParametersToNamedParameters('?', sql));
+    assertEquals(ErrorCode.INVALID_ARGUMENT, exception.getErrorCode());
+    assertTrue(
+        exception.getMessage(),
+        exception
+            .getMessage()
+            .startsWith(
+                ErrorCode.INVALID_ARGUMENT.name()
+                    + ": SQL statement contains an unclosed literal: "
+                    + sql));
   }
 
   @SuppressWarnings("unchecked")