perf: keep comments when searching for params

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.
  */

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.
  *

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())

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);
+ }
+ }
+ }
 }