chore: remove gsql parsing escape (#1850)

olavloite · web-flow · commit f865e2549c40 · 2022-04-27T11:02:01.000+02:00
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
@@ -54,12 +54,6 @@ public abstract class AbstractStatementParser {
               SpannerStatementParser.class,
               Dialect.POSTGRESQL,
               PostgreSQLStatementParser.class);
-  private static final String GSQL_STATEMENT = "/*GSQL*/";
-
-  /* Checks if the SQL statement starts with /*GSQL*/
-  private boolean isGoogleSql(String sql) {
-    return sql.startsWith(GSQL_STATEMENT);
-  }
 
   /** Get an instance of {@link AbstractStatementParser} for the specified dialect. */
   public static AbstractStatementParser getInstance(Dialect dialect) {
@@ -317,11 +311,9 @@ ClientSideStatement getClientSideStatement() {
   static final Set<String> ddlStatements = ImmutableSet.of("CREATE", "DROP", "ALTER");
   static final Set<String> selectStatements = ImmutableSet.of("SELECT", "WITH", "SHOW");
   static final Set<String> dmlStatements = ImmutableSet.of("INSERT", "UPDATE", "DELETE");
-  private final Dialect dialect;
   private final Set<ClientSideStatementImpl> statements;
 
-  AbstractStatementParser(Dialect dialect, Set<ClientSideStatementImpl> statements) {
-    this.dialect = dialect;
+  AbstractStatementParser(Set<ClientSideStatementImpl> statements) {
     this.statements = statements;
   }
 
@@ -427,12 +419,7 @@ public boolean isUpdateStatement(String sql) {
 
   private boolean statementStartsWith(String sql, Iterable<String> checkStatements) {
     Preconditions.checkNotNull(sql);
-    String[] tokens;
-    if (isGoogleSql(sql)) {
-      tokens = sql.substring(GSQL_STATEMENT.length()).split("\\s+", 2);
-    } else {
-      tokens = sql.split("\\s+", 2);
-    }
+    String[] tokens = sql.split("\\s+", 2);
     int checkIndex = 0;
     if (supportsExplain() && tokens[0].equalsIgnoreCase("EXPLAIN")) {
       checkIndex = 1;
@@ -467,20 +454,7 @@ private boolean statementStartsWith(String sql, Iterable<String> checkStatements
 
   @InternalApi
   public String removeCommentsAndTrim(String sql) {
-    switch (dialect) {
-      case POSTGRESQL:
-        if (isGoogleSql(sql.trim())) {
-          return GSQL_STATEMENT
-              + INSTANCES.get(Dialect.GOOGLE_STANDARD_SQL).removeCommentsAndTrimInternal(sql);
-        } else {
-          return removeCommentsAndTrimInternal(sql);
-        }
-      case GOOGLE_STANDARD_SQL:
-        return removeCommentsAndTrimInternal(sql);
-      default:
-        throw SpannerExceptionFactory.newSpannerException(
-            ErrorCode.INTERNAL, "Unknown dialect: " + dialect);
-    }
+    return removeCommentsAndTrimInternal(sql);
   }
 
   /** Removes any statement hints at the beginning of the statement. */
@@ -517,15 +491,7 @@ abstract ParametersInfo convertPositionalParametersToNamedParametersInternal(
 
   @InternalApi
   public ParametersInfo convertPositionalParametersToNamedParameters(char paramChar, String sql) {
-    if (dialect == Dialect.POSTGRESQL && isGoogleSql(sql.trim())) {
-      return INSTANCES
-          .get(Dialect.GOOGLE_STANDARD_SQL)
-          .convertPositionalParametersToNamedParametersInternal(paramChar, sql);
-    } else {
-      return INSTANCES
-          .get(dialect)
-          .convertPositionalParametersToNamedParametersInternal(paramChar, sql);
-    }
+    return convertPositionalParametersToNamedParametersInternal(paramChar, sql);
   }
 
   /** Convenience method that is used to estimate the number of parameters in a SQL statement. */
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
@@ -31,7 +31,6 @@
 public class PostgreSQLStatementParser extends AbstractStatementParser {
   PostgreSQLStatementParser() throws CompileException {
     super(
-        Dialect.POSTGRESQL,
         Collections.unmodifiableSet(
             ClientSideStatements.getInstance(Dialect.POSTGRESQL).getCompiledStatements()));
   }
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
@@ -31,7 +31,6 @@ public class SpannerStatementParser extends AbstractStatementParser {
 
   public SpannerStatementParser() throws CompileException {
     super(
-        Dialect.GOOGLE_STANDARD_SQL,
         Collections.unmodifiableSet(
             ClientSideStatements.getInstance(Dialect.GOOGLE_STANDARD_SQL).getCompiledStatements()));
   }
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
@@ -182,31 +182,6 @@ public void testGoogleStandardSQLRemoveCommentsGsql() {
         .isEqualTo("SELECT * FROM FOO");
   }
 
-  @Test
-  public void testPostgreSQLDialectRemoveCommentsGsql() {
-    assumeTrue(dialect == Dialect.POSTGRESQL);
-
-    assertThat(parser.removeCommentsAndTrim("/*GSQL*/")).isEqualTo("/*GSQL*/");
-    assertThat(parser.removeCommentsAndTrim("/*GSQL*/SELECT * FROM FOO"))
-        .isEqualTo("/*GSQL*/SELECT * FROM FOO");
-    assertThat(
-            parser.removeCommentsAndTrim(
-                "/*GSQL*/-- This is a one line comment\nSELECT * FROM FOO"))
-        .isEqualTo("/*GSQL*/SELECT * FROM FOO");
-    assertThat(
-            parser.removeCommentsAndTrim(
-                "/*GSQL*//* This is a simple multi line comment */\nSELECT * FROM FOO"))
-        .isEqualTo("/*GSQL*/SELECT * FROM FOO");
-    assertThat(
-            parser.removeCommentsAndTrim(
-                "/*GSQL*//* This is a \nmulti line comment */\nSELECT * FROM FOO"))
-        .isEqualTo("/*GSQL*/SELECT * FROM FOO");
-    assertThat(
-            parser.removeCommentsAndTrim(
-                "/*GSQL*//* This\nis\na\nmulti\nline\ncomment */\nSELECT * FROM FOO"))
-        .isEqualTo("/*GSQL*/SELECT * FROM FOO");
-  }
-
   @Test
   public void testStatementWithCommentContainingSlash() {
     String sql =
@@ -1003,103 +978,6 @@ private <T extends ClientSideStatementImpl> void testParseStatementWithOneParame
     assertParsing(withSuffix(")", statement), statementClass);
   }
 
-  @Test
-  public void testConvertPositionalParametersToNamedParametersWithGsqlException() {
-    assertThat(
-            parser.convertPositionalParametersToNamedParameters(
-                    '?', "/*GSQL*/select * from foo where name=?")
-                .sqlWithNamedParameters)
-        .isEqualTo("/*GSQL*/select * from foo where name=@p1");
-    assertThat(
-            parser.convertPositionalParametersToNamedParameters(
-                    '?', "/*GSQL*/?'?test?\"?test?\"?'?")
-                .sqlWithNamedParameters)
-        .isEqualTo("/*GSQL*/@p1'?test?\"?test?\"?'@p2");
-    assertThat(
-            parser.convertPositionalParametersToNamedParameters('?', "/*GSQL*/?'?it\\'?s'?")
-                .sqlWithNamedParameters)
-        .isEqualTo("/*GSQL*/@p1'?it\\'?s'@p2");
-    assertThat(
-            parser.convertPositionalParametersToNamedParameters('?', "/*GSQL*/?'?it\\\"?s'?")
-                .sqlWithNamedParameters)
-        .isEqualTo("/*GSQL*/@p1'?it\\\"?s'@p2");
-    assertThat(
-            parser.convertPositionalParametersToNamedParameters('?', "/*GSQL*/?\"?it\\\"?s\"?")
-                .sqlWithNamedParameters)
-        .isEqualTo("/*GSQL*/@p1\"?it\\\"?s\"@p2");
-    assertThat(
-            parser.convertPositionalParametersToNamedParameters('?', "/*GSQL*/?'''?it\\'?s'''?")
-                .sqlWithNamedParameters)
-        .isEqualTo("/*GSQL*/@p1'''?it\\'?s'''@p2");
-    assertThat(
-            parser.convertPositionalParametersToNamedParameters(
-                    '?', "/*GSQL*/?\"\"\"?it\\\"?s\"\"\"?")
-                .sqlWithNamedParameters)
-        .isEqualTo("/*GSQL*/@p1\"\"\"?it\\\"?s\"\"\"@p2");
-
-    assertThat(
-        parser.convertPositionalParametersToNamedParameters(
-                '?',
-                "/*GSQL*/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(
-                "/*GSQL*/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='?''?''?'")));
-
-    assertThat(
-        parser.convertPositionalParametersToNamedParameters(
-                '?',
-                "/*GSQL*/select * "
-                    + "from foo "
-                    + "where name=? "
-                    + "and col2 like ? "
-                    + "and col3 > ?")
-            .sqlWithNamedParameters,
-        is(
-            equalTo(
-                "/*GSQL*/select * "
-                    + "from foo "
-                    + "where name=@p1 "
-                    + "and col2 like @p2 "
-                    + "and col3 > @p3")));
-    assertThat(
-        parser.convertPositionalParametersToNamedParameters(
-                '?', "/*GSQL*/select * " + "from foo " + "where id between ? and ?")
-            .sqlWithNamedParameters,
-        is(equalTo("/*GSQL*/select * " + "from foo " + "where id between @p1 and @p2")));
-    assertThat(
-        parser.convertPositionalParametersToNamedParameters(
-                '?', "/*GSQL*/select * " + "from foo " + "limit ? offset ?")
-            .sqlWithNamedParameters,
-        is(equalTo("/*GSQL*/select * " + "from foo " + "limit @p1 offset @p2")));
-    assertThat(
-        parser.convertPositionalParametersToNamedParameters(
-                '?',
-                "/*GSQL*/select * "
-                    + "from foo "
-                    + "where col1=? "
-                    + "and col2 like ? "
-                    + "and col3 > ? "
-                    + "and col4 < ? "
-                    + "and col5 != ? "
-                    + "and col6 not in (?, ?, ?) "
-                    + "and col7 in (?, ?, ?) "
-                    + "and col8 between ? and ?")
-            .sqlWithNamedParameters,
-        is(
-            equalTo(
-                "/*GSQL*/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")));
-  }
-
   @Test
   public void testGoogleStandardSQLDialectConvertPositionalParametersToNamedParameters() {
     assumeTrue(dialect == Dialect.GOOGLE_STANDARD_SQL);

Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,6 @@`
`31`	`31`	`public class PostgreSQLStatementParser extends AbstractStatementParser {`
`32`	`32`	`PostgreSQLStatementParser() throws CompileException {`
`33`	`33`	`super(`
`34`		`- Dialect.POSTGRESQL,`
`35`	`34`	`Collections.unmodifiableSet(`
`36`	`35`	`ClientSideStatements.getInstance(Dialect.POSTGRESQL).getCompiledStatements()));`
`37`	`36`	`}`
Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,6 @@ public class SpannerStatementParser extends AbstractStatementParser {`
`31`	`31`
`32`	`32`	`public SpannerStatementParser() throws CompileException {`
`33`	`33`	`super(`
`34`		`- Dialect.GOOGLE_STANDARD_SQL,`
`35`	`34`	`Collections.unmodifiableSet(`
`36`	`35`	`ClientSideStatements.getInstance(Dialect.GOOGLE_STANDARD_SQL).getCompiledStatements()));`
`37`	`36`	`}`