summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Lee Shombert <shombert@google.com> 2024-04-21 17:42:44 -0700
committer Lee Shombert <shombert@google.com> 2024-04-22 16:02:07 -0700
commita27bd3dcb956b0120f528561de7c56c299ce2c9b (patch)
tree24ba8ae40b28abce585a9d28d2148a6988477417
parent60da1b02fa5596d35266d78b3cba0ebade70e084 (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.java70
-rw-r--r--core/java/android/database/sqlite/flags.aconfig8
-rw-r--r--core/tests/coretests/src/android/database/DatabaseUtilsTest.java6
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"));