Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: keep comments when searching for params #2951

Merged
merged 7 commits into from
Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -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. */
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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.
*/
Expand Down
Expand Up @@ -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:
Expand Down Expand Up @@ -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.
*
Expand Down
Expand Up @@ -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:
Expand Down Expand Up @@ -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())
Expand Down
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
}
}