fix: skipHint in the internal parser skipped too much

google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/SimpleParser.java

@@ -138,7 +138,7 @@ void skipHint() {
     // comments and comments are automatically skipped by all methods.
     if (getDialect() == Dialect.GOOGLE_STANDARD_SQL && eatTokens('@', '{')) {
       while (pos < length && !eatToken('}')) {
-        pos += statementParser.skip(sql, pos, /*result=*/ null);
+        pos = statementParser.skip(sql, pos, /*result=*/ null);
       }
     }
   }

google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/SimpleParserTest.java

@@ -23,6 +23,7 @@
 import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assume.assumeTrue;
 
 import com.google.cloud.spanner.Dialect;
 import org.junit.Test;
@@ -221,4 +222,29 @@ public void testEatSingleQuotedStringAdvancesPosition() {
     assertEquals(NOT_FOUND, parser.eatSingleQuotedString());
     assertEquals(parser.getSql().length(), parser.getPos());
   }
+
+  @Test
+  public void testSkipHint() {
+    assumeTrue("Hints in PostgreSQL are comments", dialect == Dialect.GOOGLE_STANDARD_SQL);
+
+    assertEquals("SELECT 1", skipHint("SELECT 1"));
+    assertEquals("SELECT 1", skipHint("@{rpc_priority=HIGH}SELECT 1"));
+    assertEquals("SELECT 1", skipHint("@{statement_tag='test'}SELECT 1"));
+    assertEquals("  \nSELECT 1", skipHint("  @{statement_tag =  'test'}  \nSELECT 1"));
+    assertEquals(
+        " /* comment after */ SELECT 1",
+        skipHint("/* comment before */ @{statement_tag='test'} /* comment after */ SELECT 1"));
+    assertEquals(
+        " -- comment after\nSELECT 1",
+        skipHint("-- comment before\n @{statement_tag='test'} -- comment after\nSELECT 1"));
+    assertEquals(
+        "-- comment @{statement_tag='test'}\n -- also a comment\nSELECT 1",
+        skipHint("-- comment @{statement_tag='test'}\n -- also a comment\nSELECT 1"));
+  }
+
+  static String skipHint(String sql) {
+    SimpleParser parser = new SimpleParser(Dialect.GOOGLE_STANDARD_SQL, sql);
+    parser.skipHint();
+    return parser.getSql().substring(parser.getPos());
+  }
 }

google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/StatementParserTest.java

@@ -696,46 +696,89 @@ public void testGoogleStandardSQLDialectIsQuery_QueryHints() {
 
     // Supports query hints, PostgreSQL dialect does NOT
     // Valid query hints.
-    assertTrue(parser.isQuery("@{JOIN_METHOD=HASH_JOIN} SELECT * FROM PersonsTable"));
-    assertTrue(parser.isQuery("@ {JOIN_METHOD=HASH_JOIN} SELECT * FROM PersonsTable"));
-    assertTrue(parser.isQuery("@{ JOIN_METHOD=HASH_JOIN} SELECT * FROM PersonsTable"));
-    assertTrue(parser.isQuery("@{JOIN_METHOD=HASH_JOIN } SELECT * FROM PersonsTable"));
-    assertTrue(parser.isQuery("@{JOIN_METHOD=HASH_JOIN}\nSELECT * FROM PersonsTable"));
-    assertTrue(parser.isQuery("@{\nJOIN_METHOD =  HASH_JOIN   \t}\n\t SELECT * FROM PersonsTable"));
     assertTrue(
-        parser.isQuery(
-            "@{JOIN_METHOD=HASH_JOIN}\n -- Single line comment\nSELECT * FROM PersonsTable"));
+        parser
+            .parse(Statement.of("@{JOIN_METHOD=HASH_JOIN} SELECT * FROM PersonsTable"))
+            .isQuery());
     assertTrue(
-        parser.isQuery(
-            "@{JOIN_METHOD=HASH_JOIN}\n /* Multi line comment\n with more comments\n */SELECT * FROM PersonsTable"));
+        parser
+            .parse(Statement.of("@ {JOIN_METHOD=HASH_JOIN} SELECT * FROM PersonsTable"))
+            .isQuery());
     assertTrue(
-        parser.isQuery(
-            "@{JOIN_METHOD=HASH_JOIN} WITH subQ1 AS (SELECT SchoolID FROM Roster),\n"
-                + "     subQ2 AS (SELECT OpponentID FROM PlayerStats)\n"
-                + "SELECT * FROM subQ1\n"
-                + "UNION ALL\n"
-                + "SELECT * FROM subQ2"));
+        parser
+            .parse(Statement.of("@{ JOIN_METHOD=HASH_JOIN} SELECT * FROM PersonsTable"))
+            .isQuery());
+    assertTrue(
+        parser
+            .parse(Statement.of("@{JOIN_METHOD=HASH_JOIN } SELECT * FROM PersonsTable"))
+            .isQuery());
+    assertTrue(
+        parser
+            .parse(Statement.of("@{JOIN_METHOD=HASH_JOIN}\nSELECT * FROM PersonsTable"))
+            .isQuery());
+    assertTrue(
+        parser
+            .parse(
+                Statement.of("@{\nJOIN_METHOD =  HASH_JOIN   \t}\n\t SELECT * FROM PersonsTable"))
+            .isQuery());
+    assertTrue(
+        parser
+            .parse(
+                Statement.of(
+                    "@{JOIN_METHOD=HASH_JOIN}\n -- Single line comment\nSELECT * FROM PersonsTable"))
+            .isQuery());
+    assertTrue(
+        parser
+            .parse(
+                Statement.of(
+                    "@{JOIN_METHOD=HASH_JOIN}\n /* Multi line comment\n with more comments\n */SELECT * FROM PersonsTable"))
+            .isQuery());
+    assertTrue(
+        parser
+            .parse(
+                Statement.of(
+                    "@{JOIN_METHOD=HASH_JOIN} WITH subQ1 AS (SELECT SchoolID FROM Roster),\n"
+                        + "     subQ2 AS (SELECT OpponentID FROM PlayerStats)\n"
+                        + "SELECT * FROM subQ1\n"
+                        + "UNION ALL\n"
+                        + "SELECT * FROM subQ2"))
+            .isQuery());
 
     // Multiple query hints.
     assertTrue(
-        parser.isQuery("@{FORCE_INDEX=index_name} @{JOIN_METHOD=HASH_JOIN} SELECT * FROM tbl"));
+        parser
+            .parse(
+                Statement.of("@{FORCE_INDEX=index_name, JOIN_METHOD=HASH_JOIN} SELECT * FROM tbl"))
+            .isQuery());
     assertTrue(
-        parser.isQuery("@{FORCE_INDEX=index_name} @{JOIN_METHOD=HASH_JOIN} Select * FROM tbl"));
+        parser
+            .parse(
+                Statement.of("@{FORCE_INDEX=index_name, JOIN_METHOD=HASH_JOIN} Select * FROM tbl"))
+            .isQuery());
     assertTrue(
-        parser.isQuery(
-            "@{FORCE_INDEX=index_name}\n@{JOIN_METHOD=HASH_JOIN}\nWITH subQ1 AS (SELECT SchoolID FROM Roster),\n"
-                + "     subQ2 AS (SELECT OpponentID FROM PlayerStats)\n"
-                + "SELECT * FROM subQ1\n"
-                + "UNION ALL\n"
-                + "SELECT * FROM subQ2"));
+        parser
+            .parse(
+                Statement.of(
+                    "@{FORCE_INDEX=index_name,\nJOIN_METHOD=HASH_JOIN}\nWITH subQ1 AS (SELECT SchoolID FROM Roster),\n"
+                        + "     subQ2 AS (SELECT OpponentID FROM PlayerStats)\n"
+                        + "SELECT * FROM subQ1\n"
+                        + "UNION ALL\n"
+                        + "SELECT * FROM subQ2"))
+            .isQuery());
 
     // Invalid query hints.
-    assertFalse(parser.isQuery("@{JOIN_METHOD=HASH_JOIN SELECT * FROM PersonsTable"));
-    assertFalse(parser.isQuery("@JOIN_METHOD=HASH_JOIN} SELECT * FROM PersonsTable"));
-    assertFalse(parser.isQuery("@JOIN_METHOD=HASH_JOIN SELECT * FROM PersonsTable"));
     assertFalse(
-        parser.isQuery(
-            "@{FORCE_INDEX=index_name} @{JOIN_METHOD=HASH_JOIN} UPDATE tbl set FOO=1 WHERE ID=2"));
+        parser.parse(Statement.of("@{JOIN_METHOD=HASH_JOIN SELECT * FROM PersonsTable")).isQuery());
+    assertFalse(
+        parser.parse(Statement.of("@JOIN_METHOD=HASH_JOIN} SELECT * FROM PersonsTable")).isQuery());
+    assertFalse(
+        parser.parse(Statement.of("@JOIN_METHOD=HASH_JOIN SELECT * FROM PersonsTable")).isQuery());
+    assertFalse(
+        parser
+            .parse(
+                Statement.of(
+                    "@{FORCE_INDEX=index_name} @{JOIN_METHOD=HASH_JOIN} UPDATE tbl set FOO=1 WHERE ID=2"))
+            .isQuery());
   }
 
   @Test