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.

Sign up for GitHub

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

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

Merged
merged 2 commits into from
Mar 8, 2022
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
74adda4
fix: PostgreSQL supports newline in quoted literals and identifiers
olavloite Mar 7, 2022
c5690a4
fix: typo
olavloite Mar 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...ud-spanner/src/main/java/com/google/cloud/spanner/connection/AbstractStatementParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = '$';

/**
Expand Down
220 changes: 87 additions & 133 deletions ...-spanner/src/main/java/com/google/cloud/spanner/connection/PostgreSQLStatementParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,95 +56,44 @@ protected boolean supportsExplain() {
@Override
String removeCommentsAndTrimInternal(String sql) {
Preconditions.checkNotNull(sql);
String currentTag = null;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be easier to open the entire file (online or in an IDE) to review it, than to try to do that based on this diff render from GitHub. The parser code has also been compacted a lot, as there was quite a lot of code duplication, and that makes this diff render a lot harder to read.

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,
Expand Down Expand Up @@ -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);
}
}
4 changes: 2 additions & 2 deletions ...oud-spanner/src/main/java/com/google/cloud/spanner/connection/SpannerStatementParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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++;
}
Expand All @@ -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 {
Expand Down
38 changes: 36 additions & 2 deletions ...-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/StatementParserTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down