Do not allocate new Strings in stripLeadingSqlComments

requery · Aug 29, 2023 · 19a3213 · 19a3213
1 parent ec3e07c
commit 19a3213
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 17 deletions.
diff --git a/...roid/src/androidTest/java/io/requery/android/database/sqlite/SQLiteStatementTypeTest.java b/...roid/src/androidTest/java/io/requery/android/database/sqlite/SQLiteStatementTypeTest.java
@@ -13,21 +13,47 @@ public void recognizesSelectQueryWithComments() {
         Assert.assertEquals(SQLiteStatementType.STATEMENT_SELECT, SQLiteStatementType.getSqlStatementType(query));
     }
 
+    @Test
+    public void recognizesUntrimmedSelectQueryWithoutComments() {
+        String query = " \n SELECT * FROM table WHERE id = 1";
+        Assert.assertEquals(SQLiteStatementType.STATEMENT_SELECT, SQLiteStatementType.getSqlStatementType(query));
+    }
+
+    @Test
+    public void recognizesTrimmedSelectQueryWithoutComments() {
+        String query = "SELECT * FROM table WHERE id = 1";
+        Assert.assertEquals(SQLiteStatementType.STATEMENT_SELECT, SQLiteStatementType.getSqlStatementType(query));
+    }
+
+    @Test
+    public void recognizesUpdateQueryWithComments() {
+        String query = "--comment\nINSERT INTO phones (num) VALUES ('911');";
+        Assert.assertEquals(SQLiteStatementType.STATEMENT_UPDATE, SQLiteStatementType.getSqlStatementType(query));
+    }
+
+    @Test
+    public void notCrashingOnInvalidQuery() {
+        // Checking for index out of bounds, because `getSqlStatementType` uses (statementStartIndex + 3) as its end index
+        String query = "--comment\nSE";
+        Assert.assertEquals(SQLiteStatementType.STATEMENT_OTHER, SQLiteStatementType.getSqlStatementType(query));
+    }
+
     @Test
     public void testStripSqlComments() {
         for (TestData test : queriesTestData) {
-            String strippedSql = SQLiteStatementType.stripLeadingSqlComments(test.inputQuery);
+            int start = SQLiteStatementType.statementStartIndex(test.inputQuery);
+            String strippedSql = test.inputQuery.substring(start);
             Assert.assertEquals("Error in test case\n" + test.inputQuery, test.expectedQuery, strippedSql);
         }
     }
 
     private static final TestData[] queriesTestData = {
             test("", ""),
-            test(" ", ""),
-            test("\n", ""),
+            test(" ", " "),
+            test("\n", "\n"),
             test(
                     "\n-- ?1 - version id, required\n-- ?2 - account id, optional\nSELECT\n  SUM(col1 + col2) AS count\nFROM\n  Accounts\nWHERE\n  id = ?1\nAND\n  col3 = 0\nAND\n  CASE WHEN COALESCE(?2, '') = '' THEN 1 ELSE entityId = ?2 END\n",
-                    "SELECT\n  SUM(col1 + col2) AS count\nFROM\n  Accounts\nWHERE\n  id = ?1\nAND\n  col3 = 0\nAND\n  CASE WHEN COALESCE(?2, '') = '' THEN 1 ELSE entityId = ?2 END"
+                    "SELECT\n  SUM(col1 + col2) AS count\nFROM\n  Accounts\nWHERE\n  id = ?1\nAND\n  col3 = 0\nAND\n  CASE WHEN COALESCE(?2, '') = '' THEN 1 ELSE entityId = ?2 END\n"
             ),
             test(
                     "select * from employees",
@@ -95,16 +121,21 @@ public void testStripSqlComments() {
             ),
             test(
                     "\nSELECT\n  'All Accounts' AS name,\n  'all-accounts' AS internal_name\nFROM\n  Accounts\nWHERE\n  id = ?1\nAND\n  col3 = 0\n    ",
-                    "SELECT\n  'All Accounts' AS name,\n  'all-accounts' AS internal_name\nFROM\n  Accounts\nWHERE\n  id = ?1\nAND\n  col3 = 0"
+                    "SELECT\n  'All Accounts' AS name,\n  'all-accounts' AS internal_name\nFROM\n  Accounts\nWHERE\n  id = ?1\nAND\n  col3 = 0\n    "
             ),
             test(
                     "/* Multiline Line Block Comment\nLine 2\nLine 3 */-- single line comment\nselect * from employees",
                     "select * from employees"
             ),
             test(
                     "/* Multiline Line Block Comment\nhttps://foo.bar.com/document/d/283472938749/foo.ts\nLine 3 */-- single line comment\nSELECT\n  'All Accounts' AS name,\n  'all-accounts' AS internal_name\nFROM\n  Accounts\nWHERE\n  id = ?1\nAND\n  col3 = 0\n    ",
-                    "SELECT\n  'All Accounts' AS name,\n  'all-accounts' AS internal_name\nFROM\n  Accounts\nWHERE\n  id = ?1\nAND\n  col3 = 0"
-            )
+                    "SELECT\n  'All Accounts' AS name,\n  'all-accounts' AS internal_name\nFROM\n  Accounts\nWHERE\n  id = ?1\nAND\n  col3 = 0\n    "
+            ),
+            // Shouldn't crash on invalid query
+            test(
+                    "/* Single Line Block Comment */SE",
+                    "SE"
+            ),
     };
 
     static class TestData {

diff --git a/sqlite-android/src/main/java/io/requery/android/database/sqlite/SQLiteStatementType.java b/sqlite-android/src/main/java/io/requery/android/database/sqlite/SQLiteStatementType.java
@@ -60,12 +60,12 @@ private SQLiteStatementType() {
      * @return one of the values listed above
      */
     public static int getSqlStatementType(String sql) {
-        // Strip leading comments to properly recognize the statement type
-        sql = stripLeadingSqlComments(sql);
         if (sql.length() < 3) {
             return STATEMENT_OTHER;
         }
-        String prefixSql = sql.substring(0, 3);
+        // Skip leading comments to properly recognize the statement type
+        int statementStart = statementStartIndex(sql);
+        String prefixSql = sql.substring(statementStart, Math.min(statementStart + 3, sql.length()));
 
         if (prefixSql.equalsIgnoreCase("SEL")
                 || prefixSql.equalsIgnoreCase("WIT")) {
@@ -107,14 +107,11 @@ public static int getSqlStatementType(String sql) {
     }
 
     /**
-     * Removes only leading comments, i.e. before the first non-commented statement.
-     *
-     * @param sql sql statement to remove comments from
-     * @return trimmed sql statement with leading comments removed
+     * @param sql sql statement to check
+     * @return index of the SQL statement start, skipping leading comments
      */
     @VisibleForTesting
-    static String stripLeadingSqlComments(String sql) {
-        sql = sql.trim();
+    static int statementStartIndex(String sql) {
         boolean inSingleLineComment = false;
         boolean inMultiLineComment = false;
         int statementStartIndex = 0;
@@ -144,6 +141,6 @@ static String stripLeadingSqlComments(String sql) {
             }
         }
 
-        return sql.substring(statementStartIndex);
+        return statementStartIndex;
     }
 }