diff options
| author | 2024-04-21 17:42:44 -0700 | |
|---|---|---|
| committer | 2024-04-22 16:02:07 -0700 | |
| commit | a27bd3dcb956b0120f528561de7c56c299ce2c9b (patch) | |
| tree | 24ba8ae40b28abce585a9d28d2148a6988477417 | |
| parent | 60da1b02fa5596d35266d78b3cba0ebade70e084 (diff) | |
Perform SQL comment scanning with a manual parser
SQL strings are scanned to find the (first) SQL command and to
characterize the statement. This change replaces a regular expression
parser with a hand-crafted parser.
Custom unit tests indicate that the new parser is 10x faster than the
regex implementation. Native memory allocation done by the Java
Matcher class is eliminated.
Additional unit tests have been added to test the scanner.
Test : atest
* FrameworksCoreTests:android.database
Flag: android.database.sqlite.simple_sql_comment_scanner
Bug: 329118560
Change-Id: I3124dd7f57147607a07b42cfa711e54a81348b10
| -rw-r--r-- | core/java/android/database/DatabaseUtils.java | 70 | ||||
| -rw-r--r-- | core/java/android/database/sqlite/flags.aconfig | 8 | ||||
| -rw-r--r-- | core/tests/coretests/src/android/database/DatabaseUtilsTest.java | 6 |
3 files changed, 78 insertions, 6 deletions
diff --git a/core/java/android/database/DatabaseUtils.java b/core/java/android/database/DatabaseUtils.java index 2081ced3c484..7aa034966011 100644 --- a/core/java/android/database/DatabaseUtils.java +++ b/core/java/android/database/DatabaseUtils.java @@ -22,6 +22,7 @@ import android.compat.annotation.UnsupportedAppUsage; import android.content.ContentValues; import android.content.Context; import android.content.OperationApplicationException; +import android.database.sqlite.Flags; import android.database.sqlite.SQLiteAbortException; import android.database.sqlite.SQLiteConstraintException; import android.database.sqlite.SQLiteDatabase; @@ -1609,7 +1610,7 @@ public class DatabaseUtils { * Comments either start with "--" and run to the end of the line or are C-style block * comments. The function returns null if a prefix could not be found. */ - private static String getSqlStatementPrefixExtended(String sql) { + private static String getSqlStatementPrefixExtendedRegex(String sql) { Matcher m = sPrefixPattern.matcher(sql); if (m.lookingAt()) { return m.group(PREFIX_GROUP_NUM).toUpperCase(Locale.ROOT); @@ -1619,6 +1620,61 @@ public class DatabaseUtils { } /** + * Return the index of the first character past comments and whitespace. -1 is returned if + * a comment is malformed. + */ + private static int getSqlStatementPrefixOffset(String s) { + final int limit = s.length() - 2; + if (limit < 0) return -1; + int i = 0; + while (i < limit) { + final char c = s.charAt(i); + if (c <= ' ') { + // This behavior conforms to String.trim(), which is used by the legacy Android + // SQL prefix logic. This test is not unicode-aware. Notice that it accepts the + // null character as whitespace even though the null character will terminate the + // SQL string in native code. + i++; + } else if (c == '-') { + if (s.charAt(i+1) != '-') return i; + i = s.indexOf('\n', i+2); + if (i < 0) return -1; + i++; + } else if (c == '/') { + if (s.charAt(i+1) != '*') return i; + i++; + do { + i = s.indexOf('*', i+1); + if (i < 0) return -1; + i++; + } while (s.charAt(i) != '/'); + i++; + } else { + return i; + } + } + return -1; + } + + /** + * Scan past leading comments without using the Java regex routines. + */ + private static String getSqlStatementPrefixExtendedNoRegex(String sql) { + int n = getSqlStatementPrefixOffset(sql); + if (n < 0) { + // Bad comment syntax. + return null; + } + final int end = sql.length(); + if (n > end) { + // Bad scanning. This indicates a programming error. + return null; + } + final int eos = Math.min(n+3, end); + return sql.substring(n, eos); + } + + /** * Return the extended statement type for the SQL statement. This is not a public API and it * can return values that are not publicly visible. * @hide @@ -1663,11 +1719,15 @@ public class DatabaseUtils { * @hide */ public static int getSqlStatementTypeExtended(@NonNull String sql) { - int type = categorizeStatement(getSqlStatementPrefixSimple(sql), sql); - if (type == STATEMENT_COMMENT) { - type = categorizeStatement(getSqlStatementPrefixExtended(sql), sql); + if (Flags.simpleSqlCommentScanner()) { + return categorizeStatement(getSqlStatementPrefixExtendedNoRegex(sql), sql); + } else { + int type = categorizeStatement(getSqlStatementPrefixSimple(sql), sql); + if (type == STATEMENT_COMMENT) { + type = categorizeStatement(getSqlStatementPrefixExtendedRegex(sql), sql); + } + return type; } - return type; } /** diff --git a/core/java/android/database/sqlite/flags.aconfig b/core/java/android/database/sqlite/flags.aconfig index 7ecffaf01549..1f06dfac255a 100644 --- a/core/java/android/database/sqlite/flags.aconfig +++ b/core/java/android/database/sqlite/flags.aconfig @@ -16,3 +16,11 @@ flag { description: "Permit updates to TEMP tables in read-only transactions" bug: "317993835" } + +flag { + name: "simple_sql_comment_scanner" + namespace: "system_performance" + is_fixed_read_only: true + description: "Scan SQL comments by hand instead of with a regex" + bug: "329118560" +} diff --git a/core/tests/coretests/src/android/database/DatabaseUtilsTest.java b/core/tests/coretests/src/android/database/DatabaseUtilsTest.java index 2323527e7b4c..c9cb2cc416e8 100644 --- a/core/tests/coretests/src/android/database/DatabaseUtilsTest.java +++ b/core/tests/coretests/src/android/database/DatabaseUtilsTest.java @@ -78,7 +78,8 @@ public class DatabaseUtilsTest { final int sel = STATEMENT_SELECT; assertEquals(sel, getSqlStatementType("SELECT")); assertEquals(sel, getSqlStatementType(" SELECT")); - assertEquals(sel, getSqlStatementType(" \n SELECT")); + assertEquals(sel, getSqlStatementType(" \n\r\f\t SELECT")); + assertEquals(sel, getSqlStatementType(" \n\r\f\t SEL")); final int upd = STATEMENT_UPDATE; assertEquals(upd, getSqlStatementType("UPDATE")); @@ -95,6 +96,9 @@ public class DatabaseUtilsTest { assertEquals(othr, getSqlStatementType("SE LECT")); assertEquals(othr, getSqlStatementType("-- cmt\n SE")); assertEquals(othr, getSqlStatementType("WITH")); + assertEquals(othr, getSqlStatementType("-")); + assertEquals(othr, getSqlStatementType("--")); + assertEquals(othr, getSqlStatementType("*/* foo */ SEL")); // Verify that leading line-comments are skipped. assertEquals(sel, getSqlStatementType("-- cmt\n SELECT")); |