Skip to content

Commit

Permalink
fix: ( and ) are allowed at the end of keywords (#312)
Browse files Browse the repository at this point in the history 
* fix: ( and ) are allowed at the end of keywords

Fixes multiple minor issues with the simple parser:
1. ( and ) are allowed at the end of a keyword (e.g. `insert into foo values(1)`
2. There must be a whitespace or a parentheses at the end of a keyword

* fix: detect end of keyword on non-valid identifier char

* test: add more tests + remove unused method
  • Loading branch information
@olavloite
olavloite committed Aug 12, 2022
1 parent b182c7b commit f3ebfb5
Show file tree
Hide file tree
Showing 7 changed files with 214 additions and 76 deletions.
32 changes: 16 additions & 16 deletions src/main/java/com/google/cloud/spanner/pgadapter/statements/DdlExecutor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,35 +188,35 @@ StatementResult execute(ParsedStatement parsedStatement, Statement statement) {
*/
Statement translate(ParsedStatement parsedStatement, Statement statement) {
SimpleParser parser = new SimpleParser(parsedStatement.getSqlWithoutComments());
if (parser.eat("create")) {
if (parser.eatKeyword("create")) {
statement = translateCreate(parser, statement);
} else if (parser.eat("drop")) {
} else if (parser.eatKeyword("drop")) {
statement = translateDrop(parser, statement);
}

return statement;
}

Statement translateCreate(SimpleParser parser, Statement statement) {
if (parser.eat("table")) {
if (parser.eatKeyword("table")) {
Statement createTableStatement = translateCreateTable(parser, statement);
if (createTableStatement == null) {
return null;
}
return maybeRemovePrimaryKeyConstraintName(createTableStatement);
}
boolean unique = parser.eat("unique");
if (parser.eat("index")) {
boolean unique = parser.eatKeyword("unique");
if (parser.eatKeyword("index")) {
return translateCreateIndex(parser, statement, unique);
}
return statement;
}

Statement translateDrop(SimpleParser parser, Statement statement) {
if (parser.eat("table")) {
if (parser.eatKeyword("table")) {
return translateDropTable(parser, statement);
}
if (parser.eat("index")) {
if (parser.eatKeyword("index")) {
return translateDropIndex(parser, statement);
}
return statement;
Expand Down Expand Up @@ -244,7 +244,7 @@ private Statement translateCreateTableOrIndex(
Statement statement,
String type,
Function<TableOrIndexName, Boolean> existsFunction) {
if (!parser.eat("if", "not", "exists")) {
if (!parser.eatKeyword("if", "not", "exists")) {
return statement;
}
return maybeExecuteStatement(
Expand All @@ -256,7 +256,7 @@ private Statement translateDropTableOrIndex(
Statement statement,
String type,
Function<TableOrIndexName, Boolean> existsFunction) {
if (!parser.eat("if", "exists")) {
if (!parser.eatKeyword("if", "exists")) {
return statement;
}
return maybeExecuteStatement(parser, statement, "drop", type, existsFunction);
Expand Down Expand Up @@ -348,25 +348,25 @@ Statement maybeRemovePrimaryKeyConstraintName(Statement createTableStatement) {
ParsedStatement parsedStatement =
AbstractStatementParser.getInstance(Dialect.POSTGRESQL).parse(createTableStatement);
SimpleParser parser = new SimpleParser(parsedStatement.getSqlWithoutComments());
parser.eat("create", "table", "if", "not", "exists");
parser.eatKeyword("create", "table", "if", "not", "exists");
TableOrIndexName tableName = parser.readTableOrIndexName();
if (tableName == null) {
return createTableStatement;
}
if (!parser.eat("(")) {
if (!parser.eatToken("(")) {
return createTableStatement;
}
while (true) {
if (parser.eat(")")) {
if (parser.eatToken(")")) {
break;
} else if (parser.eat(",")) {
} else if (parser.eatToken(",")) {
continue;
} else if (parser.peek("constraint")) {
} else if (parser.peekKeyword("constraint")) {
int positionBeforeConstraintDefinition = parser.getPos();
parser.eat("constraint");
parser.eatKeyword("constraint");
String constraintName = unquoteIdentifier(parser.readIdentifierPart());
int positionAfterConstraintName = parser.getPos();
if (parser.eat("primary", "key")) {
if (parser.eatKeyword("primary", "key")) {
if (!constraintName.equalsIgnoreCase("pk_" + unquoteIdentifier(tableName.name))) {
return createTableStatement;
}
Expand Down
59 changes: 36 additions & 23 deletions ...ain/java/com/google/cloud/spanner/pgadapter/statements/IntermediatePreparedStatement.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -280,46 +280,59 @@ private Statement transformDmlToSelectParams(Set<String> parameters) {
static @Nullable Statement transformInsertToSelectParams(
Connection connection, String sql, Set<String> parameters) {
SimpleParser parser = new SimpleParser(sql);
if (!parser.eat("insert")) {
if (!parser.eatKeyword("insert")) {
return null;
}
parser.eat("into");
parser.eatKeyword("into");
TableOrIndexName table = parser.readTableOrIndexName();
if (table == null) {
return null;
}
parser.skipWhitespaces();

List<String> columnsList = null;
if (parser.eat("(")) {
columnsList = parser.parseExpressionList();
if (!parser.eat(")")) {
return null;
int posBeforeToken = parser.getPos();
if (parser.eatToken("(")) {
if (parser.peekKeyword("select") || parser.peekToken("(")) {
// Revert and assume that the insert uses a select statement.
parser.setPos(posBeforeToken);
} else {
columnsList = parser.parseExpressionList();
if (!parser.eatToken(")")) {
return null;
}
}
}

parser.skipWhitespaces();
int potentialSelectStart = parser.getPos();
if (parser.eat("select")) {
// This is an `insert into <table> [(...)] select ...` statement. Then we can just use the
// select statement as the result.
return transformSelectToSelectParams(
parser.getSql().substring(potentialSelectStart), parameters);
} else if (!parser.eat("values")) {
if (!parser.eatKeyword("values")) {
while (parser.eatToken("(")) {
// ignore
}
if (parser.eatKeyword("select")) {
// This is an `insert into <table> [(...)] select ...` statement. Then we can just use the
// select statement as the result.
return transformSelectToSelectParams(
parser.getSql().substring(potentialSelectStart), parameters);
}
return null;
}

if (columnsList == null || columnsList.isEmpty()) {
columnsList = getAllColumns(connection, table);
}
List<List<String>> rows = new ArrayList<>();
while (parser.eat("(")) {
while (parser.eatToken("(")) {
List<String> row = parser.parseExpressionList();
if (row == null || row.isEmpty() || !parser.eat(")") || row.size() != columnsList.size()) {
if (row == null
|| row.isEmpty()
|| !parser.eatToken(")")
|| row.size() != columnsList.size()) {
return null;
}
rows.add(row);
if (!parser.eat(",")) {
if (!parser.eatToken(",")) {
break;
}
}
Expand Down Expand Up @@ -377,23 +390,23 @@ static List<String> getAllColumns(Connection connection, TableOrIndexName table)
@VisibleForTesting
static Statement transformUpdateToSelectParams(String sql, Set<String> parameters) {
SimpleParser parser = new SimpleParser(sql);
if (!parser.eat("update")) {
if (!parser.eatKeyword("update")) {
return null;
}
parser.eat("only");
parser.eatKeyword("only");
TableOrIndexName table = parser.readTableOrIndexName();
if (table == null) {
return null;
}
if (!parser.eat("set")) {
if (!parser.eatKeyword("set")) {
return null;
}
List<String> assignmentsList = parser.parseExpressionListUntil("where");
List<String> assignmentsList = parser.parseExpressionListUntilKeyword("where");
if (assignmentsList == null || assignmentsList.isEmpty()) {
return null;
}
int whereStart = parser.getPos();
if (!parser.eat("where")) {
if (!parser.eatKeyword("where")) {
whereStart = -1;
}

Expand Down Expand Up @@ -427,17 +440,17 @@ static Statement transformUpdateToSelectParams(String sql, Set<String> parameter
@VisibleForTesting
static Statement transformDeleteToSelectParams(String sql, Set<String> parameters) {
SimpleParser parser = new SimpleParser(sql);
if (!parser.eat("delete")) {
if (!parser.eatKeyword("delete")) {
return null;
}
parser.eat("from");
parser.eatKeyword("from");
TableOrIndexName table = parser.readTableOrIndexName();
if (table == null) {
return null;
}
parser.skipWhitespaces();
int whereStart = parser.getPos();
if (!parser.eat("where")) {
if (!parser.eatKeyword("where")) {
// Deletes must have a where clause, otherwise there cannot be any parameters.
return null;
}
Expand Down
16 changes: 8 additions & 8 deletions src/main/java/com/google/cloud/spanner/pgadapter/statements/SessionStatementParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -258,13 +258,13 @@ private static String unquote(String value) {
return null;
}
SimpleParser parser = new SimpleParser(parsedStatement.getSqlWithoutComments());
if (parser.eat("set")) {
if (parser.eatKeyword("set")) {
return parseSetStatement(parser);
}
if (parser.eat("reset")) {
if (parser.eatKeyword("reset")) {
return parseResetStatement(parser);
}
if (parser.eat("show")) {
if (parser.eatKeyword("show")) {
return parseShowStatement(parser);
}

Expand All @@ -273,11 +273,11 @@ private static String unquote(String value) {

static SetStatement parseSetStatement(SimpleParser parser) {
SetStatement.Builder builder = new Builder();
if (parser.eat("local")) {
if (parser.eatKeyword("local")) {
builder.setLocal();
} else {
// Ignore, this is the default.
parser.eat("session");
parser.eatKeyword("session");
}
TableOrIndexName name = parser.readTableOrIndexName();
if (name == null) {
Expand All @@ -287,7 +287,7 @@ static SetStatement parseSetStatement(SimpleParser parser) {
}
builder.setName(name);

if (!(parser.eat("to") || parser.eat("="))) {
if (!(parser.eatKeyword("to") || parser.eatToken("="))) {
throw SpannerExceptionFactory.newSpannerException(
ErrorCode.INVALID_ARGUMENT,
"Invalid SET statement: " + parser.getSql() + ". Expected TO or =.");
Expand Down Expand Up @@ -318,7 +318,7 @@ static SetStatement parseSetStatement(SimpleParser parser) {
}

static ResetStatement parseResetStatement(SimpleParser parser) {
if (parser.eat("all")) {
if (parser.eatKeyword("all")) {
String remaining = parser.parseExpression();
if (!Strings.isNullOrEmpty(remaining)) {
throw SpannerExceptionFactory.newSpannerException(
Expand Down Expand Up @@ -350,7 +350,7 @@ static ResetStatement parseResetStatement(SimpleParser parser) {
}

static ShowStatement parseShowStatement(SimpleParser parser) {
if (parser.eat("all")) {
if (parser.eatKeyword("all")) {
String remaining = parser.parseExpression();
if (!Strings.isNullOrEmpty(remaining)) {
throw SpannerExceptionFactory.newSpannerException(
Expand Down
Loading

0 comments on commit f3ebfb5

Please sign in to comment.