Fix detecting of offset & limit keywords

alex268 · Sep 19, 2024 · 3e6f173 · 3e6f173
1 parent b63b14b
commit 3e6f173
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 16 deletions.
diff --git a/jdbc/src/main/java/tech/ydb/jdbc/query/YdbQueryParser.java b/jdbc/src/main/java/tech/ydb/jdbc/query/YdbQueryParser.java
@@ -124,6 +124,8 @@ public String parseSQL(String origin) throws SQLException {
                     }
                     break;
                 default:
+                    lastKeywordIsOffsetLimit = lastKeywordIsOffsetLimit && Character.isWhitespace(ch);
+
                     if (keywordStart >= 0) {
                         isInsideKeyword = Character.isJavaIdentifierPart(ch);
                         break;
@@ -136,6 +138,7 @@ public String parseSQL(String origin) throws SQLException {
                     break;
             }
 
+
             if (keywordStart >= 0 && (!isInsideKeyword || (i == chars.length - 1))) {
                 lastKeywordIsOffsetLimit = false;
                 int keywordLength = (isInsideKeyword ? i + 1 : keywordEnd) - keywordStart;
@@ -150,7 +153,7 @@ public String parseSQL(String origin) throws SQLException {
 
                     if (parseOffsetKeyword(chars, keywordStart, keywordLength)
                             || parseLimitKeyword(chars, keywordStart, keywordLength)) {
-                        lastKeywordIsOffsetLimit = true;
+                        lastKeywordIsOffsetLimit = Character.isWhitespace(ch);
                     }
                 } else {
                     boolean skipped = false;

diff --git a/jdbc/src/test/java/tech/ydb/jdbc/query/YdbQueryParserTest.java b/jdbc/src/test/java/tech/ydb/jdbc/query/YdbQueryParserTest.java
@@ -154,29 +154,55 @@ public void scanSelectWithKeyTest() throws SQLException {
         Assertions.assertEquals(QueryType.SCAN_QUERY, parser.detectQueryType());
     }
 
-    @Test
-    public void offsetParameterTest() throws SQLException {
-        String query = ""
-                + "select * from test_table where true=true -- test request\n"
-                + " offset /* comment */ ? limit 20";
+    @ParameterizedTest(name = "[{index}] {0} has offset or limit parameter")
+    @ValueSource(strings = {
+        "select * from test_table where true=true -- test request\noffset /* comment */ ? limit 20",
+        "select * from test_table where true=true /*comm*/offset ?\tlimit\t\n?;",
+        "select offset, limit from test_table  offset 20 limit -- comment\n?;",
+    })
+    public void offsetParameterTest(String query) throws SQLException {
+        YdbQueryParser parser = new YdbQueryParser(true, true);
+        parser.parseSQL(query);
+
+        Assertions.assertEquals(1, parser.getStatements().size());
 
+        QueryStatement statement = parser.getStatements().get(0);
+        Assertions.assertEquals(QueryType.DATA_QUERY, statement.getType());
+
+        Assertions.assertFalse(statement.getParams().isEmpty());
+        int idx = 0;
+        for (ParamDescription prm : statement.getParams()) {
+            idx++;
+            Assertions.assertEquals("$jp" + idx, prm.name());
+            Assertions.assertNotNull(prm.type()); // forced UInt64 type
+            Assertions.assertEquals(PrimitiveType.Uint64, prm.type().ydbType()); // forced UInt64 type
+        }
+    }
+
+    @ParameterizedTest(name = "[{index}] {0} hasn't offset or limit parameter")
+    @ValueSource(strings = {
+        "select * from test_table where limit = ? or offset = ?",
+        "update test_table set limit = ?, offset = ? where id = ?",
+        "select * from test_table where limit=? or offset=?",
+        "update test_table set limit=?, offset=? where id=?",
+        "select * from test_table where limit? or offset?",
+    })
+    public void noOffsetParameterTest(String query) throws SQLException {
         YdbQueryParser parser = new YdbQueryParser(true, true);
-        String parsed = parser.parseSQL(query);
-        Assertions.assertEquals(""
-                + "select * from test_table where true=true -- test request\n"
-                + " offset /* comment */ $jp1 limit 20",
-                parsed);
+        parser.parseSQL(query);
 
         Assertions.assertEquals(1, parser.getStatements().size());
 
         QueryStatement statement = parser.getStatements().get(0);
         Assertions.assertEquals(QueryType.DATA_QUERY, statement.getType());
-        Assertions.assertEquals(1, statement.getParams().size());
 
-        ParamDescription prm1 = statement.getParams().get(0);
-        Assertions.assertEquals("$jp1", prm1.name());
-        Assertions.assertNotNull(prm1.type());
-        Assertions.assertEquals(PrimitiveType.Uint64, prm1.type().ydbType());
+        Assertions.assertFalse(statement.getParams().isEmpty());
+        int idx = 0;
+        for (ParamDescription prm : statement.getParams()) {
+            idx++;
+            Assertions.assertEquals("$jp" + idx, prm.name());
+            Assertions.assertNull(prm.type()); // uknown type
+        }
     }
 
     @ParameterizedTest(name = "[{index}] {0} is batched insert query")